* [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX @ 2021-09-08 12:15 Tom de Vries via Gdb-patches 2021-09-20 23:04 ` [PING][PATCH, " Tom de Vries via Gdb-patches 2021-09-23 14:27 ` [PATCH, " Simon Marchi via Gdb-patches 0 siblings, 2 replies; 7+ messages in thread From: Tom de Vries via Gdb-patches @ 2021-09-08 12:15 UTC (permalink / raw) To: gdb-patches Hi, The problem reported in PR28318 is that when using a gcc version that (while supporting c++11) does not support c++11 by default, the configure test for std::thread support will fail. If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf, then we'd have: ... CXX="g++ -std=gnu++11" ... and the test for std::thread support would succeed. Instead, gdb uses a custom AX_CXX_COMPILE_STDCXX (see commit 0bcda685399) which does: ... CXX=g++ CXX_DIALECT=-std=gnu++11 ... We could 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 ... The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default AX_CXX_COMPILE_STDCXX from autoconf. Tested on x86_64-linux. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28318 Any comments? Thanks, - Tom [gdb/build] Add CXX_DIALECT to CXX --- gdb/ax_cxx_compile_stdcxx.m4 | 8 ++++++++ gdb/configure | 8 ++++++++ gdbserver/configure | 8 ++++++++ gdbsupport/configure | 8 ++++++++ 4 files changed, 32 insertions(+) diff --git a/gdb/ax_cxx_compile_stdcxx.m4 b/gdb/ax_cxx_compile_stdcxx.m4 index 413755a2e88..29d8e10bcc4 100644 --- a/gdb/ax_cxx_compile_stdcxx.m4 +++ b/gdb/ax_cxx_compile_stdcxx.m4 @@ -94,6 +94,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl CXX="$ac_save_CXX"]) if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -118,6 +122,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl CXX="$ac_save_CXX"]) if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi diff --git a/gdb/configure b/gdb/configure index f0b1af4a6ea..98badb46cfd 100755 --- a/gdb/configure +++ b/gdb/configure @@ -5841,6 +5841,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -6160,6 +6164,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi diff --git a/gdbserver/configure b/gdbserver/configure index b227167e270..f05c1a9b976 100755 --- a/gdbserver/configure +++ b/gdbserver/configure @@ -5625,6 +5625,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -5944,6 +5948,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi diff --git a/gdbsupport/configure b/gdbsupport/configure index a9dd02c5b72..ae6047865ae 100755 --- a/gdbsupport/configure +++ b/gdbsupport/configure @@ -6520,6 +6520,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -6839,6 +6843,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PING][PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX 2021-09-08 12:15 [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX Tom de Vries via Gdb-patches @ 2021-09-20 23:04 ` Tom de Vries via Gdb-patches 2021-09-23 14:27 ` [PATCH, " Simon Marchi via Gdb-patches 1 sibling, 0 replies; 7+ messages in thread From: Tom de Vries via Gdb-patches @ 2021-09-20 23:04 UTC (permalink / raw) To: gdb-patches On 9/8/21 2:15 PM, Tom de Vries wrote: > Hi, > > The problem reported in PR28318 is that when using a gcc version that (while > supporting c++11) does not support c++11 by default, the configure test for > std::thread support will fail. > > If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf, then we'd > have: > ... > CXX="g++ -std=gnu++11" > ... > and the test for std::thread support would succeed. > > Instead, gdb uses a custom AX_CXX_COMPILE_STDCXX (see commit 0bcda685399) > which does: > ... > CXX=g++ > CXX_DIALECT=-std=gnu++11 > ... > > We could 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 > ... > > The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default > AX_CXX_COMPILE_STDCXX from autoconf. > > Tested on x86_64-linux. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28318 > > Any comments? > Ping. Thanks, - Tom > [gdb/build] Add CXX_DIALECT to CXX > > --- > gdb/ax_cxx_compile_stdcxx.m4 | 8 ++++++++ > gdb/configure | 8 ++++++++ > gdbserver/configure | 8 ++++++++ > gdbsupport/configure | 8 ++++++++ > 4 files changed, 32 insertions(+) > > diff --git a/gdb/ax_cxx_compile_stdcxx.m4 b/gdb/ax_cxx_compile_stdcxx.m4 > index 413755a2e88..29d8e10bcc4 100644 > --- a/gdb/ax_cxx_compile_stdcxx.m4 > +++ b/gdb/ax_cxx_compile_stdcxx.m4 > @@ -94,6 +94,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl > CXX="$ac_save_CXX"]) > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -118,6 +122,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl > CXX="$ac_save_CXX"]) > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > diff --git a/gdb/configure b/gdb/configure > index f0b1af4a6ea..98badb46cfd 100755 > --- a/gdb/configure > +++ b/gdb/configure > @@ -5841,6 +5841,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -6160,6 +6164,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > diff --git a/gdbserver/configure b/gdbserver/configure > index b227167e270..f05c1a9b976 100755 > --- a/gdbserver/configure > +++ b/gdbserver/configure > @@ -5625,6 +5625,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -5944,6 +5948,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > diff --git a/gdbsupport/configure b/gdbsupport/configure > index a9dd02c5b72..ae6047865ae 100755 > --- a/gdbsupport/configure > +++ b/gdbsupport/configure > @@ -6520,6 +6520,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -6839,6 +6843,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX 2021-09-08 12:15 [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX Tom de Vries via Gdb-patches 2021-09-20 23:04 ` [PING][PATCH, " Tom de Vries via Gdb-patches @ 2021-09-23 14:27 ` Simon Marchi via Gdb-patches 2021-09-23 14:33 ` Pedro Alves 2021-09-27 12:59 ` Tom de Vries via Gdb-patches 1 sibling, 2 replies; 7+ messages in thread From: Simon Marchi via Gdb-patches @ 2021-09-23 14:27 UTC (permalink / raw) To: Tom de Vries, gdb-patches On 2021-09-08 8:15 a.m., Tom de Vries wrote: > Hi, > > The problem reported in PR28318 is that when using a gcc version that (while > supporting c++11) does not support c++11 by default, the configure test for > std::thread support will fail. > > If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf, then we'd > have: > ... > CXX="g++ -std=gnu++11" > ... > and the test for std::thread support would succeed. > > Instead, gdb uses a custom AX_CXX_COMPILE_STDCXX (see commit 0bcda685399) > which does: > ... > CXX=g++ > CXX_DIALECT=-std=gnu++11 > ... > > We could 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 > ... > > The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default > AX_CXX_COMPILE_STDCXX from autoconf. From autoconf-archive? At first glance, it looks like CXX_DIALECT would not be needed anymore, since it's only used to be appended to CXX in Makefiles. So now it would end up twice in the command-line, which is not harmful but not useful. However, if we look at the original explanation of why we have this local modification: 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. https://gitlab.com/gnutools/binutils-gdb/-/commit/0bcda68539948828795564b35a497dc69c27f768 So it sounds like we need both (and your patch is correct): - 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 This should probably be explained in the comment, because that situation is strange, and people might wonder why we end up with two -std=... switches, and might try to "fix" it. Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX 2021-09-23 14:27 ` [PATCH, " Simon Marchi via Gdb-patches @ 2021-09-23 14:33 ` Pedro Alves 2021-09-27 12:59 ` Tom de Vries via Gdb-patches 1 sibling, 0 replies; 7+ messages in thread From: Pedro Alves @ 2021-09-23 14:33 UTC (permalink / raw) To: Simon Marchi, Tom de Vries, gdb-patches On 2021-09-23 3:27 p.m., Simon Marchi wrote: > > > On 2021-09-08 8:15 a.m., Tom de Vries wrote: >> Hi, >> >> The problem reported in PR28318 is that when using a gcc version that (while >> supporting c++11) does not support c++11 by default, the configure test for >> std::thread support will fail. >> >> If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf, then we'd >> have: >> ... >> CXX="g++ -std=gnu++11" >> ... >> and the test for std::thread support would succeed. >> >> Instead, gdb uses a custom AX_CXX_COMPILE_STDCXX (see commit 0bcda685399) >> which does: >> ... >> CXX=g++ >> CXX_DIALECT=-std=gnu++11 >> ... >> >> We could 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 >> ... >> >> The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default >> AX_CXX_COMPILE_STDCXX from autoconf. > > From autoconf-archive? > > At first glance, it looks like CXX_DIALECT would not be needed anymore, > since it's only used to be appended to CXX in Makefiles. So now it > would end up twice in the command-line, which is not harmful but not > useful. > > However, if we look at the original explanation of why we have this > local modification: > > 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. > > https://gitlab.com/gnutools/binutils-gdb/-/commit/0bcda68539948828795564b35a497dc69c27f768 > > So it sounds like we need both (and your patch is correct): > > - 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 Thanks for raising this aspect. I had been meaning to reply raising it, and ask where that original motivation is still a concern, as it wasn't clear from the patch description. Pedro Alves > > This should probably be explained in the comment, because that situation > is strange, and people might wonder why we end up with two -std=... > switches, and might try to "fix" it. > > Simon > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX 2021-09-23 14:27 ` [PATCH, " Simon Marchi via Gdb-patches 2021-09-23 14:33 ` Pedro Alves @ 2021-09-27 12:59 ` Tom de Vries via Gdb-patches 2021-10-04 13:58 ` Tom de Vries via Gdb-patches 1 sibling, 1 reply; 7+ messages in thread From: Tom de Vries via Gdb-patches @ 2021-09-27 12:59 UTC (permalink / raw) To: Simon Marchi, gdb-patches [-- Attachment #1: Type: text/plain, Size: 2705 bytes --] On 9/23/21 4:27 PM, Simon Marchi wrote: > > > On 2021-09-08 8:15 a.m., Tom de Vries wrote: >> Hi, >> >> The problem reported in PR28318 is that when using a gcc version that (while >> supporting c++11) does not support c++11 by default, the configure test for >> std::thread support will fail. >> >> If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf, then we'd >> have: >> ... >> CXX="g++ -std=gnu++11" >> ... >> and the test for std::thread support would succeed. >> >> Instead, gdb uses a custom AX_CXX_COMPILE_STDCXX (see commit 0bcda685399) >> which does: >> ... >> CXX=g++ >> CXX_DIALECT=-std=gnu++11 >> ... >> >> We could 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 >> ... >> >> The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default >> AX_CXX_COMPILE_STDCXX from autoconf. > > From autoconf-archive? > Yes, updated. > At first glance, it looks like CXX_DIALECT would not be needed anymore, > since it's only used to be appended to CXX in Makefiles. So now it > would end up twice in the command-line, which is not harmful but not > useful. > > However, if we look at the original explanation of why we have this > local modification: > > 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. > > https://gitlab.com/gnutools/binutils-gdb/-/commit/0bcda68539948828795564b35a497dc69c27f768 > > So it sounds like we need both (and your patch is correct): > > - 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 > > This should probably be explained in the comment, because that situation > is strange, and people might wonder why we end up with two -std=... > switches, and might try to "fix" it. Ack, I've rewritten the commit log to make this more clear (and copied part of you comments). Any further comments? Thanks, - Tom [-- Attachment #2: 0001-gdb-build-Add-CXX_DIALECT-to-CXX.patch --] [-- Type: text/x-patch, Size: 5242 bytes --] [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/ax_cxx_compile_stdcxx.m4 | 8 ++++++++ gdb/configure | 8 ++++++++ gdbserver/configure | 8 ++++++++ gdbsupport/configure | 8 ++++++++ 4 files changed, 32 insertions(+) diff --git a/gdb/ax_cxx_compile_stdcxx.m4 b/gdb/ax_cxx_compile_stdcxx.m4 index 413755a2e88..29d8e10bcc4 100644 --- a/gdb/ax_cxx_compile_stdcxx.m4 +++ b/gdb/ax_cxx_compile_stdcxx.m4 @@ -94,6 +94,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl CXX="$ac_save_CXX"]) if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -118,6 +122,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl CXX="$ac_save_CXX"]) if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi diff --git a/gdb/configure b/gdb/configure index f0b1af4a6ea..98badb46cfd 100755 --- a/gdb/configure +++ b/gdb/configure @@ -5841,6 +5841,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -6160,6 +6164,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi diff --git a/gdbserver/configure b/gdbserver/configure index b227167e270..f05c1a9b976 100755 --- a/gdbserver/configure +++ b/gdbserver/configure @@ -5625,6 +5625,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -5944,6 +5948,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi diff --git a/gdbsupport/configure b/gdbsupport/configure index a9dd02c5b72..ae6047865ae 100755 --- a/gdbsupport/configure +++ b/gdbsupport/configure @@ -6520,6 +6520,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi @@ -6839,6 +6843,10 @@ eval ac_res=\$$cachevar $as_echo "$ac_res" >&6; } if eval test x\$$cachevar = xyes; then CXX_DIALECT="$switch" + CXX="$CXX $switch" + if test -n "$CXXCPP" ; then + CXXCPP="$CXXCPP $switch" + fi ac_success=yes break fi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX 2021-09-27 12:59 ` Tom de Vries via Gdb-patches @ 2021-10-04 13:58 ` Tom de Vries via Gdb-patches 2021-10-04 16:16 ` Simon Marchi via Gdb-patches 0 siblings, 1 reply; 7+ messages in thread From: Tom de Vries via Gdb-patches @ 2021-10-04 13:58 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 9/27/21 2:59 PM, Tom de Vries via Gdb-patches wrote: > On 9/23/21 4:27 PM, Simon Marchi wrote: >> >> On 2021-09-08 8:15 a.m., Tom de Vries wrote: >>> Hi, >>> >>> The problem reported in PR28318 is that when using a gcc version that (while >>> supporting c++11) does not support c++11 by default, the configure test for >>> std::thread support will fail. >>> >>> If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf, then we'd >>> have: >>> ... >>> CXX="g++ -std=gnu++11" >>> ... >>> and the test for std::thread support would succeed. >>> >>> Instead, gdb uses a custom AX_CXX_COMPILE_STDCXX (see commit 0bcda685399) >>> which does: >>> ... >>> CXX=g++ >>> CXX_DIALECT=-std=gnu++11 >>> ... >>> >>> We could 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 >>> ... >>> >>> The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default >>> AX_CXX_COMPILE_STDCXX from autoconf. >> From autoconf-archive? >> > Yes, updated. > >> At first glance, it looks like CXX_DIALECT would not be needed anymore, >> since it's only used to be appended to CXX in Makefiles. So now it >> would end up twice in the command-line, which is not harmful but not >> useful. >> >> However, if we look at the original explanation of why we have this >> local modification: >> >> 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. >> >> https://gitlab.com/gnutools/binutils-gdb/-/commit/0bcda68539948828795564b35a497dc69c27f768 >> >> So it sounds like we need both (and your patch is correct): >> >> - 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 >> >> This should probably be explained in the comment, because that situation >> is strange, and people might wonder why we end up with two -std=... >> switches, and might try to "fix" it. > Ack, I've rewritten the commit log to make this more clear (and copied > part of you comments). > > Any further comments? > I'll commit this tomorrow, unless there are further comments. Thanks, - Tom > 0001-gdb-build-Add-CXX_DIALECT-to-CXX.patch > > [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/ax_cxx_compile_stdcxx.m4 | 8 ++++++++ > gdb/configure | 8 ++++++++ > gdbserver/configure | 8 ++++++++ > gdbsupport/configure | 8 ++++++++ > 4 files changed, 32 insertions(+) > > diff --git a/gdb/ax_cxx_compile_stdcxx.m4 b/gdb/ax_cxx_compile_stdcxx.m4 > index 413755a2e88..29d8e10bcc4 100644 > --- a/gdb/ax_cxx_compile_stdcxx.m4 > +++ b/gdb/ax_cxx_compile_stdcxx.m4 > @@ -94,6 +94,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl > CXX="$ac_save_CXX"]) > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -118,6 +122,10 @@ AC_DEFUN([AX_CXX_COMPILE_STDCXX], [dnl > CXX="$ac_save_CXX"]) > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > diff --git a/gdb/configure b/gdb/configure > index f0b1af4a6ea..98badb46cfd 100755 > --- a/gdb/configure > +++ b/gdb/configure > @@ -5841,6 +5841,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -6160,6 +6164,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > diff --git a/gdbserver/configure b/gdbserver/configure > index b227167e270..f05c1a9b976 100755 > --- a/gdbserver/configure > +++ b/gdbserver/configure > @@ -5625,6 +5625,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -5944,6 +5948,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > diff --git a/gdbsupport/configure b/gdbsupport/configure > index a9dd02c5b72..ae6047865ae 100755 > --- a/gdbsupport/configure > +++ b/gdbsupport/configure > @@ -6520,6 +6520,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > @@ -6839,6 +6843,10 @@ eval ac_res=\$$cachevar > $as_echo "$ac_res" >&6; } > if eval test x\$$cachevar = xyes; then > CXX_DIALECT="$switch" > + CXX="$CXX $switch" > + if test -n "$CXXCPP" ; then > + CXXCPP="$CXXCPP $switch" > + fi > ac_success=yes > break > fi > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX 2021-10-04 13:58 ` Tom de Vries via Gdb-patches @ 2021-10-04 16:16 ` Simon Marchi via Gdb-patches 0 siblings, 0 replies; 7+ messages in thread From: Simon Marchi via Gdb-patches @ 2021-10-04 16:16 UTC (permalink / raw) To: Tom de Vries, gdb-patches On 2021-10-04 09:58, Tom de Vries wrote: > On 9/27/21 2:59 PM, Tom de Vries via Gdb-patches wrote: >> On 9/23/21 4:27 PM, Simon Marchi wrote: >>> >>> On 2021-09-08 8:15 a.m., Tom de Vries wrote: >>>> Hi, >>>> >>>> The problem reported in PR28318 is that when using a gcc version that (while >>>> supporting c++11) does not support c++11 by default, the configure test for >>>> std::thread support will fail. >>>> >>>> If gdb would use the default AX_CXX_COMPILE_STDCXX from autoconf, then we'd >>>> have: >>>> ... >>>> CXX="g++ -std=gnu++11" >>>> ... >>>> and the test for std::thread support would succeed. >>>> >>>> Instead, gdb uses a custom AX_CXX_COMPILE_STDCXX (see commit 0bcda685399) >>>> which does: >>>> ... >>>> CXX=g++ >>>> CXX_DIALECT=-std=gnu++11 >>>> ... >>>> >>>> We could 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 >>>> ... >>>> >>>> The code added in gdb/ax_cxx_compile_stdcxx.m4 is copied from the default >>>> AX_CXX_COMPILE_STDCXX from autoconf. >>> From autoconf-archive? >>> >> Yes, updated. >> >>> At first glance, it looks like CXX_DIALECT would not be needed anymore, >>> since it's only used to be appended to CXX in Makefiles. So now it >>> would end up twice in the command-line, which is not harmful but not >>> useful. >>> >>> However, if we look at the original explanation of why we have this >>> local modification: >>> >>> 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. >>> >>> https://gitlab.com/gnutools/binutils-gdb/-/commit/0bcda68539948828795564b35a497dc69c27f768 >>> >>> So it sounds like we need both (and your patch is correct): >>> >>> - 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 >>> >>> This should probably be explained in the comment, because that situation >>> is strange, and people might wonder why we end up with two -std=... >>> switches, and might try to "fix" it. >> Ack, I've rewritten the commit log to make this more clear (and copied >> part of you comments). >> >> Any further comments? >> > > I'll commit this tomorrow, unless there are further comments. > > Thanks, > - Tom That LGTM, thanks. Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-04 16:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-08 12:15 [PATCH, master+11][gdb/build] Add CXX_DIALECT to CXX Tom de Vries via Gdb-patches 2021-09-20 23:04 ` [PING][PATCH, " Tom de Vries via Gdb-patches 2021-09-23 14:27 ` [PATCH, " Simon Marchi via Gdb-patches 2021-09-23 14:33 ` Pedro Alves 2021-09-27 12:59 ` Tom de Vries via Gdb-patches 2021-10-04 13:58 ` Tom de Vries via Gdb-patches 2021-10-04 16:16 ` Simon Marchi via Gdb-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox