gl_MANYWARN_ALL_GCC() leads to many spurious warnings

gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Samuel Bronson]

It seems that gl_MANYWARN_ALL_GCC is intended to enable (virtually)  
all warnings implemented by GCC, no matter how noisy or badly tested  
they are in whatever GCC version is in use.

The result is that, even after we explicitly disable dozens of them  
again, we still generally get a lot of spurious warnings when building  
with --enable-gcc-warnings.   Furthermore, since that option also  
enables -Werror, Emacs generally does not build at all in this  
configuration (unless you monkey around with WERROR_CFLAGS).

Why not simply use "-W -Wall -Wextra" and perhaps a few others, plus a  
few overrides to turn off the warnings those enable that aren't useful  
with Emacs?  That way, we'd only get warnings that had been reasonably  
well tested.

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Paul Eggert]

On 07/06/2012 02:17 PM, Samuel Bronson wrote:

> Why not simply use "-W -Wall -Wextra" and perhaps a few others, plus
> a few overrides to turn off the warnings those enable that aren't
> useful with Emacs?  That way, we'd only get warnings that had been
> reasonably well tested.

Something like that might well be useful, but it's a different option
than what's intended here.  --enable-gcc-warnings is intended for
developers, and it tries to take advantage of the latest and greatest
GCC features.  If it enabled only flags that work well on all GCC
versions, the static checking would be of considerably lower quality.

It might be useful to have another configure-time option that would
enable just the warnings that work fairly well back to GCC version
something-or-another, if someone wanted to maintain that.  It might be
a maintenance hassle, though, as there are lots of old and buggy GCCs
out there.

By the way, --enable-gcc-warnings does not actually enable all
warnings, only the warnings that work well with the latest GCC on
platforms that it's tested on.

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Samuel Bronson]

On Jul 6, 2012, at 5:41 PM, Paul Eggert wrote:

> On 07/06/2012 02:17 PM, Samuel Bronson wrote:
>
>> Why not simply use "-W -Wall -Wextra" and perhaps a few others, plus
>> a few overrides to turn off the warnings those enable that aren't
>> useful with Emacs?  That way, we'd only get warnings that had been
>> reasonably well tested.
>
> Something like that might well be useful, but it's a different option
> than what's intended here.  --enable-gcc-warnings is intended for
> developers, and it tries to take advantage of the latest and greatest
> GCC features.  If it enabled only flags that work well on all GCC
> versions, the static checking would be of considerably lower quality.

So, why does it use flags like -Wunreachable-code (which was so buggy  
that it eventually got yanked -- it's now silently ignored) and -Wsync- 
nand (which is totally irrelevant, and warns that this warning isn't  
allowed for Objective C code) and -Wunused-macros (which seems rather  
buggy in GCC 4.7.1 -- it doesn't seem to count appearing in an #ifdef  
as a use?).

> It might be useful to have another configure-time option that would
> enable just the warnings that work fairly well back to GCC version
> something-or-another, if someone wanted to maintain that.  It might be
> a maintenance hassle, though, as there are lots of old and buggy GCCs
> out there.

Might make sense for this version, though:
powerpc-apple-darwin9-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5577)

(That seems to be the last version Apple released for use on PowerPC  
Macs.)  Old, yes.  Buggy, yes.  But it has its advantages...

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Paul Eggert]

On 07/06/2012 03:38 PM, Samuel Bronson wrote:

> why does it use flags like -Wunreachable-code (which was so buggy
> that it eventually got yanked -- it's now silently ignored)

The big picture is that --enable-gcc-warnings hasn't been
ironed out --with-ns.  This can be fixed, and I expect it
will find a few real bugs along with many things that are
just nit cleanups, but it'll take some work.  Is this
something you'd like to help out with?  It could be combined
with the idea of maintaining another configure-time option
useful for finding bugs on Apple.

I started on this task, checking in the results in trunk bzr
108941, but there's lots more where that came from and to be
honest I don't understand the --with-ns code that well.  I
did see some unused-macro warnings but they seemed correct
to me -- perhaps I am missing something?  Anyway, you can
look at the diffs in 108941 to see how to go about this,
and/or to let me know what I did wrong.

Anyway, to get back to your question, I suspect
-Wunreachable-code was there because it is harmless with the
latest GCC, so nobody bothered to remove it.  Since this
flag doesn't ever help I removed it in trunk bzr 108941.

> and -Wsync-nand (which is totally irrelevant, and warns that this
> warning isn't allowed for Objective C code)

Same thing -- this flag never bothered anybody in the
--without-ns world.  Since it never helps either, I removed
it too.

> and -Wunused-macros (which seems rather buggy in GCC 4.7.1 -- it
> doesn't seem to count appearing in an #ifdef as a use?).

I don't observe a bug here.  The following program compiles
just fine with GCC 4.7.1, with -Wunused-macros.  (This is on
Fedora 15 x86-64.)

  #define FOO bar
  #ifdef FOO
  int main (void) { return 0; }
  #endif

What bug do you see?

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Samuel Bronson]

On Jul 7, 2012, at 3:53 PM, Paul Eggert wrote:

> On 07/06/2012 03:38 PM, Samuel Bronson wrote:
>
>> why does it use flags like -Wunreachable-code (which was so buggy
>> that it eventually got yanked -- it's now silently ignored)
>
> The big picture is that --enable-gcc-warnings hasn't been
> ironed out --with-ns.  This can be fixed, and I expect it
> will find a few real bugs along with many things that are
> just nit cleanups, but it'll take some work.  Is this
> something you'd like to help out with?  It could be combined
> with the idea of maintaining another configure-time option
> useful for finding bugs on Apple.

Yeah, I've actually started trying to get rid of a lot of warnings in  
my own tree already; I've been waiting on my copyright assignment  
papers before actually submitting.

At this point I don't think the warnings will turn up much in the way  
of actual bugs; it is turning up a fair amount of seemingly redundant  
code, though.

I did find one in GCC 4.7.1, though; it gives warnings like this:

/Users/user/hacking/emacs/src/nsterm.m: In function '-[EmacsView  
markedRange]':
/Users/user/hacking/emacs/src/nsterm.m:5041:5: warning: format not a  
string literal and no format arguments [-Wformat-security]

for calls that most assuredly do use string literals:

- (NSRange)markedRange
{
   NSRange rng = workingText != nil
     ? NSMakeRange (0, [workingText length]) : NSMakeRange  
(NSNotFound, 0);
   if (NS_KEYLOG)
     NSLog (@"markedRange request");
   return rng;
}

given this declaration (from NSObjCRuntime.h):

FOUNDATION_EXPORT void NSLog(NSString *format, ...)  
__attribute__((format(__NSString__, 1, 2)));

> I started on this task, checking in the results in trunk bzr
> 108941, but there's lots more where that came from and to be
> honest I don't understand the --with-ns code that well.  I
> did see some unused-macro warnings but they seemed correct
> to me -- perhaps I am missing something?  Anyway, you can
> look at the diffs in 108941 to see how to go about this,
> and/or to let me know what I did wrong.
>
> Anyway, to get back to your question, I suspect
> -Wunreachable-code was there because it is harmless with the
> latest GCC, so nobody bothered to remove it.  Since this
> flag doesn't ever help I removed it in trunk bzr 108941.
>
>> and -Wsync-nand (which is totally irrelevant, and warns that this
>> warning isn't allowed for Objective C code)
>
> Same thing -- this flag never bothered anybody in the
> --without-ns world.  Since it never helps either, I removed
> it too.

Fair enough, I guess.

>> and -Wunused-macros (which seems rather buggy in GCC 4.7.1 -- it
>> doesn't seem to count appearing in an #ifdef as a use?).
>
> I don't observe a bug here.  The following program compiles
> just fine with GCC 4.7.1, with -Wunused-macros.  (This is on
> Fedora 15 x86-64.)
>
>  #define FOO bar
>  #ifdef FOO
>  int main (void) { return 0; }
>  #endif
>
> What bug do you see?

Well, okay, it doesn't seem *quite* buggy in GCC 4.7.1 -- it just  
seems to solve the wrong problem, based on what it does with regex.c  
when building in lib-src:

/sw/bin/ccache /sw/bin/gcc-fsf-4.7 -std=gnu99 -c   -Wall -W -Wformat- 
y2k -Wformat-security -Winit-self -Wmissing-include-dirs -Wunused - 
Wunknown-pragmas -Wstrict-aliasing -Wdeclaration-after-statement - 
Wunsafe-loop-optimizations -Wpointer-arith -Wbad-function-cast -Wcast- 
align -Wwrite-strings -Wstrict-prototypes -Wold-style-definition - 
Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn - 
Wmissing-format-attribute -Wpacked -Wunreachable-code -Winvalid-pch - 
Wvolatile-register-var -Wdisabled-optimization -Wbuiltin-macro- 
redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand -Wattributes - 
Wcoverage-mismatch -Wunused-macros -Wabi -Wcpp -Wdeprecated - 
Wdeprecated-declarations -Wdiv-by-zero -Wdouble-promotion -Wendif- 
labels -Wextra -Wformat-contains-nul -Wformat-extra-args -Wformat-zero- 
length -Wformat=2 -Wmultichar -Wnormalized=nfc -Woverflow -Wpointer-to- 
int-cast -Wpragmas -Wsuggest-attribute=noreturn -Wtrampolines -Wno- 
missing-field-initializers -Wno-sign-compare -Wno-type-limits -Wno- 
switch -Wno-unused-parameter -Wno-format-nonliteral -Wno-logical-op - 
fdiagnostics-show-option -funit-at-a-time  -I. -I../src -I../lib -I/ 
Users/user/hacking/emacs/lib-src -I/Users/user/hacking/emacs/lib- 
src/../src -I/Users/user/hacking/emacs/lib-src/../lib  -DXASSERTS=1 -g  
-O2 -DCONFIG_BROKETS -DINHIBIT_STRING_HEADER \
	  /Users/user/hacking/emacs/lib-src/../src/regex.c
/Users/user/hacking/emacs/lib-src/../src/regex.c:805:0: warning: macro  
"CHARSET_RANGE_TABLE_BITS" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:268:0: warning: macro  
"CHAR_HEAD_P" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:269:0: warning: macro  
"SINGLE_BYTE_CHAR_P" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:1880:0: warning:  
macro "EXTEND_RANGE_TABLE" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:1890:0: warning:  
macro "SET_RANGE_TABLE_WORK_AREA_BIT" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:270:0: warning: macro  
"SAME_CHARSET_P" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:285:0: warning: macro  
"BYTE8_TO_CHAR" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:284:0: warning: macro  
"MAKE_CHAR" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:263:0: warning: macro  
"CHARSET_LEADING_CODE_BASE" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:262:0: warning: macro  
"CHAR_CHARSET" is not used [-Wunused-macros]
/Users/user/hacking/emacs/lib-src/../src/regex.c:1903:0: warning:  
macro "SET_RANGE_TABLE_WORK_AREA" is not used [-Wunused-macros]

(It seems that these macros are only used #ifdef emacs)

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Paul Eggert]

On 07/07/2012 05:41 PM, Samuel Bronson wrote:
> /Users/user/hacking/emacs/src/nsterm.m:5041:5: warning: format not a string literal and no format arguments [-Wformat-security]

I don't grok ObjC well enough to help out much, but it is
possible to disable a bogus warning like that selectively,
as in the following example.

> /Users/user/hacking/emacs/lib-src/../src/regex.c:805:0: warning: macro "CHARSET_RANGE_TABLE_BITS" is not used [-Wunused-macros] 

That warning should be suppresed by the following line in
regex.c:

> #  pragma GCC diagnostic ignored "-Wunused-macros"

Could you investigate why that isn't working for you?
It works for me (GCC 4.7.1).

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Samuel Bronson]

On Jul 8, 2012, at 3:32 AM, Paul Eggert wrote:

> On 07/07/2012 05:41 PM, Samuel Bronson wrote:
>> /Users/user/hacking/emacs/src/nsterm.m:5041:5: warning: format not  
>> a string literal and no format arguments [-Wformat-security]
>
> I don't grok ObjC well enough to help out much, but it is
> possible to disable a bogus warning like that selectively,
> as in the following example.

Well, the manual has this to say:

      For Objective-C dialects, `NSString' (or `__NSString__') is
      recognized in the same context.  Declarations including these
      format attributes will be parsed for correct syntax, however the
      result of checking of such format strings is not yet defined, and
      will not be carried out by this version of the compiler.

This leads me to think that GCC should probably just give one warning  
per file (or called function?) that this value for the attribute isn't  
yet implemented, and that these warnings should probably never trigger  
-Werror...

>> /Users/user/hacking/emacs/lib-src/../src/regex.c:805:0: warning:  
>> macro "CHARSET_RANGE_TABLE_BITS" is not used [-Wunused-macros]
>
> That warning should be suppresed by the following line in
> regex.c:
>
>> #  pragma GCC diagnostic ignored "-Wunused-macros"
>
> Could you investigate why that isn't working for you?
> It works for me (GCC 4.7.1).

I don't really know how to go about investigating that, short of  
building GCC manually (preferably with far less stages of  
bootstrapping) and running it under GDB.  Any ideas?

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Paul Eggert]

On 07/08/2012 07:10 AM, Samuel Bronson wrote:

> I don't really know how to go about investigating that, short of building GCC manually

I wasn't thinking of anything that drastic -- just
compile with gcc -E instead of gcc -c and look
to see what happened to the pragmas, something like that.

I just now tried with stock Ubuntu 12.04, i.e.,
gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, with
its bundled GNUstep libraries, and did not observe
either the regex.c -Wunused-macros warnings or the
-Wformat-security warnings.  So this is not merely
a GCC 4.7.1 issue, for whatever that's worth.  I
did see a passle of other warnings.  Perhaps the
warnings that you're observing come up only after
you fix the ones I saw?

I do want to focus on GCC 4.7.1, though.  In my experience
older GCCs are rife with bugs in this area, and it's not
worthwhile to contort code to avoid them.

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Samuel Bronson]

On Jul 8, 2012, at 10:15 PM, Paul Eggert wrote:

> On 07/08/2012 07:10 AM, Samuel Bronson wrote:
>
>> I don't really know how to go about investigating that, short of  
>> building GCC manually
>
> I wasn't thinking of anything that drastic -- just
> compile with gcc -E instead of gcc -c and look
> to see what happened to the pragmas, something like that.
>
> I just now tried with stock Ubuntu 12.04, i.e.,
> gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, with
> its bundled GNUstep libraries, and did not observe
> either the regex.c -Wunused-macros warnings or the
> -Wformat-security warnings.  So this is not merely
> a GCC 4.7.1 issue, for whatever that's worth.  I
> did see a passle of other warnings.  Perhaps the
> warnings that you're observing come up only after
> you fix the ones I saw?

Hmm.  I'm beginning to suspect that the regex.c warnings are somehow  
related to my use of ccache...

> I do want to focus on GCC 4.7.1, though.  In my experience
> older GCCs are rife with bugs in this area, and it's not
> worthwhile to contort code to avoid them.

Yes, of course; code should generally only be changed to accommodate  
such shortcomings when it doesn't involve gymnastics and doesn't  
create places for actual bugs to lurk.

Re: gl_MANYWARN_ALL_GCC() leads to many spurious warnings

[Samuel Bronson]

On Jul 9, 2012, at 11:23 AM, Samuel Bronson wrote:
> On Jul 8, 2012, at 10:15 PM, Paul Eggert wrote:
>
>> On 07/08/2012 07:10 AM, Samuel Bronson wrote:
>>
>>> I don't really know how to go about investigating that, short of  
>>> building GCC manually
>>
>> I wasn't thinking of anything that drastic -- just
>> compile with gcc -E instead of gcc -c and look
>> to see what happened to the pragmas, something like that.
>>
>> I just now tried with stock Ubuntu 12.04, i.e.,
>> gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, with
>> its bundled GNUstep libraries, and did not observe
>> either the regex.c -Wunused-macros warnings or the
>> -Wformat-security warnings.  So this is not merely
>> a GCC 4.7.1 issue, for whatever that's worth.  I
>> did see a passle of other warnings.  Perhaps the
>> warnings that you're observing come up only after
>> you fix the ones I saw?
>
> Hmm.  I'm beginning to suspect that the regex.c warnings are somehow  
> related to my use of ccache...

Well, I was right; it was related to my use of ccache.  But it's still  
a GCC bug; gcc -E isn't honoring #pragma diagnostic ignored "-Wunused- 
macros", and since ccache defaults to running the preprocessor first,  
then compiling its output (so it can skip the compilation if it  
already has results cached for that preprocessor output), this causes  
ccache to output the bogus warnings.  I reported this as <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53920 
 >.

Presumably, the reason that you don't get the format warnings with  
GNUstep because, since only Apple's compilers have full support for  
__attribute__((format(NSString,...)), GNUstep probably doesn't use  
this attribute in its prototype for NSLog.  I submitted a bug report  
about them at <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53905>.