[PATCH] gdb: put user-supplied CFLAGS at the end

public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed

* [PATCH] gdb: put user-supplied CFLAGS at the end
@ 2020-10-05 16:40 Simon Marchi
  2020-10-07 12:10 ` Pedro Alves
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-10-05 16:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

GDB currently doesn't build cleanly with clang (a -Wdeprecated-copy-dtor
error).  I configured my clang-based GDB build with
CXXFLAGS="-Wno-error=deprecated-copy-dtor", so I can use it despite that
problem.  However, I found that it had no effect.  This is because my
-Wno-error=Wdeprecated-copy-dtor switch is followed by -Werror in the
command line, which switches back all warnings to be errors.

If we want the user-supplied C(XX)FLAGS to be able to override flags
added by our configure script, the user-supplied C(XX)FLAGS should
appear after the configure-supplied flags.

This patch moves the user-supplied flags at the very end of the command
line, which fixes the problem described above.

gdb/ChangeLog:

	* Makefile (INTERNAL_CFLAGS_BASE): Move CXXFLAGS at the end.
	(INTERNAL_WARN_CFLAGS): Move INTERNAL_CFLAGS_BASE at the end.
	(INTERNAL_CFLAGS): Move INTERNAL_WARN_CFLAGS at the end.

Change-Id: I00e054506695e0e9536095c6d14827e48abd8f69
---
 gdb/Makefile.in | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index afce26276ecd..6f4b299452d5 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -598,14 +598,14 @@ INTERNAL_CPPFLAGS = $(CPPFLAGS) @GUILE_CPPFLAGS@ @PYTHON_CPPFLAGS@ \
 
 # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
 INTERNAL_CFLAGS_BASE = \
-	$(CXXFLAGS) $(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
+	$(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
 	$(GDB_CFLAGS) $(OPCODES_CFLAGS) $(READLINE_CFLAGS) $(ZLIBINC) \
 	$(BFD_CFLAGS) $(INCLUDE_CFLAGS) $(LIBDECNUMBER_CFLAGS) \
 	$(INTL_CFLAGS) $(INCGNU) $(INCSUPPORT) $(ENABLE_CFLAGS) \
 	$(INTERNAL_CPPFLAGS) $(SRCHIGH_CFLAGS) $(TOP_CFLAGS) $(PTHREAD_CFLAGS) \
-	$(DEBUGINFOD_CFLAGS)
-INTERNAL_WARN_CFLAGS = $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS)
-INTERNAL_CFLAGS = $(INTERNAL_WARN_CFLAGS) $(GDB_WERROR_CFLAGS)
+	$(DEBUGINFOD_CFLAGS) $(CXXFLAGS)
+INTERNAL_WARN_CFLAGS = $(GDB_WARN_CFLAGS) $(INTERNAL_CFLAGS_BASE)
+INTERNAL_CFLAGS = $(GDB_WERROR_CFLAGS) $(INTERNAL_WARN_CFLAGS)
 
 # LDFLAGS is specifically reserved for setting from the command line
 # when running make.
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb: put user-supplied CFLAGS at the end
  2020-10-05 16:40 [PATCH] gdb: put user-supplied CFLAGS at the end Simon Marchi
@ 2020-10-07 12:10 ` Pedro Alves
  2020-10-07 14:25   ` Simon Marchi
  2020-10-07 15:27   ` [PATCH v2] " Simon Marchi
  0 siblings, 2 replies; 6+ messages in thread
From: Pedro Alves @ 2020-10-07 12:10 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 10/5/20 5:40 PM, Simon Marchi via Gdb-patches wrote:
> GDB currently doesn't build cleanly with clang (a -Wdeprecated-copy-dtor
> error).  I configured my clang-based GDB build with
> CXXFLAGS="-Wno-error=deprecated-copy-dtor", so I can use it despite that
> problem.  However, I found that it had no effect.  This is because my
> -Wno-error=Wdeprecated-copy-dtor switch is followed by -Werror in the
> command line, which switches back all warnings to be errors.
> 
> If we want the user-supplied C(XX)FLAGS to be able to override flags
> added by our configure script, the user-supplied C(XX)FLAGS should
> appear after the configure-supplied flags.
> 

Agreed.

> This patch moves the user-supplied flags at the very end of the command
> line, which fixes the problem described above.
> 

> gdb/ChangeLog:
> 
> 	* Makefile (INTERNAL_CFLAGS_BASE): Move CXXFLAGS at the end.

Typo: Makefile.in.

> 	(INTERNAL_WARN_CFLAGS): Move INTERNAL_CFLAGS_BASE at the end.
> 	(INTERNAL_CFLAGS): Move INTERNAL_WARN_CFLAGS at the end.
> 
> Change-Id: I00e054506695e0e9536095c6d14827e48abd8f69
> ---
>  gdb/Makefile.in | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index afce26276ecd..6f4b299452d5 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -598,14 +598,14 @@ INTERNAL_CPPFLAGS = $(CPPFLAGS) @GUILE_CPPFLAGS@ @PYTHON_CPPFLAGS@ \
>  
>  # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
>  INTERNAL_CFLAGS_BASE = \
> -	$(CXXFLAGS) $(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
> +	$(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
>  	$(GDB_CFLAGS) $(OPCODES_CFLAGS) $(READLINE_CFLAGS) $(ZLIBINC) \
>  	$(BFD_CFLAGS) $(INCLUDE_CFLAGS) $(LIBDECNUMBER_CFLAGS) \
>  	$(INTL_CFLAGS) $(INCGNU) $(INCSUPPORT) $(ENABLE_CFLAGS) \
>  	$(INTERNAL_CPPFLAGS) $(SRCHIGH_CFLAGS) $(TOP_CFLAGS) $(PTHREAD_CFLAGS) \
> -	$(DEBUGINFOD_CFLAGS)
> -INTERNAL_WARN_CFLAGS = $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS)
> -INTERNAL_CFLAGS = $(INTERNAL_WARN_CFLAGS) $(GDB_WERROR_CFLAGS)
> +	$(DEBUGINFOD_CFLAGS) $(CXXFLAGS)
> +INTERNAL_WARN_CFLAGS = $(GDB_WARN_CFLAGS) $(INTERNAL_CFLAGS_BASE)
> +INTERNAL_CFLAGS = $(GDB_WERROR_CFLAGS) $(INTERNAL_WARN_CFLAGS)
>  

$(CXXFLAGS) in INTERNAL_CFLAGS_BASE looks a little bit buried, IMHO.
The intent that CXXFLAGS must be last isn't very explicit.

Also, while compiling python files, CXXFLAGS won't be at the end:

 # Python files need special flags.
 python/%.o: INTERNAL_CFLAGS += $(PYTHON_CFLAGS)


Maybe it would be better to tweak the two spots to use INTERNAL_CFLAGS
and add CXXFLAGS there explicitly:

 + # Add comment about wanting CXXFLAGS to remain last.
 -COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(COMPILE.post)
 +COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $CXXFLAGS $(COMPILE.post)

Which would be kind of similar to what automake outputs in gdbsupport/Makefile.in :

  CXXCOMPILE = $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) \
  	$(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS)


Also, I think gdbserver/Makefile.in needs the same tweak?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gdb: put user-supplied CFLAGS at the end
  2020-10-07 12:10 ` Pedro Alves
@ 2020-10-07 14:25   ` Simon Marchi
  2020-10-07 15:27   ` [PATCH v2] " Simon Marchi
  1 sibling, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-10-07 14:25 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

On 2020-10-07 8:10 a.m., Pedro Alves wrote:
> On 10/5/20 5:40 PM, Simon Marchi via Gdb-patches wrote:
>> gdb/ChangeLog:
>>
>> 	* Makefile (INTERNAL_CFLAGS_BASE): Move CXXFLAGS at the end.
>
> Typo: Makefile.in.

Thanks, fixed.

> $(CXXFLAGS) in INTERNAL_CFLAGS_BASE looks a little bit buried, IMHO.
> The intent that CXXFLAGS must be last isn't very explicit.
>
> Also, while compiling python files, CXXFLAGS won't be at the end:
>
>  # Python files need special flags.
>  python/%.o: INTERNAL_CFLAGS += $(PYTHON_CFLAGS)
>
>
> Maybe it would be better to tweak the two spots to use INTERNAL_CFLAGS
> and add CXXFLAGS there explicitly:
>
>  + # Add comment about wanting CXXFLAGS to remain last.
>  -COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(COMPILE.post)
>  +COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $CXXFLAGS $(COMPILE.post)
>
> Which would be kind of similar to what automake outputs in gdbsupport/Makefile.in :
>
>   CXXCOMPILE = $(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) \
>   	$(AM_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS)

Makes sense, I'll do that.

> Also, I think gdbserver/Makefile.in needs the same tweak?

Probably, I'll check and apply it there if needed.

Thanks for the review.

Simon


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] gdb: put user-supplied CFLAGS at the end
  2020-10-07 12:10 ` Pedro Alves
  2020-10-07 14:25   ` Simon Marchi
@ 2020-10-07 15:27   ` Simon Marchi
  2020-10-07 17:34     ` Pedro Alves
  1 sibling, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-10-07 15:27 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

New in v2:

* Put $(CXXFLAGS) in the top-level COMPILE command instead of keeping it
  buried in INTERNAL_CFLAGS.
* Do the same change in gdbserver.

---8<---

GDB currently doesn't build cleanly with clang (a -Wdeprecated-copy-dtor
error).  I configured my clang-based GDB build with
CXXFLAGS="-Wno-error=deprecated-copy-dtor", so I can use it despite that
problem.  However, I found that it had no effect.  This is because my
-Wno-error=Wdeprecated-copy-dtor switch is followed by -Werror in the
command line, which switches back all warnings to be errors.

If we want the user-supplied C(XX)FLAGS to be able to override flags
added by our configure script, the user-supplied C(XX)FLAGS should
appear after the configure-supplied flags.

This patch moves the user-supplied CXXFLAGS at the very end of the
compilation command line, which fixes the problem described above.  This
means moving it out of INTERNAL_CFLAGS and inlining it in the users of
INTERNAL_CFLAGS.

I observed the problem when building GDB, but the same problem could
happen with GDBserver, so the change is done there too.

In GDBserver, INTERNAL_CFLAGS is passed when linking

gdb/ChangeLog:

	* Makefile.in (COMPILE): Add CXXFLAGS.
	(INTERNAL_CFLAGS_BASE): Remove CXXFLAGS.
	(check-headers): Add CXXFLAGS.

gdbserver/ChangeLog:

	* Makefile.in (COMPILE): Add CXXFLAGS.
	(INTERNAL_CFLAGS_BASE): Remove CXXFLAGS.
	(gdbserver$(EXEEXT)): Add CXXFLAGS.
	(gdbreplay$(EXEEXT)): Add CXXFLAGS.
	($(IPA_LIB)): Add CXXFLAGS.
	(IPAGENT_COMPILE): Add CXXFLAGS.

Change-Id: I00e054506695e0e9536095c6d14827e48abd8f69
---
 gdb/Makefile.in       | 10 +++++++---
 gdbserver/Makefile.in | 14 +++++++++++---
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index afce26276ecd..e14d41cef915 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -118,9 +118,13 @@ include $(srcdir)/silent-rules.mk
 # GNU make is used.  The overrides implement dependency tracking.
 COMPILE.pre = $(CXX) -x c++ $(CXX_DIALECT)
 COMPILE.post = -c -o $@
-COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(COMPILE.post)
 POSTCOMPILE = @true
 
+# CXXFLAGS is at the very end on purpose, so that user-supplied flags can
+# override internal flags.
+COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(CXXFLAGS) \
+	$(COMPILE.post)
+
 YACC = @YACC@
 
 # This is used to rebuild ada-lex.c from ada-lex.l.  If the program is
@@ -598,7 +602,7 @@ INTERNAL_CPPFLAGS = $(CPPFLAGS) @GUILE_CPPFLAGS@ @PYTHON_CPPFLAGS@ \
 
 # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
 INTERNAL_CFLAGS_BASE = \
-	$(CXXFLAGS) $(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
+	$(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) \
 	$(GDB_CFLAGS) $(OPCODES_CFLAGS) $(READLINE_CFLAGS) $(ZLIBINC) \
 	$(BFD_CFLAGS) $(INCLUDE_CFLAGS) $(LIBDECNUMBER_CFLAGS) \
 	$(INTL_CFLAGS) $(INCGNU) $(INCSUPPORT) $(ENABLE_CFLAGS) \
@@ -1712,7 +1716,7 @@ check-headers:
 	@echo Checking headers.
 	for i in $(CHECK_HEADERS) ; do \
 		$(CXX) $(CXX_DIALECT) -x c++-header -c -fsyntax-only \
-		$(INTERNAL_CFLAGS) -include defs.h $(srcdir)/$$i ; \
+		$(INTERNAL_CFLAGS) $(CXXFLAGS) -include defs.h $(srcdir)/$$i ; \
 	done
 .PHONY: check-headers
 
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index 8b80acebdf1e..903c4a855516 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -83,9 +83,12 @@ include $(srcdir)/../gdb/silent-rules.mk
 # GNU make is used.  The overrides implement dependency tracking.
 COMPILE.pre = $(CXX) $(CXX_DIALECT)
 COMPILE.post = -c -o $@
-COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(COMPILE.post)
 POSTCOMPILE = @true
 
+# CXXFLAGS is at the very end on purpose, so that user-supplied flags can
+# override internal flags.
+COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(INTERNAL_CFLAGS) $(CXXFLAGS) $(COMPILE.post)
+
 # It is also possible that you will need to add -I/usr/include/sys to the
 # CFLAGS section if your system doesn't have fcntl.h in /usr/include (which
 # is where it should be according to Posix).
@@ -158,7 +161,7 @@ PTHREAD_LIBS = @PTHREAD_LIBS@
 WIN32APILIBS = @WIN32APILIBS@
 
 # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
-INTERNAL_CFLAGS_BASE = ${CXXFLAGS} ${GLOBAL_CFLAGS} \
+INTERNAL_CFLAGS_BASE = ${GLOBAL_CFLAGS} \
 	${PROFILE_CFLAGS} ${INCLUDE_CFLAGS} ${CPPFLAGS} $(PTHREAD_CFLAGS)
 INTERNAL_WARN_CFLAGS = ${INTERNAL_CFLAGS_BASE} $(WARN_CFLAGS)
 INTERNAL_CFLAGS = ${INTERNAL_WARN_CFLAGS} $(WERROR_CFLAGS) -DGDBSERVER
@@ -352,6 +355,7 @@ gdbserver$(EXEEXT): $(sort $(OBS)) ${CDEPS} $(LIBGNU) $(LIBIBERTY) \
 		$(INTL_DEPS) $(GDBSUPPORT)
 	$(SILENCE) rm -f gdbserver$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
+		$(CXXFLAGS) \
 		-o gdbserver$(EXEEXT) $(OBS) $(GDBSUPPORT) $(LIBGNU) \
 		$(LIBIBERTY) $(INTL) $(GDBSERVER_LIBS) $(XM_CLIBS) \
 		$(WIN32APILIBS)
@@ -360,6 +364,7 @@ gdbreplay$(EXEEXT): $(sort $(GDBREPLAY_OBS)) $(LIBGNU) $(LIBIBERTY) \
 		$(INTL_DEPS) $(GDBSUPPORT)
 	$(SILENCE) rm -f gdbreplay$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
+		$(CXXFLAGS) \
 		-o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) $(XM_CLIBS) \
 		$(GDBSUPPORT) $(LIBGNU) $(LIBIBERTY) $(INTL) \
 		$(WIN32APILIBS)
@@ -387,6 +392,7 @@ $(IPA_LIB): $(sort $(IPA_OBJS)) ${CDEPS}
 	$(SILENCE) rm -f $(IPA_LIB)
 	$(ECHO_CXXLD) $(CC_LD) -shared -fPIC -Wl,--soname=$(IPA_LIB) \
 		-Wl,--no-undefined $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
+		 $(CXXFLAGS) \
 		-o $(IPA_LIB) ${IPA_OBJS} -ldl -pthread
 
 # Put the proper machine-specific files first, so M-. on a machine
@@ -482,7 +488,9 @@ IPAGENT_CFLAGS = $(INTERNAL_CFLAGS) $(UST_CFLAGS) \
 	-fPIC -DIN_PROCESS_AGENT \
 	-fvisibility=hidden
 
-IPAGENT_COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(IPAGENT_CFLAGS) $(COMPILE.post)
+# CXXFLAGS is at the very end on purpose, so that user-supplied flags can
+# override internal flags.
+IPAGENT_COMPILE = $(ECHO_CXX) $(COMPILE.pre) $(IPAGENT_CFLAGS) $(CXXFLAGS) $(COMPILE.post)
 
 # Rules for special cases.
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] gdb: put user-supplied CFLAGS at the end
  2020-10-07 15:27   ` [PATCH v2] " Simon Marchi
@ 2020-10-07 17:34     ` Pedro Alves
  2020-10-07 17:55       ` Simon Marchi
  0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2020-10-07 17:34 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 10/7/20 4:27 PM, Simon Marchi via Gdb-patches wrote:
> New in v2:
> 
> * Put $(CXXFLAGS) in the top-level COMPILE command instead of keeping it
>   buried in INTERNAL_CFLAGS.
> * Do the same change in gdbserver.

Thank you.  This version LGTM.

Pedro Alves

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] gdb: put user-supplied CFLAGS at the end
  2020-10-07 17:34     ` Pedro Alves
@ 2020-10-07 17:55       ` Simon Marchi
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-10-07 17:55 UTC (permalink / raw)
  To: Pedro Alves, Simon Marchi, gdb-patches

On 2020-10-07 1:34 p.m., Pedro Alves wrote:
> On 10/7/20 4:27 PM, Simon Marchi via Gdb-patches wrote:
>> New in v2:
>>
>> * Put $(CXXFLAGS) in the top-level COMPILE command instead of keeping it
>>   buried in INTERNAL_CFLAGS.
>> * Do the same change in gdbserver.
> 
> Thank you.  This version LGTM.
> 
> Pedro Alves
> 

Thanks, I'll push it.

Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-07 17:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-05 16:40 [PATCH] gdb: put user-supplied CFLAGS at the end Simon Marchi
2020-10-07 12:10 ` Pedro Alves
2020-10-07 14:25   ` Simon Marchi
2020-10-07 15:27   ` [PATCH v2] " Simon Marchi
2020-10-07 17:34     ` Pedro Alves
2020-10-07 17:55       ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).