Coping with gcc warning due to limitation of gcc analysis?

Coping with gcc warning due to limitation of gcc analysis?

[Samuel Thibault]

Hello,

We have a warning in sysdeps/mach/hurd/getresgid.c:

../sysdeps/mach/hurd/getresgid.c:57:9: warning: 'saved' may be used uninitialized in this function

Basically the structure of the function is the following:

int
__getresgid (gid_t *rgid, gid_t *egid, gid_t *sgid)
{
  gid_t saved;
  ...

  err = foo ();
  if (!err)
    {
      if (bar)
	err = EBAR;
      else
	{
	  saved = bat;
	}
    }

  if (err)
    return err;

  *sgid = saved;  /* The warning is triggered here */
  ...
}

i.e. in all the cases where `saved' is not initialized, err is set to
non-zero, and thus we return before ever using saved.

What is the glibc-preferred way to deal with this? Of course I could
just initialize saved to a dumb value like -1 but the reader might
wonder why unless we put a blunt comment, and a smarter compiler might
later warn "-1 set in `saved' is never used"...

Samuel

Re: Coping with gcc warning due to limitation of gcc analysis?

[Jeff Law]

On 01/27/2018 01:54 PM, Samuel Thibault wrote:
> Hello,
> 
> We have a warning in sysdeps/mach/hurd/getresgid.c:
> 
> ../sysdeps/mach/hurd/getresgid.c:57:9: warning: 'saved' may be used uninitialized in this function
> 
> Basically the structure of the function is the following:
> 
> int
> __getresgid (gid_t *rgid, gid_t *egid, gid_t *sgid)
> {
>   gid_t saved;
>   ...
> 
>   err = foo ();
>   if (!err)
>     {
>       if (bar)
> 	err = EBAR;
>       else
> 	{
> 	  saved = bat;
> 	}
>     }
> 
>   if (err)
>     return err;
> 
>   *sgid = saved;  /* The warning is triggered here */
>   ...
> }
> 
> i.e. in all the cases where `saved' is not initialized, err is set to
> non-zero, and thus we return before ever using saved.
> 
> What is the glibc-preferred way to deal with this? Of course I could
> just initialize saved to a dumb value like -1 but the reader might
> wonder why unless we put a blunt comment, and a smarter compiler might
> later warn "-1 set in `saved' is never used"...
First, make sure you've filed a bug with a reproducer in GCC :-)  We
work diligently to address these false positives.  It may even be the
case that this is one we've already fixed on the trunk.

Jeff

Re: Coping with gcc warning due to limitation of gcc analysis?

[Samuel Thibault]

Jeff Law, on sam. 27 janv. 2018 14:25:29 -0700, wrote:
> On 01/27/2018 01:54 PM, Samuel Thibault wrote:
> > What is the glibc-preferred way to deal with this? Of course I could
> > just initialize saved to a dumb value like -1 but the reader might
> > wonder why unless we put a blunt comment, and a smarter compiler might
> > later warn "-1 set in `saved' is never used"...
> First, make sure you've filed a bug with a reproducer in GCC :-)

I have just filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84078

So should I set CFLAGS-getresgid.c += -Wno-error=maybe-uninitialized
?

Samuel

Re: Coping with gcc warning due to limitation of gcc analysis?

[Florian Weimer]

* Samuel Thibault:

> What is the glibc-preferred way to deal with this?

The preferred way is to rewrite the code to make it clearer.  For
__getresgid, that would probably involve writing to *rgid, *egid,
*sgid directly, without relying on the temporary variables.

For cases where this is not possible, there are the
DIAG_PUSH_NEEDS_COMMENT, DIAG_IGNORE_NEEDS_COMMENT,
DIAG_POP_NEEDS_COMMENT macros.

Re: Coping with gcc warning due to limitation of gcc analysis?

[Joseph Myers]

On Sat, 27 Jan 2018, Samuel Thibault wrote:

> What is the glibc-preferred way to deal with this? Of course I could
> just initialize saved to a dumb value like -1 but the reader might
> wonder why unless we put a blunt comment, and a smarter compiler might
> later warn "-1 set in `saved' is never used"...

As a general rule: we avoid such initializers that are only intended to 
suppress warnings rather than ever to have that initial value used.  If 
the code can be changed to avoid the warning without being less efficient, 
that's preferred.  Otherwise, use the DIAG_*_NEEDS_COMMENT macros (with 
appropriate explanatory comment detailing the warning and why it's a false 
positive).  Using -Wno-error=* or -Wno-* in the Makefiles is less 
desirable, and generally only appropriate if the pragmas don't work for 
some reason (e.g. no option controlling the warning in question to use in 
a pragma) or can't be used (e.g. file copied verbatim from an upstream 
source not using such pragmas and we don't want local changes to it).

-- 
Joseph S. Myers
joseph@codesourcery.com