[commit] Build memmem with -Wno-error.

[commit] Build memmem with -Wno-error.

[Joel Brobecker]

See comment explaining why we need to do this. (and also discussed
on IRC with Pedro).

gdb/gdbserver/ChangeLog:

        * Makefile.in (memmem.o): Build with -Wno-error.

Tested on x86_64-linux. Since it was verbally approved by Pedro,
I will commit this sometime today.

---
 gdb/gdbserver/Makefile.in |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index e397bd7..ffec46f 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -347,8 +347,13 @@ gdbreplay.o: gdbreplay.c config.h
 signals.o: ../common/signals.c $(server_h) $(signals_def)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
+# We build memmem.c with -Wno-error because this file is not under
+# our control.  On LynxOS, the compiler generates some warnings
+# because str-two-way.h uses a constant (MAX_SIZE) whose definition
+# makes it ambiguous whether it is signed or unsigned ("warning: this
+# decimal constant is unsigned only in ISO C90").
 memmem.o: ../gnulib/memmem.c
-	$(CC) -o memmem.o -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $<
+	$(CC) -o memmem.o -c $(CPPFLAGS) $(INTERNAL_CFLAGS) -Wno-error $<
 
 i386_low_h = $(srcdir)/i386-low.h
 
-- 
1.7.1

Re: [commit] Build memmem with -Wno-error.

[Pedro Alves]

On Tuesday 31 August 2010 20:08:26, Joel Brobecker wrote:

> +# We build memmem.c with -Wno-error because this file is not under
> +# our control.  On LynxOS, the compiler generates some warnings
> +# because str-two-way.h uses a constant (MAX_SIZE) whose definition
> +# makes it ambiguous whether it is signed or unsigned ("warning: this
> +# decimal constant is unsigned only in ISO C90").
>  memmem.o: ../gnulib/memmem.c
> -	$(CC) -o memmem.o -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $<
> +	$(CC) -o memmem.o -c $(CPPFLAGS) $(INTERNAL_CFLAGS) -Wno-error $<


Sorry, I didn't realize you were talking about "-Wno-error" literally.
This may break non-gcc compilers.  It would be better to come up with
an $(INTERNAL_CFLAGS) variant that does not include -Werror in the first
place, and use that instead in the memmem.o rule.  gdb/Makefile.in uses
INTERNAL_WARN_CFLAGS for exactly that.  I suggest to do the same here.  Take
a look at the monitor.o rule in gdb/Makefile.in.

-- 
Pedro Alves

Re: [commit] Build memmem with -Wno-error.

[Jan Kratochvil]

On Tue, 31 Aug 2010 21:48:34 +0200, Pedro Alves wrote:
> It would be better to come up with
> an $(INTERNAL_CFLAGS) variant that does not include -Werror in the first
> place, and use that instead in the memmem.o rule.  gdb/Makefile.in uses
> INTERNAL_WARN_CFLAGS for exactly that.  I suggest to do the same here.  Take
> a look at the monitor.o rule in gdb/Makefile.in.

This is a longterm cosmetic problem.  Anytime I modify anything in defs.h VIM
jumps to the warnings in monitor.c which I have to skip.

There should be the explicit -Wno-format-nonliteral form.  Just removing
-Wformat-nonliteral or -Werror is not enough.

Combined the patch together with making it more restrictive.  It should no
longer be IMO such a concern since Joel started removing -Werror for releases.

Tested compilation on x86_64-fedora14snapshot-linux-gnu.


Thanks,
Jan


gdb/
2010-08-31  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* Makefile.in (GDB_WARN_CFLAGS_NO_FORMAT): Convert it to the no- form.
	(monitor.o): Replace $(INTERNAL_WARN_CFLAGS) by $(INTERNAL_CFLAGS) and
	add $(GDB_WARN_CFLAGS_NO_FORMAT).
	(printcmd.o): Replace $(INTERNAL_CFLAGS_BASE) by $(INTERNAL_CFLAGS).

--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -155,7 +155,8 @@ WERROR_CFLAGS = @WERROR_CFLAGS@
 GDB_WARN_CFLAGS = $(WARN_CFLAGS)
 GDB_WERROR_CFLAGS = $(WERROR_CFLAGS)
 
-GDB_WARN_CFLAGS_NO_FORMAT = `echo " $(GDB_WARN_CFLAGS) " | sed "s/ -Wformat-nonliteral / /g"`
+GDB_WARN_CFLAGS_NO_FORMAT = `echo " $(GDB_WARN_CFLAGS) " \
+		   | sed "s/ -Wformat-nonliteral / -Wno-format-nonliteral /g"`
 
 RDYNAMIC = @RDYNAMIC@
 
@@ -1531,14 +1532,15 @@ main.o: $(srcdir)/main.c
 # definitly will not work.  "monitor.c" needs to be rewritten so that
 # it doesn't use format strings and instead uses callbacks.
 monitor.o: $(srcdir)/monitor.c
-	$(COMPILE.pre) $(INTERNAL_WARN_CFLAGS) $(COMPILE.post) $(srcdir)/monitor.c
+	$(COMPILE.pre) $(INTERNAL_CFLAGS) $(GDB_WARN_CFLAGS_NO_FORMAT) \
+		$(COMPILE.post) $(srcdir)/monitor.c
 	$(POSTCOMPILE)
 
 # Do not try to build "printcmd.c" with -Wformat-nonliteral.  It manually
 # checks format strings.
 printcmd.o: $(srcdir)/printcmd.c
-	$(COMPILE.pre) $(INTERNAL_CFLAGS_BASE) $(GDB_WARN_CFLAGS_NO_FORMAT) \
-		$(GDB_WERROR_CFLAGS) $(COMPILE.post) $(srcdir)/printcmd.c
+	$(COMPILE.pre) $(INTERNAL_CFLAGS) $(GDB_WARN_CFLAGS_NO_FORMAT) \
+		$(COMPILE.post) $(srcdir)/printcmd.c
 	$(POSTCOMPILE)
 
 # Message files.  Based on code in gcc/Makefile.in.

Re: [commit] Build memmem with -Wno-error.

[Pedro Alves]

On Tuesday 31 August 2010 21:14:55, Jan Kratochvil wrote:
> On Tue, 31 Aug 2010 21:48:34 +0200, Pedro Alves wrote:
> > It would be better to come up with
> > an $(INTERNAL_CFLAGS) variant that does not include -Werror in the first
> > place, and use that instead in the memmem.o rule.  gdb/Makefile.in uses
> > INTERNAL_WARN_CFLAGS for exactly that.  I suggest to do the same here.  Take
> > a look at the monitor.o rule in gdb/Makefile.in.
> 
> This is a longterm cosmetic problem.  Anytime I modify anything in defs.h VIM
> jumps to the warnings in monitor.c which I have to skip.
> 
> There should be the explicit -Wno-format-nonliteral form.  Just removing
> -Wformat-nonliteral or -Werror is not enough.
> 
> Combined the patch together with making it more restrictive.  It should no
> longer be IMO such a concern since Joel started removing -Werror for releases.
> 
> Tested compilation on x86_64-fedora14snapshot-linux-gnu.

I'm not sure whether this may break the build on older gcc's, but
given --disable-werror, it's fine with me to give this a try.

-- 
Pedro Alves

Re: [commit] Build memmem with -Wno-error.

[Joel Brobecker]

> I'm not sure whether this may break the build on older gcc's, but
> given --disable-werror, it's fine with me to give this a try.

I had a look at the documentation for older versions of GCC, up to
version 2.95.3, and all manuals say that for every -Wsomething, there
is the counter-part Wno-something. Unless there was an omission in
the implementation, I think that the sed adjustment should be fine.

-- 
Joel

Re: [commit] Build memmem with -Wno-error.

[Pedro Alves]

On Tuesday 31 August 2010 22:22:23, Joel Brobecker wrote:
> > I'm not sure whether this may break the build on older gcc's, but
> > given --disable-werror, it's fine with me to give this a try.
> 
> I had a look at the documentation for older versions of GCC, up to
> version 2.95.3, and all manuals say that for every -Wsomething, there
> is the counter-part Wno-something. Unless there was an omission in
> the implementation, I think that the sed adjustment should be fine.

Thanks.  I was more concerned with the the fact that the file after
the patch is build with -Werror, and that that particular -W flag may
not exist in such order versions, and/or another flag is necessary to
disable the warning in question.  One would assume not though, hence
me being fine with this until proven otherwise.  :-)

-- 
Pedro Alves

Re: [commit] Build memmem with -Wno-error.

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

> Sorry, I didn't realize you were talking about "-Wno-error" literally.
> This may break non-gcc compilers.  It would be better to come up with
> an $(INTERNAL_CFLAGS) variant that does not include -Werror in the first
> place, and use that instead in the memmem.o rule.  gdb/Makefile.in uses
> INTERNAL_WARN_CFLAGS for exactly that.  I suggest to do the same here.  Take
> a look at the monitor.o rule in gdb/Makefile.in.

Duh - rookie mistake :-(.

Here is a new version that separates the warning flags, -Werror flags
and the rest into 3 separate variables...

Retested on x86_64-linux.

-- 
Joel

[-- Attachment #2: gdbserver-memmem.diff --]
[-- Type: text/x-diff, Size: 1923 bytes --]

commit c16350d2bf3a0fcd4fc6f746837d381c3268eaeb
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Tue Aug 31 11:17:39 2010 -0400

    Compile memmem.o without -Werror.
    
    This reproduces the same approach as in GDB to allow us to build
    specific files without -Werror.
    
    gdb/gdbserver/ChangeLog:
    
            * Makefile.in (INTERNAL_CFLAGS_BASE): New variable. Extracted
            from INTERNAL_CFLAGS.
            (INTERNAL_WARN_CFLAGS): New variable.
            (INTERNAL_CFLAGS): Adjust, using INTERNAL_WARN_CFLAGS.

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index e397bd7..ffbb14a 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -94,8 +94,10 @@ WERROR_CFLAGS = @WERROR_CFLAGS@
 CFLAGS = @CFLAGS@
 
 # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
-INTERNAL_CFLAGS =  $(WARN_CFLAGS) $(WERROR_CFLAGS) ${CFLAGS} ${GLOBAL_CFLAGS} \
+INTERNAL_CFLAGS_BASE =  ${CFLAGS} ${GLOBAL_CFLAGS} \
 	${PROFILE_CFLAGS} ${INCLUDE_CFLAGS}
+INTERNAL_WARN_CFLAGS =  ${INTERNAL_CFLAGS_BASE} $(WARN_CFLAGS)
+INTERNAL_CFLAGS =  ${INTERNAL_WARN_CFLAGS} $(WERROR_CFLAGS)
 
 # LDFLAGS is specifically reserved for setting from the command line
 # when running make.
@@ -347,8 +349,13 @@ gdbreplay.o: gdbreplay.c config.h
 signals.o: ../common/signals.c $(server_h) $(signals_def)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
 
+# We build memmem.c without -Werror because this file is not under
+# our control.  On LynxOS, the compiler generates some warnings
+# because str-two-way.h uses a constant (MAX_SIZE) whose definition
+# makes it ambiguous whether it is signed or unsigned ("warning: this
+# decimal constant is unsigned only in ISO C90").
 memmem.o: ../gnulib/memmem.c
-	$(CC) -o memmem.o -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $<
+	$(CC) -o memmem.o -c $(CPPFLAGS) $(INTERNAL_WARN_CFLAGS) $<
 
 i386_low_h = $(srcdir)/i386-low.h
 

Re: [commit] Build memmem with -Wno-error.

[Joel Brobecker]

> Thanks.  I was more concerned with the the fact that the file after
> the patch is build with -Werror, and that that particular -W flag may
> not exist in such order versions, and/or another flag is necessary to
> disable the warning in question.  One would assume not though, hence
> me being fine with this until proven otherwise.  :-)

As far as I could tell, there is some configury to avoid the use
of that flag if the compiler does not support it.

    build_warnings="-Wall -Wdeclaration-after-statement -Wpointer-arith [...]"
    for w in ${build_warnings}; do
        case $w in
        -Werr*) WERROR_CFLAGS=-Werror ;;
        *) # Check that GCC accepts it
            saved_CFLAGS="$CFLAGS"
            CFLAGS="$CFLAGS $w"
            AC_TRY_COMPILE([],[],WARN_CFLAGS="${WARN_CFLAGS} $w",)
            CFLAGS="$saved_CFLAGS"
        esac
    done

?

-- 
Joel

Re: [commit] Build memmem with -Wno-error.

[Jan Kratochvil]

On Wed, 01 Sep 2010 00:07:20 +0200, Joel Brobecker wrote:
> As far as I could tell, there is some configury to avoid the use
> of that flag if the compiler does not support it.

Just there could be a GCC historical moment when the message
	warning: format not a string literal, argument types not checked

was already produced while neither -Wformat-nonliteral nor
-Wno-format-nonliteral was yet supported.

The change enabling -Werror in such case would break it.  But let's pretend
-Wformat-nonliteral + -Wno-format-nonliteral were introduced at the moment
	warning: format not a string literal, argument types not checked

was introduced.

(I will check it in later.)


Thanks,
Jan

Re: [commit] Build memmem with -Wno-error.

[Pedro Alves]

On Tuesday 31 August 2010 23:13:14, Jan Kratochvil wrote:
> On Wed, 01 Sep 2010 00:07:20 +0200, Joel Brobecker wrote:
> > As far as I could tell, there is some configury to avoid the use
> > of that flag if the compiler does not support it.
> 
> Just there could be a GCC historical moment when the message
> 	warning: format not a string literal, argument types not checked
> 
> was already produced while neither -Wformat-nonliteral nor
> -Wno-format-nonliteral was yet supported.
> 
> The change enabling -Werror in such case would break it.  But let's pretend
> -Wformat-nonliteral + -Wno-format-nonliteral were introduced at the moment
> 	warning: format not a string literal, argument types not checked
> 
> was introduced.

Exactly.

> (I will check it in later.)

-- 
Pedro Alves

Re: [commit] Build memmem with -Wno-error.

[Pedro Alves]

On Tuesday 31 August 2010 23:03:58, Joel Brobecker wrote:

> Here is a new version that separates the warning flags, -Werror flags
> and the rest into 3 separate variables...

Looks great, thanks.

-- 
Pedro Alves

Re: [commit] Build memmem with -Wno-error.

[Joel Brobecker]

> > Here is a new version that separates the warning flags, -Werror flags
> > and the rest into 3 separate variables...
> 
> Looks great, thanks.

Thanks! Now checked in.

-- 
Joel

Re: [commit] Build memmem with -Wno-error.

[Jan Kratochvil]

On Tue, 31 Aug 2010 22:34:16 +0200, Pedro Alves wrote:
> On Tuesday 31 August 2010 21:14:55, Jan Kratochvil wrote:
> > This is a longterm cosmetic problem.  Anytime I modify anything in defs.h VIM
> > jumps to the warnings in monitor.c which I have to skip.
> > 
> > There should be the explicit -Wno-format-nonliteral form.  Just removing
> > -Wformat-nonliteral or -Werror is not enough.
[...]
> 
> I'm not sure whether this may break the build on older gcc's, but
> given --disable-werror, it's fine with me to give this a try.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2010-09/msg00017.html


Thanks,
Jan