Coping with gcc warning due to limitation of gcc analysis?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Coping with gcc warning due to limitation of gcc analysis?

Samuel Thibault-9
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
Reply | Threaded
Open this post in threaded view
|

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
Reply | Threaded
Open this post in threaded view
|

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

Samuel Thibault-9
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
Reply | Threaded
Open this post in threaded view
|

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

Florian Weimer
In reply to this post by Samuel Thibault-9
* 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.
Reply | Threaded
Open this post in threaded view
|

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

Joseph Myers
In reply to this post by Samuel Thibault-9
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
[hidden email]