Bug 28318 - [gdb/build] std::thread support configure check does not use CXX_DIALECT
Summary: [gdb/build] std::thread support configure check does not use CXX_DIALECT
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: build (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 11.2
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-08 07:49 UTC by Tom de Vries
Modified: 2021-10-04 18:58 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
[gdb/build] Add CXX_DIALECT to CXX (674 bytes, patch)
2021-09-08 08:48 UTC, Tom de Vries
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2021-09-08 07:49:39 UTC
As reported here (https://sourceware.org/pipermail/gdb-patches/2021-September/181862.html ):
...
I find:
...
CXX_DIALECT='-std=gnu++11'
...
in the config.log, but that setting is not used when checking for
CXX_STD_THREAD, so we have:
...
configure:14614: checking for std::thread
configure:14631: g++ -c -pthread -Wall -O2 -g     conftest.cpp >&5
In file included from /usr/include/c++/4.8/thread:35:0,
                 from conftest.cpp:167:
/usr/include/c++/4.8/bits/c++0x_warning.h:32:2: error: #error This file
requires compiler and library support for the ISO C++ 2011 standard.
This support is currently experimental, and must be enabled with the
-std=c++11 or -std=gnu++11 compiler options.
 #error This file requires compiler and library support for the \
  ^
conftest.cpp: In function 'int main()':
conftest.cpp:172:1: error: 'thread' is not a member of 'std'
 std::thread t(callback);
 ^
conftest.cpp:172:13: error: expected ';' before 't'
 std::thread t(callback);
             ^
configure:14631: $? = 1
...

It could be that:
...
$ g++ -std=gnu++11 -c -pthread -Wall -O2 -g     conftest.cpp
...
actually would succeed.
...
Comment 1 Tom de Vries 2021-09-08 08:04:48 UTC
We have a custom implementation of AX_CXX_COMPILE_STDCXX at gdb/ax_cxx_compile_stdcxx.m4:
...
    Use AX_CXX_COMPILE_STDCXX to detect if the compiler supports C++11,
    and if -std=xxx switches are necessary to enable C++11.
    
    We need to tweak AX_CXX_COMPILE_STDCXX a bit though.  Pristine
    upstream AX_CXX_COMPILE_STDCXX appends -std=gnu++11 to CXX directly.
    That doesn't work for us, because the top level Makefile passes CXX
    down to subdirs, and that overrides whatever gdb/Makefile may set CXX
    to.  The result would be that a make invocation from the build/gdb/
    directory would use "g++ -std=gnu++11" as expected, while a make
    invocation at the top level would not.
    
    So instead of having AX_CXX_COMPILE_STDCXX set CXX directly, tweak it
    to AC_SUBST a separate variable -- CXX_DIALECT -- and use '$(CXX)
    (CXX_DIALECT)' to compile/link.
...
which introduced the CXX_DIALECT.

If we would use the regular AX_CXX_COMPILE_STDCXX, AFAIU the problem would not occur.

So, either:
- we add CXX_DIALECT to all uses of CXX in the configure script, or 
- we add CXX_DIALECT to CXX directly.  AFAIU, that wouldn't break anything
  though it would result in confusing double options in command lines.
Comment 2 Tom de Vries 2021-09-08 08:48:38 UTC
Created attachment 13657 [details]
[gdb/build] Add CXX_DIALECT to CXX

(In reply to Tom de Vries from comment #1)
> - we add CXX_DIALECT to CXX directly.  AFAIU, that wouldn't break anything
>   though it would result in confusing double options in command lines.

Patch implementing this.

Indeed now we have:
...
configure:8916: checking for std::thread
configure:8933: g++ -std=gnu++11 -c -pthread -Wall -O2 -g     conftest.cpp >&5
configure:8933: $? = 0
configure:8940: result: yes
...
but indeed in the build/gdb/Makefile:
...
CXX = g++ -std=gnu++11
CXX_DIALECT = -std=gnu++11
...
Comment 3 Tom de Vries 2021-09-08 08:51:09 UTC
(In reply to Tom de Vries from comment #2)
> but indeed in the build/gdb/Makefile:
> ...
> CXX = g++ -std=gnu++11
> CXX_DIALECT = -std=gnu++11
> ...

As well as:
...
CC = gcc -std=gnu99
...

It seems we have CXX_DIALECT but not C_DIALECT.
Comment 5 Sourceware Commits 2021-10-04 16:56:46 UTC
The gdb-11-branch branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9b4d030ed52b52fe208edaa8be640f4dd359defc

commit 9b4d030ed52b52fe208edaa8be640f4dd359defc
Author: Tom de Vries <tdevries@suse.de>
Date:   Mon Oct 4 18:56:42 2021 +0200

    [gdb/build] Add CXX_DIALECT to CXX
    
    Say we use a gcc version that (while supporting c++11) does not support c++11
    by default, and needs an -std setting to enable it.
    
    If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf-archive, then
    we'd have:
    ...
    CXX="g++ -std=gnu++11"
    ...
    
    That mechanism however has the following problem (quoting from commit
    0bcda685399):
    ...
    the top level Makefile passes CXX down to subdirs, and that overrides whatever
    gdb/Makefile may set CXX to.  The result would be that a make invocation from
    the build/gdb/ directory would use "g++ -std=gnu++11" as expected, while a
    make invocation at the top level would not.
    ...
    
    Commit 0bcda685399 fixes this by using a custom AX_CXX_COMPILE_STDCXX which
    does:
    ...
    CXX=g++
    CXX_DIALECT=-std=gnu++11
    ...
    
    The problem reported in PR28318 is that using the custom instead of the
    default AX_CXX_COMPILE_STDCXX makes the configure test for std::thread
    support fail.
    
    We could simply add $CXX_DIALECT to the test for std::thread support, but
    that would have to be repeated for each added c++ support test.
    
    Instead, fix this by doing:
    ...
    CXX="g++ -std=gnu++11"
    CXX_DIALECT=-std=gnu++11
    ...
    
    This is somewhat awkward, since it results in -std=gnu++11 occuring twice in
    some situations:
    ...
    $ touch src/gdb/dwarf2/read.c
    $ ( cd build/gdb; make V=1 dwarf2/read.o )
    g++-4.8 -std=gnu++11 -x c++ -std=gnu++11 ...
    ...
    
    However, both settings are needed:
     - the switch in CXX for the std::thread tests (and other tests)
     - the switch in CXX_DIALECT so it can be appended in Makefiles, to
       counteract the fact that the top-level Makefile overrides CXX
    
    The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default
    AX_CXX_COMPILE_STDCXX from autoconf-archive.
    
    Tested on x86_64-linux.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28318
    
    gdb/ChangeLog:
    
    2021-10-04  Tom de Vries  <tdevries@suse.de>
    
            PR build/28318
            * ax_cxx_compile_stdcxx.m4: Add CXX_DIALECT to CXX.
            * configure: Regenerate.
    
    gdbserver/ChangeLog:
    
    2021-10-04  Tom de Vries  <tdevries@suse.de>
    
            PR build/28318
            * configure: Regenerate.
    
    gdbsupport/ChangeLog:
    
    2021-10-04  Tom de Vries  <tdevries@suse.de>
    
            PR build/28318
            * configure: Regenerate.
Comment 6 Tom de Vries 2021-10-04 16:58:54 UTC
This should be closed with target milestone 11.2, waiting for milestone to become available.
Comment 7 Joel Brobecker 2021-10-04 18:31:33 UTC
Adding the 11.2 milestone to indicate that this was fixed in that version.
Otherwise, we'll be missing this item from the 11.2 release announcement.
Comment 8 Tom de Vries 2021-10-04 18:58:39 UTC
Patch committed to trunk and gdb-11-branch, marking resolved-fixed.