Add macros for diagnostic control, use them in locale/weightwc.h

Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

In <https://sourceware.org/ml/libc-alpha/2014-11/msg00326.html>,
Roland requested internal macros for use of "#pragma GCC diagnostic".

This patch adds such macros and uses them to disable
-Wmaybe-uninitialized warnings for some code in locale/weightwc.h.  I
put a GCC version in the arguments to DIAG_IGNORE, as that seems
something useful to grep for when obsoleting support for an old GCC
version and needing to decide if warning-disabling code is still
relevant.

The use in weightwc.h has a __GNUC_PREREQ (4, 7) conditional because
4.6 doesn't have -Wmaybe-uninitialized (split out of -Wuninitialized
for 4.7).  I didn't include a #else case to disable -Wuninitialized
for 4.6 because I don't know if 4.6 produces these warnings (but if
someone does see them with 4.6, such a #else case should be added -
with 4.6 the version number in the DIAG_IGNORE call).

These macros should be usable for replacing existing -Wno-* use in
makefiles (as also suggested by Roland), though I have no plans to
work on that (only on use of the macros in cases where warnings are
currently present that need disabling to use -Werror).

Tested for x86_64 that installed stripped shared libraries are
unchanged by this patch.

2014-11-18  Joseph Myers  <joseph@codesourcery.com>

	* include/libc-internal.h (DIAG_PUSH): New macro.
	(DIAG_POP): Likewise.
	(_DIAG_STR1): Likewise.
	(_DIAG_STR): Likewise.
	(DIAG_IGNORE): Likewise.
	* locale/weightwc.h: Include <libc-internal.h>.
	(findidx): Disable -Wmaybe-uninitialized around some dereferences
	of CP.

diff --git a/include/libc-internal.h b/include/libc-internal.h
index 78f82da..0be89e6 100644
--- a/include/libc-internal.h
+++ b/include/libc-internal.h
@@ -70,4 +70,26 @@ extern void __init_misc (int, char **, char **);
 #define PTR_ALIGN_UP(base, size) \
   ((__typeof__ (base)) ALIGN_UP ((uintptr_t) (base), (size)))
 
+/* Push diagnostic state.  */
+#define DIAG_PUSH _Pragma ("GCC diagnostic push")
+
+/* Pop diagnostic state.  */
+#define DIAG_POP _Pragma ("GCC diagnostic pop")
+
+#define _DIAG_STR1(s) #s
+#define _DIAG_STR(s) _DIAG_STR1(s)
+
+/* Ignore the diagnostic OPTION.  VERSION is the most recent GCC
+   version for which the diagnostic has been confirmed to appear in
+   the absence of the pragma (in the form MAJOR.MINOR for GCC 4.x,
+   just MAJOR for GCC 5 and later).  Uses of this pragma should be
+   reviewed when the GCC version given is no longer supported for
+   building glibc.  Uses should come with a comment giving more
+   details of the diagnostic, and an architecture on which it is seen
+   if possibly optimization-related and not in architecture-specific
+   code.  This macro should only be used if the diagnostic seems hard
+   to fix (for example, optimization-related false positives).  */
+#define DIAG_IGNORE(option, version) \
+  _Pragma (_DIAG_STR (GCC diagnostic ignored _DIAG_STR (option)))
+
 #endif /* _LIBC_INTERNAL  */
diff --git a/locale/weightwc.h b/locale/weightwc.h
index 0f70b00..d79bd21 100644
--- a/locale/weightwc.h
+++ b/locale/weightwc.h
@@ -19,6 +19,8 @@
 #ifndef _WEIGHTWC_H_
 #define _WEIGHTWC_H_	1
 
+#include <libc-internal.h>
+
 /* Find index of weight.  */
 static inline int32_t __attribute__ ((always_inline))
 findidx (const int32_t *table,
@@ -90,6 +92,16 @@ findidx (const int32_t *table,
 	      continue;
 	    }
 
+	  /* Seen on x86_64 (inlined from
+	     fnmatch_loop.c:internal_fnwmatch): "'*((void *)&str+4)'
+	     may be used uninitialized in this function" (the
+	     diagnostic can apply to multiple dereferences of CP, not
+	     just one, so all affected dereferences need to be between
+	     DIAG_PUSH and DIAG_POP).  */
+	  DIAG_PUSH;
+#if __GNUC_PREREQ (4, 7)
+	  DIAG_IGNORE (-Wmaybe-uninitialized, 4.9);
+#endif
 	  if (cp[nhere - 1] > usrc[nhere -1])
 	    {
 	      cp += 2 * nhere;
@@ -105,6 +117,7 @@ findidx (const int32_t *table,
 	  /* This range matches the next characters.  Now find
 	     the offset in the indirect table.  */
 	  offset = usrc[nhere - 1] - cp[nhere - 1];
+	  DIAG_POP;
 	  *cpp += nhere;
 
 	  return indirect[-i + offset];

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

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

On 11/18/2014 10:04 AM, Joseph Myers wrote:
> +	  DIAG_PUSH;
> +#if __GNUC_PREREQ (4, 7)
> +	  DIAG_IGNORE (-Wmaybe-uninitialized, 4.9);
> +#endif
>   	  if (cp[nhere - 1] > usrc[nhere -1])
>   	    {
>   	      cp += 2 * nhere;
> @@ -105,6 +117,7 @@ findidx (const int32_t *table,
>   	  /* This range matches the next characters.  Now find
>   	     the offset in the indirect table.  */
>   	  offset = usrc[nhere - 1] - cp[nhere - 1];
> +	  DIAG_POP;

My kneejerk reaction is that attempting to minimize the scope of 
disabled warnings will make code harder to read and maintain, and the 
costs of minimization may outweigh the benefits.

Also, the distinction between GCC version "4, 7" (which is checked) and 
version "4.9" (which is not) is bothersome. Inevitably the unchecked 
version numbers will become wrong.  If we're going to have a "4.9" at 
all, we should check it.

How about the attached (untested) patch instead?  This is simpler and 
less intrusive and should scale better.  It disables the diagnostic in 
some places that it doesn't absolutely need to, but it separates 
concerns better and overall I expect it would have a better 
cost-to-benefit ratio.

[-- Attachment #2: Wuninitialized.patch --]
[-- Type: text/x-patch, Size: 1056 bytes --]

diff --git a/ChangeLog b/ChangeLog
index 37b1459..ab9dc88 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2014-11-18  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* locale/weightwc.h (findidx): Disable -Wmaybe-uninitialized.
+
 2014-11-18  Roland McGrath  <roland@hack.frob.com>
 
 	* nptl/createthread.c: New file.
diff --git a/locale/weightwc.h b/locale/weightwc.h
index 0f70b00..383d34e 100644
--- a/locale/weightwc.h
+++ b/locale/weightwc.h
@@ -19,6 +19,14 @@
 #ifndef _WEIGHTWC_H_
 #define _WEIGHTWC_H_	1
 
+#pragma GCC diagnostic push
+
+/* Seen on x86_64 (inlined from fnmatch_loop.c:internal_fnwmatch):
+   "'*((void *)&str+4)' may be used uninitialized in this function".  */
+#if __GNUC_PREREQ (4, 7) && !__GNUC_PREPREQ (4, 10)
+# pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif
+
 /* Find index of weight.  */
 static inline int32_t __attribute__ ((always_inline))
 findidx (const int32_t *table,
@@ -115,4 +123,6 @@ findidx (const int32_t *table,
   return 0x43219876;
 }
 
+#pragma GCC diagnostic pop
+
 #endif	/* weightwc.h */

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Roland McGrath]

> My kneejerk reaction is that attempting to minimize the scope of 
> disabled warnings will make code harder to read and maintain, and the 
> costs of minimization may outweigh the benefits.

I strongly disagree.  We should only ever disable warnings with the
tightest possible scope around where we know for sure we need to.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

On 11/18/2014 01:18 PM, Roland McGrath wrote:
> We should only ever disable warnings with the
> tightest possible scope around where we know for sure we need to.

In that case neither my patch nor Joseph's should be accepted.  If I 
understand Joseph's patch correctly,the maybe-uninitialized warning 
needs to be disabled only when dereferencing'cp'.  For example, in the 
expression 'cp[cnt]', the warning should be disabled only for the load 
from 'cp[cnt]'; it should not be disabled for the load from 'cp' itself, 
or for the load from 'cnt'.  So Joseph's proposed patch doesn't have a 
tight scope here.  It should be possible to tighten itto cover only the 
places where troublesome dereferences actually occur, e.g., by defining 
a macro 'NO_MAYBE_UNINITIALIZED_DEREFERENCE (p)' that suppresses the 
warning while dereferencing the pointer P, by using 
'NO_MAYBE_UNINITIALIZED_DEREFERENCE (cp + cnt)' in place of 'cp[cnt]', etc.

Also, as I understand it the warning should be disabled only on x86-64, 
and only if optimization is enabled, as we don't know of problems with 
other platforms or when optimization is turned off.

There may be other ways to tighten the scope, too.

However, I'm still not convinced that it's worth the trouble to insist 
on tight scopes like this.  Many of these bogus warnings are due to 
compiler bugs; I'm afraid we've seen this too often in gnulib, when 
supporting projects that use -Werror.  If we start worrying about tight 
scopes for workarounds for GCC cry-wolf bugs, we will too easily go down 
the rabbit hole of maintaining a fragile and evolving list of GCC 
compiler bugs rather than maintaining glibc.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Tue, 18 Nov 2014, Paul Eggert wrote:

> Also, the distinction between GCC version "4, 7" (which is checked) and
> version "4.9" (which is not) is bothersome. Inevitably the unchecked version
> numbers will become wrong.  If we're going to have a "4.9" at all, we should
> check it.

Checking it means reviewing if the warning still appears when GCC 5 
becomes the minimum supported version for building glibc.  The relevant 
version to compare the 4.9 with is the minimum version for building glibc 
(which is not currently available in C code in glibc), not the version 
actually being used to build glibc.

> +#pragma GCC diagnostic push

No, as stated in 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00326.html> it's desired 
to go via internal macros for this.

> +/* Seen on x86_64 (inlined from fnmatch_loop.c:internal_fnwmatch):
> +   "'*((void *)&str+4)' may be used uninitialized in this function".  */
> +#if __GNUC_PREREQ (4, 7) && !__GNUC_PREPREQ (4, 10)
> +# pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
> +#endif

No, __GNUC_PREPREQ (4, 10) is wrong.  We don't want to introduce lots of 
build failures every time GCC branches and the mainline version increases.  
We have no evidence that the warning does not appear with GCC 5; the 
assertion is simply that it was observed with 4.9.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Tue, 18 Nov 2014, Paul Eggert wrote:

> In that case neither my patch nor Joseph's should be accepted.  If I
> understand Joseph's patch correctly,the maybe-uninitialized warning needs to
> be disabled only when dereferencing'cp'.  For example, in the expression
> 'cp[cnt]', the warning should be disabled only for the load from 'cp[cnt]'; it
> should not be disabled for the load from 'cp' itself, or for the load from
> 'cnt'.  So Joseph's proposed patch doesn't have a tight scope here.  It should
> be possible to tighten itto cover only the places where troublesome
> dereferences actually occur, e.g., by defining a macro
> 'NO_MAYBE_UNINITIALIZED_DEREFERENCE (p)' that suppresses the warning while
> dereferencing the pointer P, by using 'NO_MAYBE_UNINITIALIZED_DEREFERENCE (cp
> + cnt)' in place of 'cp[cnt]', etc.

Since these pragmas work, for middle-end warnings such as these, by 
tracking a range of locations for which a particular diagnostic is handled 
in a particular way, I don't expect such a use in statement expressions 
inside macros to work reliably (and it doesn't work in my tests); the 
smallest granularity I expect to be reliable is that of whole statements 
in source files outside of macros (with the pragma calls on separate 
lines).  (My test:

#define UNINIT(x) \
  ({ \
    __typeof (x) __tmp; \
    _Pragma ("GCC diagnostic push"); \
    _Pragma ("GCC diagnostic ignored \"-Wuninitialized\""); \
    __tmp = (x); \
    _Pragma ("GCC diagnostic pop"); \
    __tmp; \
  })

int
f (void)
{
  int a, b;
  return UNINIT (a) + UNINIT (b);
}

)

> Also, as I understand it the warning should be disabled only on x86-64, and

No, rather, it's known it needs disabling on x86_64, but we don't want to 
cause build failures on all other architectures until their maintainers 
confirm it's needed there.  The reference to x86_64 is simply a hint about 
where to test when GCC 4.9 is no longer a supported compiler for building 
glibc.

> only if optimization is enabled, as we don't know of problems with other
> platforms or when optimization is turned off.

glibc has to be built with optimization.

Disabling for the relevant block of source lines, on all architectures and 
for all GCC versions that support the relevant -W option, is a pragmatic 
choice for how to keep down the number of spurious build failures with 
default -Werror without hiding warnings unduly (basically, the alternative 
is the existing practice of -Wno- options for whole source files) or 
requiring excessive work for architecture maintainers (if we decide it's 
OK to disable a particular warning, this just needs doing once, and then 
that case doesn't need thinking about again until the GCC version with 
which the warning was seen is no longer supported for building glibc).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

On 11/18/2014 03:00 PM, Joseph Myers wrote:
> Checking it means reviewing if the warning still appears...  We don't want to introduce lots of build failures every time GCC branches and the mainline version increases.
>   
In that case, we can do the equivalent of the following instead:

    #if __GNUC_PREREQ (4, 7)
    # if !__GNUC_PREPREQ (4, 10)
    #  pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
    # else
    #  pragma GCC diagnostic warning "-Wmaybe-uninitialized"
    # endif
    #endif


That is,  we can check the "4.9" without causing build failures in 5.0.  
That should be better than having an unchecked 4.9 sitting in the source.

(The above should use _Pragma not #pragma given your other comments, but 
this doesn't affect the main point here.)

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

On 11/18/2014 03:16 PM, Joseph Myers wrote:
> the smallest granularity I expect to be reliable is that of whole statements in source files outside of macros (with the pragma calls on separate lines)

It appears that we've been thinking along parallel lines.  I tried some 
of the tests that you evidently did, and found that GCC 4.9.2 doesn't 
work even then, in that the granularity isn't reliable even for whole 
statements, or even for whole functions.  This is a regression from GCC 
4.8.  I filed a bug report here:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63943

> Disabling for the relevant block of source lines, on all architectures and for all GCC versions that support the relevant -W option, is a pragmatic choice

Yes, and I guess my point is that the pragmatic choice we've taken in 
Gnulib is to disable diagnostics at the top level, as GCC has too many 
bugs in this area for us to go on wild goose chases trying to fine-tune 
diagnostics in smaller areas.  I'm skeptical whether it'll be worthwhile 
for glibc to chase those geese either, at least in the near future.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Tue, 18 Nov 2014, Paul Eggert wrote:

> On 11/18/2014 03:00 PM, Joseph Myers wrote:
> > Checking it means reviewing if the warning still appears...  We don't want
> > to introduce lots of build failures every time GCC branches and the mainline
> > version increases.
> >   
> In that case, we can do the equivalent of the following instead:
> 
>    #if __GNUC_PREREQ (4, 7)
>    # if !__GNUC_PREPREQ (4, 10)
>    #  pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
>    # else
>    #  pragma GCC diagnostic warning "-Wmaybe-uninitialized"
>    # endif
>    #endif
> 
> 
> That is,  we can check the "4.9" without causing build failures in 5.0.  That
> should be better than having an unchecked 4.9 sitting in the source.

But the basic point of using -Werror is that people don't look at warnings 
that scroll by, and shouldn't need to; cluttering build logs up with 
warnings doesn't help.  And the relevant time to check is when 4.9 is no 
longer supported (one person, or one per architecture, doing the checks 
then), rather than keeping updating the pragma every time a new GCC 
version branches.

Making the pragma turn the diagnostic into a warning is an interesting 
idea for how to do the checks at the point when we increase the minimum 
GCC version - it may be more convenient to turn all the cases where 4.9 is 
specified into warnings to check them all at once, rather than try 
deleting them and see which need adding back.  I don't think, however, 
that we need to work out now how we'll manage those checks in future and 
whether macros making more sophisticated use of the version number might 
be useful then.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Tue, 18 Nov 2014, Paul Eggert wrote:

> > Disabling for the relevant block of source lines, on all architectures and
> > for all GCC versions that support the relevant -W option, is a pragmatic
> > choice
> 
> Yes, and I guess my point is that the pragmatic choice we've taken in Gnulib
> is to disable diagnostics at the top level, as GCC has too many bugs in this
> area for us to go on wild goose chases trying to fine-tune diagnostics in
> smaller areas.  I'm skeptical whether it'll be worthwhile for glibc to chase
> those geese either, at least in the near future.

This is the one such optimization-related warning I see on x86_64 
(building glibc, rather than the testsuite - but in the testsuite we can 
be quite free with using dumb code, redundant initialization etc. to avoid 
warnings, without being at all concerned about resulting inefficiencies) 
with 4.9.  I think there are only a handful I've seen on other 
architectures.  (There may of course be some currently disabled with -Wno- 
options for whole files in the makefiles.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

Joseph Myers wrote:
> the relevant time to check is when 4.9 is no
> longer supported

If that's the case, I'm having trouble seeing why it's worth our time to have 
that unchecked "4.9" into the code.  If the only time the "4.9" matters is when 
we bump the min GCC release, we can simply reverify all the version-dependent 
diagnostics when that happens.  That way, we don't need to put the "4.9" into 
the source code now, which is a good thing because the "4.9" is probably wrong 
and it's confusing even if it's right.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

Joseph Myers wrote:
> This is the one such optimization-related warning I see on x86_64

Wow.  You're lucky.  I've run into quite a few such warnings with coreutils, 
Emacs, etc.  Perhaps it's because they enable more GCC warnings.

If this is the only warning we run into with glibc, then it doesn't matter much 
how it's addressed.  If we start running into this more, though, I expect the 
draft approach won't scale well.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Andreas Schwab]

Joseph Myers <joseph@codesourcery.com> writes:

> This is the one such optimization-related warning I see on x86_64 

This is the only other case I see:

res_send.c:794:8: warning: 'resplen' may be used uninitialized in this function [-Wmaybe-uninitialized]

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Tue, 18 Nov 2014, Paul Eggert wrote:

> Joseph Myers wrote:
> > the relevant time to check is when 4.9 is no
> > longer supported
> 
> If that's the case, I'm having trouble seeing why it's worth our time to have
> that unchecked "4.9" into the code.  If the only time the "4.9" matters is
> when we bump the min GCC release, we can simply reverify all the
> version-dependent diagnostics when that happens.  That way, we don't need to

The point is it's something you can grep for so that only a subset of the 
diagnostic disabling needs checking each time the minimum GCC version 
increases.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Wed, 19 Nov 2014, Andreas Schwab wrote:

> Joseph Myers <joseph@codesourcery.com> writes:
> 
> > This is the one such optimization-related warning I see on x86_64 
> 
> This is the only other case I see:
> 
> res_send.c:794:8: warning: 'resplen' may be used uninitialized in this function [-Wmaybe-uninitialized]

I've seen that on other architectures.  My proposal is to get the build 
and test clean with -Werror (or -Werror -Wno-error=undef if the -Wundef 
fixes aren't all in on time) for a single (architecture, compiler) pair, 
before adding the configuration support for enabling -Werror by default 
with --disable-werror to disable it and leaving it to people seeing other 
warnings to fix them or add the macros calling the pragmas as appropriate.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

Does anyone else wish to comment on any aspect of this patch 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00459.html>?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

Joseph Myers wrote:
> The point is it's something you can grep for so that only a subset of the
> diagnostic disabling needs checking each time the minimum GCC version
> increases.

This worries me, because it implies these macros will be so commonly used that 
the cost in adding (and more importantly, having developers read) GCC version 
numbers every time the macros are used is worth the benefit of checking only a 
subset of the uses once in a blue moon.  If these macros are rarely used, the 
version numbers won't help much; and if the macros are used often I expect we 
will have so many other problems that the version numbers won't help much.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

A couple of other thoughts.

First, the syntax DIAG_IGNORE (-Wmaybe-uninitialized, ...) is too clever and 
brittle.  Too clever, because in C code it looks like it's subtracting 
'uninitialized' from the negative of 'Wmaybe'.  Too brittle, because suppose GCC 
adds a warning option syntax with commas in it? 
"-Wsuggest-attribute=format,noreturn", say?  Then DIAG_IGNORE 
(-Wsuggest-attribute=format,noreturn, ...) won't work because of C's 
tokenization rules.  For both of these reasions it would be better to use a 
string, e.g., DIAG_IGNORE ("-Wmaybe-uninitialized", ...).

More important, I looked through some of the Gnulib and GNU utilities code that 
deals with working around bugs in GCC's diagnostics generation, and 
-Wmaybe-uninitialized in particular has caused us so much grief that we 
typically silence it in a different way, which I should mention.  We use a macro 
definition like this:

   #ifdef lint
   # define IF_LINT(Code) Code
   #else
   # define IF_LINT(Code) /* empty */
   #endif

and then write declarations like this:

   off_t size IF_LINT ( = 0);

when GCC wrongly complains that 'size' may be used unininitialized.  That way, 
people who want to use -Wmaybe-uninitialized can get clean compiles by compiling 
with '-Wmaybe-uninitialized -Dlint', and people who don't care about this stuff 
can omit the options and get slightly more-efficient code.  Obviously there are 
some pitfalls in this approach but overall its benefits outweigh its cost for 
us.  In particular, it lets us carefully isolate the uninitialized-warning bug, 
whereas the DIAG_IGNORE approach is more of a blunderbuss that disables the 
warning in a too-large region because of GCC's bugs.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Fri, 21 Nov 2014, Paul Eggert wrote:

> A couple of other thoughts.
> 
> First, the syntax DIAG_IGNORE (-Wmaybe-uninitialized, ...) is too clever and
> brittle.  Too clever, because in C code it looks like it's subtracting
> 'uninitialized' from the negative of 'Wmaybe'.  Too brittle, because suppose
> GCC adds a warning option syntax with commas in it?
> "-Wsuggest-attribute=format,noreturn", say?  Then DIAG_IGNORE
> (-Wsuggest-attribute=format,noreturn, ...) won't work because of C's
> tokenization rules.  For both of these reasions it would be better to use a
> string, e.g., DIAG_IGNORE ("-Wmaybe-uninitialized", ...).

That seems reasonable, but does anyone else have comments on it?

>   off_t size IF_LINT ( = 0);
> 
> when GCC wrongly complains that 'size' may be used unininitialized.  That way,
> people who want to use -Wmaybe-uninitialized can get clean compiles by
> compiling with '-Wmaybe-uninitialized -Dlint', and people who don't care about
> this stuff can omit the options and get slightly more-efficient code.
> Obviously there are some pitfalls in this approach but overall its benefits
> outweigh its cost for us.  In particular, it lets us carefully isolate the
> uninitialized-warning bug, whereas the DIAG_IGNORE approach is more of a
> blunderbuss that disables the warning in a too-large region because of GCC's
> bugs.

I believe it's established glibc practice not to add initializations that 
would change generated code to silence warnings.  In any case, that 
doesn't appear to help for the case this patch relates to; the 
uninitialized object '*((void *)&str+4)' is outside the bounds of "UCHAR 
str[1]" (this is a case where for these particular findidx calls the 
out-of-bounds access is unreachable, but GCC doesn't see that - very much 
like -Warray-bounds bogus warnings sometimes seen on some architectures 
for one or two files), and initializing within the bounds doesn't stop the 
warning.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Fri, 21 Nov 2014, Paul Eggert wrote:

> Joseph Myers wrote:
> > The point is it's something you can grep for so that only a subset of the
> > diagnostic disabling needs checking each time the minimum GCC version
> > increases.
> 
> This worries me, because it implies these macros will be so commonly used that
> the cost in adding (and more importantly, having developers read) GCC version
> numbers every time the macros are used is worth the benefit of checking only a
> subset of the uses once in a blue moon.  If these macros are rarely used, the
> version numbers won't help much; and if the macros are used often I expect we
> will have so many other problems that the version numbers won't help much.

Even with a few uses, it's still helpful to be able to check for new GCC 
versions incrementally rather than requiring all the checks for all pragma 
uses to be done at once, or the state of which have been checked to be 
tracked externally - as soon as there are pragmas for warnings seen on 
multiple architectures, it may not be convenient for the person increasing 
the required GCC version to check whether all the warnings are still 
applicable, and so the version numbers mean the state of the partial 
transition is visible in the source tree.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

Joseph Myers wrote:
> it's established glibc practice not to add initializations that
> would change generated code to silence warnings.

Sure, and that's what the IF_LINT macro calls do in coreutils, Emacs, etc.: they 
don't alter the code used in production.  In practice this leads to code that is 
definitely easier to maintain and arguably more resistant against faults than 
the DIAG_IGNORE approach being proposed.

> that doesn't appear to help for the case this patch relates to; the
> uninitialized object '*((void *)&str+4)' is outside the bounds of "UCHAR
> str[1]"

If GCC is griping about (say) some component of the array xyz not being 
initialized, then in the worst case one can pacify it by initializing the array 
in its declaration, e.g., 'int xyz[100] IF_LINT (= {0});'.  Of course if GCC is 
griping only about xyz[2], one can tighten this by initializing just xyz[2], 
which is preferable.  In my experience, it's rare to need to do anything so 
drastic as to initialize an entire array, as GCC warning bugs generally have to 
do with local scalars.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Roland McGrath]

I intend to follow up, but I ran out of time this week.
I'll follow up by the end of Monday.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Fri, 21 Nov 2014, Paul Eggert wrote:

> Joseph Myers wrote:
> > it's established glibc practice not to add initializations that
> > would change generated code to silence warnings.
> 
> Sure, and that's what the IF_LINT macro calls do in coreutils, Emacs, etc.:
> they don't alter the code used in production.  In practice this leads to code
> that is definitely easier to maintain and arguably more resistant against
> faults than the DIAG_IGNORE approach being proposed.

The idea for glibc is to use -Werror in production (as per 
<https://sourceware.org/ml/libc-alpha/2012-08/msg00404.html> - "It's 
especially important to catch subtle regressions when building releases.") 
- which means warnings need to be avoided in production as well.

> > that doesn't appear to help for the case this patch relates to; the
> > uninitialized object '*((void *)&str+4)' is outside the bounds of "UCHAR
> > str[1]"
> 
> If GCC is griping about (say) some component of the array xyz not being
> initialized, then in the worst case one can pacify it by initializing the
> array in its declaration, e.g., 'int xyz[100] IF_LINT (= {0});'.  Of course if
> GCC is griping only about xyz[2], one can tighten this by initializing just
> xyz[2], which is preferable.  In my experience, it's rare to need to do
> anything so drastic as to initialize an entire array, as GCC warning bugs
> generally have to do with local scalars.

As I understand this warning, it's warning about str[1] for an array 
wint_t str[1] for which only str[0] is valid and only str[0] exists to be 
initialized (but the - unreachable - code is, after propagating some 
constants, accessing str[1]).  So it's not possible to initialize the 
element warned about without also increasing the size of the array (and 
experimentally, initializing the existing element makes no change to the 
warning).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

I should add: I do *not* think we should have any sort of -Werror or 
diagnostic control policy that involves this level of analysis for each 
patch disabling warnings; it should be sufficient that either the warning 
appears to be a false positive without an obvious easy and safe fix or 
there is some other reason it's hard or inappropriate to fix (e.g. tests 
of deprecated interfaces, _FORTIFY_SOURCE checks that also generate 
compile-time warnings, etc.).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

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

Joseph Myers wrote:
> it's warning about str[1] for an array
> wint_t str[1] for which only str[0] is valid and only str[0] exists to be
> initialized (but the - unreachable - code is, after propagating some
> constants, accessing str[1]).

Yeouch, GCC is even more confused than I thought.

Instead of masking the GCC bug, how about changing the libc source slightly so 
that the bug isn't tickled?  The attached patch does that, and this fixes the 
bug for me (x86-64, both GCC 4.9.2 and GCC 4.8.3) without having to fiddle with 
any pragmas.  As a bonus this patch seems to make the resulting machine code 
slightly more efficient; it's smaller, anyway.

We did this sort of thing with gnulib etc. too.  If memory serves, occasionally 
the buggy warnings were signs of other trouble within GCC, so it wasn't 
unreasonable to change the code to pacify the compiler.  I do try to file GCC 
bug reports for this sort of thing but I'm not perfect in that regard.

[-- Attachment #2: 0001-fnmatch-work-around-GCC-compiler-warning-bug-with-un.patch --]
[-- Type: text/x-diff, Size: 2568 bytes --]

From 7c16bdc58c74e3515d8d71e5e006005566e5a32a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 21 Nov 2014 20:57:58 -0800
Subject: [PATCH] fnmatch: work around GCC compiler warning bug with uninit var

* posix/fnmatch_loop.c (FCT): Use a scalar not a one-item array.
This works around a bug with x86-64 GCC 4.9.2 and earlier
where 'gcc -O2 -Wmaybe-uninitialized' incorrectly complains
"../locale/weightwc.h:93:7: warning: '*((void *)&str+4)' may be
used uninitialized in this function [-Wmaybe-uninitialized]".
---
 ChangeLog            | 9 +++++++++
 posix/fnmatch_loop.c | 8 ++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c75dab7..3c7b9e4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2014-11-21  Paul Eggert  <eggert@cs.ucla.edu>
+
+	fnmatch: work around GCC compiler warning bug with uninit var
+	* posix/fnmatch_loop.c (FCT): Use a scalar not a one-item array.
+	This works around a bug with x86-64 GCC 4.9.2 and earlier
+	where 'gcc -O2 -Wmaybe-uninitialized' incorrectly complains
+	"../locale/weightwc.h:93:7: warning: '*((void *)&str+4)' may be
+	used uninitialized in this function [-Wmaybe-uninitialized]".
+
 2014-11-21  Roland McGrath  <roland@hack.frob.com>
 
 	* nptl/pthread_create.c (__pthread_create_2_1): Set
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
index db6d9d7..c1d8b69 100644
--- a/posix/fnmatch_loop.c
+++ b/posix/fnmatch_loop.c
@@ -343,7 +343,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 #ifdef _LIBC
 		else if (c == L('[') && *p == L('='))
 		  {
-		    UCHAR str[1];
+		    UCHAR str;
 		    uint32_t nrules =
 		      _NL_CURRENT_WORD (LC_COLLATE, _NL_COLLATE_NRULES);
 		    const CHAR *startp = p;
@@ -355,7 +355,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 			c = L('[');
 			goto normal_bracket;
 		      }
-		    str[0] = c;
+		    str = c;
 
 		    c = *++p;
 		    if (c != L('=') || p[1] != L(']'))
@@ -368,7 +368,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 
 		    if (nrules == 0)
 		      {
-			if ((UCHAR) *n == str[0])
+			if ((UCHAR) *n == str)
 			  goto matched;
 		      }
 		    else
@@ -383,7 +383,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 # endif
 			const int32_t *indirect;
 			int32_t idx;
-			const UCHAR *cp = (const UCHAR *) str;
+			const UCHAR *cp = (const UCHAR *) &str;
 
 # if WIDE_CHAR_VERSION
 			table = (const int32_t *)
-- 
1.9.3

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Roland McGrath]

When I said I wanted a macro, what I had in mind was a single macro
that would wrap the affected code, e.g.

	SUPPRESS_WARNING ("maybe-uninitialized",
			  ... code inside here ...
			 )

My rationale was to structure the source code such that it would be
difficult to accidentally spread the suppression zone across more code
than really needed it (or at least be easy to be lazy and still not do
that).

Paul took my strawman "tightest possible scope" to its absurd extreme.
That maybe have been a bit of baiting on my part.  I always expected
we would find a balance between the literal tighest possible scope and
a scope that is tight enough to minimize the risk of masking warnings
we actually want to see (especially as the code is changed later)
while not making the code so contorted that it really hampers
maintainability in another way.

If GCC's limitations make it infeasible to narrow the suppression
scope very much, and/or make it impossible to use a surrounding macro
to do so like my idea, then those ideas are pretty much out the
window.  The pragma syntax is a bit verbose and clunky, so some macros
to make things more concise are still probably worthwhile.

The bottom line is that I want it to be tractable and not gratuitously
cumbersome to suppress particular warnings, but I want it to be
difficult to do so indiscriminately or accidentally and easiest to
maintain and change the code such that we won't inadvertently have
warnings suppressed for new code.  I'm far less concerned about
warnings in test code.  So, if outside of test code we in fact have a
very small number of warnings in need of suppression, perhaps I've
made a mountain of a molehill.

Regardless of the mechanics we settle on, I want to say some things
about review standards for new warning suppressions.  The bar to
adding a warning suppression should be quite high.  It should be
especially high for the "find a bug via control-flow analysis" class
of warnings such as -Wmaybe-uninitialized.  The weightwc.h example in
Joseph's patch has a little of the important information, but IMHO
it's almost a perfect example of what not to do.  It says under what
conditions the warning was observed (machine, compiler version), which
is important information.  

What's at least as important is the rationale by which we're confident
that the compiler is wrong and the code is right.  Just because a
warning has been there for a long time, and we've let it pass, and we
aren't aware of a bug in the code, does not mean that there isn't
actually a bug in the code.  I looked at the fnmatch/weightwc case a
while ago and after a few minutes of looking at the code (with which I
was not especially familiar) could not easily convince myself that I
knew for sure that use of an uninitialized value was impossible.
Joseph has since described his own analysis of the code and made the
case that it is in fact safe.  An explanation of that analysis and the
"proof" (informally speaking) of why the warning is wrong is what
should be required in comments at the site of a warning suppression.

When someone takes the time to do such analysis, he will then be close
enough to the code that he might see ways to change the code so that
the warning goes away without a suppression, as Paul did for this
case.  That is always the best possible result, and what we should be
striving for.  Accepting that a warning suppression is correct should
be the penultimate resort, and never done lightly (the last one, that
we never get to, being pessimizing the code with dead stores and the
like).


Thanks,
Roland

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Roland McGrath]

Thanks very much for tracking down that change!  That's exactly what we'd
like to see done instead of warning suppression whenever it's possible.
The only thing I'd like you to do differently is to put more explanation in
the code itself rather than only in the ChangeLog.  e.g.

	/* It's important that this be a scalar variable rather than a
	   one-element array, because GCC (at least 4.9.2 -O2 on x86-64)
	   can be confused by the array and diagnose a "used initialized"
	   in a dead branch in the findidx function.  */


Thanks,
Roland

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Mon, 24 Nov 2014, Roland McGrath wrote:

> If GCC's limitations make it infeasible to narrow the suppression
> scope very much, and/or make it impossible to use a surrounding macro
> to do so like my idea, then those ideas are pretty much out the
> window.  The pragma syntax is a bit verbose and clunky, so some macros
> to make things more concise are still probably worthwhile.

The surrounding macro doesn't provide the interval of lines with the 
pragma enabled, which is needed for it to work.

(And in some cases, a pragma for the whole file may make sense - if a test 
is deliberately testing deprecated interfaces or _FORTIFY_SOURCE runtime 
handling, disabling corresponding warnings for the whole file seems 
appropriate.)

> The bottom line is that I want it to be tractable and not gratuitously
> cumbersome to suppress particular warnings, but I want it to be
> difficult to do so indiscriminately or accidentally and easiest to
> maintain and change the code such that we won't inadvertently have
> warnings suppressed for new code.  I'm far less concerned about
> warnings in test code.  So, if outside of test code we in fact have a
> very small number of warnings in need of suppression, perhaps I've
> made a mountain of a molehill.

We have the weightwc.h warnings for which Paul has found another fix.  We 
have the nscd/connections.c "ignoring return value of 'setuid', declared 
with attribute warn_unused_result [-Wunused-result]" (and similar for 
setgid), in a context where another id-setting call has already failed, 
and while you could assign to a dummy variable it's not clear that has any 
advantage over explicitly suppressing the warnings.  We have, for some 
architecture/compiler combinations, "res_send.c:795:41: warning: 'resplen' 
may be used uninitialized in this function [-Wmaybe-uninitialized]" (a 
case where the relevant code only gets executed if the variable is 
initialized - the first time round a loop will execute a different if 
branch and cause it to be initialized).  And that's it for warnings (on 
x86_64 / ARM / PowerPC, in my testing) for which suppression seems a 
sensible first approach.

Maybe there would be more arising from conversion of -Wno- options in 
makefiles to use of macros / pragmas - but if so, high standards simply 
imply it's hard to move from suppression for a whole file to more 
selective suppression.

> Regardless of the mechanics we settle on, I want to say some things
> about review standards for new warning suppressions.  The bar to
> adding a warning suppression should be quite high.  It should be

I think we should be perfectly willing to file a bug in Bugzilla and then 
add a suppression referencing the bug, if there's something tricky about 
determining if there's a real glibc bug there or a better way to address 
the warning - that means the build stays working on more platforms (in the 
presence of -Werror).  I see no reason why we should assume a possible 
problem in existing code shown up by a warning (from new GCC or on another 
architecture) is more serious than other bugs.  -Werror is first about 
stopping new issues getting in accidentally, rather than forcing certain 
existing issues (those that cause warnings) to be higher priority than 
other existing issues.

(For example, on PowerPC you get

../sysdeps/ieee754/ldbl-128/e_lgammal_r.c:75:1: warning: floating constant 
exceeds range of 'long double' [-Woverflow]
../sysdeps/ieee754/ldbl-128/e_lgammal_r.c:78:1: warning: floating constant 
exceeds range of 'long double' [-Woverflow]

and while the right long-term fix is certainly *not* a warning 
suppression, it would seem reasonable as an interim fix for build breakage 
for an indefinite amount of time, with the comment referring to bug 
16347.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

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

On 11/24/2014 04:00 PM, Roland McGrath wrote:
> The only thing I'd like you to do differently is to put more explanation in
> the code itself

Thanks, I installed the patch with that change.

[-- Attachment #2: 0001-fnmatch-work-around-GCC-compiler-warning-bug-with-un.patch --]
[-- Type: text/x-patch, Size: 2853 bytes --]

From 27f9b288f9af8ad7862c9852c4e002dbdf8207cc Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 25 Nov 2014 14:12:48 -0800
Subject: [PATCH] fnmatch: work around GCC compiler warning bug with uninit var

* posix/fnmatch_loop.c (FCT): Use a scalar not a one-item array.
This works around a bug with x86-64 GCC 4.9.2 and earlier
where 'gcc -O2 -Wmaybe-uninitialized' incorrectly complains
"../locale/weightwc.h:93:7: warning: '*((void *)&str+4)' may be
used uninitialized in this function [-Wmaybe-uninitialized]".
---
 ChangeLog            |  9 +++++++++
 posix/fnmatch_loop.c | 13 +++++++++----
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8ee0650..c020ed4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2014-11-25  Paul Eggert  <eggert@cs.ucla.edu>
+
+	fnmatch: work around GCC compiler warning bug with uninit var
+	* posix/fnmatch_loop.c (FCT): Use a scalar not a one-item array.
+	This works around a bug with x86-64 GCC 4.9.2 and earlier
+	where 'gcc -O2 -Wmaybe-uninitialized' incorrectly complains
+	"../locale/weightwc.h:93:7: warning: '*((void *)&str+4)' may be
+	used uninitialized in this function [-Wmaybe-uninitialized]".
+
 2014-11-25  Joseph Myers  <joseph@codesourcery.com>
 
 	* posix/bug-regex31.c (main): Return RES not 0.
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
index db6d9d7..1e27913 100644
--- a/posix/fnmatch_loop.c
+++ b/posix/fnmatch_loop.c
@@ -343,7 +343,12 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 #ifdef _LIBC
 		else if (c == L('[') && *p == L('='))
 		  {
-		    UCHAR str[1];
+		    /* It's important that STR be a scalar variable rather
+		       than a one-element array, because GCC (at least 4.9.2
+		       -O2 on x86-64) can be confused by the array and
+		       diagnose a "used initialized" in a dead branch in the
+		       findidx function.  */
+		    UCHAR str;
 		    uint32_t nrules =
 		      _NL_CURRENT_WORD (LC_COLLATE, _NL_COLLATE_NRULES);
 		    const CHAR *startp = p;
@@ -355,7 +360,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 			c = L('[');
 			goto normal_bracket;
 		      }
-		    str[0] = c;
+		    str = c;
 
 		    c = *++p;
 		    if (c != L('=') || p[1] != L(']'))
@@ -368,7 +373,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 
 		    if (nrules == 0)
 		      {
-			if ((UCHAR) *n == str[0])
+			if ((UCHAR) *n == str)
 			  goto matched;
 		      }
 		    else
@@ -383,7 +388,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 # endif
 			const int32_t *indirect;
 			int32_t idx;
-			const UCHAR *cp = (const UCHAR *) str;
+			const UCHAR *cp = (const UCHAR *) &str;
 
 # if WIDE_CHAR_VERSION
 			table = (const int32_t *)
-- 
1.9.3

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

On 11/24/2014 05:29 PM, Joseph Myers wrote:
> We have the nscd/connections.c "ignoring return value of 'setuid', declared
> with attribute warn_unused_result [-Wunused-result]" (and similar for
> setgid), in a context where another id-setting call has already failed

Yes, we've run into that several times in GNU applications and we came 
up with a better way to fix it, in the Gnulib ignore-value module, which 
defines this:

# define ignore_value(x) \
     (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))

One can then write "ignore_value (setuid (server_uid))" instead of 
"setuid (server_uid)".  So we shouldn't need a pragma for this one.

> We have, for some
> architecture/compiler combinations, "res_send.c:795:41: warning: 'resplen'
> may be used uninitialized in this function [-Wmaybe-uninitialized]" (a
> case where the relevant code only gets executed if the variable is
> initialized - the first time round a loop will execute a different if
> branch and cause it to be initialized).

Neither you nor Roland liked the IF_LINT approach that Gnulib uses for 
this sort of thing, but I hope we can figure out something else to 
satisfy glibc's constraints.  How about something like this?

#define ASSUME_MEM_INITIALIZED(v) asm ("" : "=m" (v))

and then write "ASSUME_MEM_INITIALIZED (resplen)" after declaring resplen.

> And that's it for warnings (on
> x86_64 / ARM / PowerPC, in my testing) for which suppression seems a
> sensible first approach.

In that case, it's not unreasonable to hope that we can avoid the 
pragmas entirely.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Roland McGrath]

> Neither you nor Roland liked the IF_LINT approach that Gnulib uses for 
> this sort of thing, but I hope we can figure out something else to 
> satisfy glibc's constraints.  How about something like this?
> 
> #define ASSUME_MEM_INITIALIZED(v) asm ("" : "=m" (v))
> 
> and then write "ASSUME_MEM_INITIALIZED (resplen)" after declaring resplen.

With "=g" as the constraint it probably won't affect code generation at all.
So that could be OK.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Tue, 25 Nov 2014, Paul Eggert wrote:

> On 11/24/2014 05:29 PM, Joseph Myers wrote:
> > We have the nscd/connections.c "ignoring return value of 'setuid', declared
> > with attribute warn_unused_result [-Wunused-result]" (and similar for
> > setgid), in a context where another id-setting call has already failed
> 
> Yes, we've run into that several times in GNU applications and we came up with
> a better way to fix it, in the Gnulib ignore-value module, which defines this:
> 
> # define ignore_value(x) \
>     (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
> 
> One can then write "ignore_value (setuid (server_uid))" instead of "setuid
> (server_uid)".  So we shouldn't need a pragma for this one.

Sure, that can be done if preferred to using pragmas to make it clear this 
is exactly about disabling a diagnostic.

> > We have, for some
> > architecture/compiler combinations, "res_send.c:795:41: warning: 'resplen'
> > may be used uninitialized in this function [-Wmaybe-uninitialized]" (a
> > case where the relevant code only gets executed if the variable is
> > initialized - the first time round a loop will execute a different if
> > branch and cause it to be initialized).
> 
> Neither you nor Roland liked the IF_LINT approach that Gnulib uses for this
> sort of thing, but I hope we can figure out something else to satisfy glibc's
> constraints.  How about something like this?
> 
> #define ASSUME_MEM_INITIALIZED(v) asm ("" : "=m" (v))
> 
> and then write "ASSUME_MEM_INITIALIZED (resplen)" after declaring resplen.

It's only in a particular bit of code where we want to assume it's 
initialized.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

Joseph Myers wrote:
> It's only in a particular bit of code where we want to assume it's
> initialized.

We could use ASSUME_INITIALIZED only in the particular bit of code where it's 
needed, as it doesn't have to be used immediately after a declaration.

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Joseph Myers]

On Tue, 25 Nov 2014, Paul Eggert wrote:

> Joseph Myers wrote:
> > It's only in a particular bit of code where we want to assume it's
> > initialized.
> 
> We could use ASSUME_INITIALIZED only in the particular bit of code where it's
> needed, as it doesn't have to be used immediately after a declaration.

But won't the proposed asm, with its output operand for resplen, imply 
that any previous value in resplen is dead - which is not correct if you 
use it only there (where the point is that in fact there is a value there, 
which is not dead, but the compiler can't see there must be a value there) 
rather than before any possible initialization?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Add macros for diagnostic control, use them in locale/weightwc.h

[Paul Eggert]

Joseph Myers wrote:
> On Tue, 25 Nov 2014, Paul Eggert wrote:
>
>> >Joseph Myers wrote:
>>> > >It's only in a particular bit of code where we want to assume it's
>>> > >initialized.
>> >
>> >We could use ASSUME_INITIALIZED only in the particular bit of code where it's
>> >needed, as it doesn't have to be used immediately after a declaration.
> But won't the proposed asm, with its output operand for resplen, imply
> that any previous value in resplen is dead

Ah, true.  Still, if we use it just after resplen's declaration, we've localized 
the assumption well -- better than we would have done with the pragmas, and 
that's good enough.