Re: [PATCH] Fix -Wmaybe-uninitialized warnings under -O0

From: Jeff King <peff@peff.net>
Date: Apr 3, 2020, 14:04 UTC
Message-ID: <20200403140447.GA639586@coredump.intra.peff.net>
In-Reply-To: <20200401140643.GA880372@generichostname>

On Wed, Apr 01, 2020 at 10:06:43AM -0400, Denton Liu wrote:

Show 10 quoted lines

> > So why does your version behave differently? And if this is a temporary
> > state for a buggy version of gcc (that may be fixed in the next point
> > release), is it worth changing our source code to appease it?
> 
> A correction to the earlier message... It seems like I wasn't reporting
> the correct settings. I was actually compiling with -Og, not -O0
> (whoops!).
> 
> I tested it with gcc-8 and it seems like it also reports the same
> problem. Also, -O1 reports warnings as well.

Ah, OK, I can reproduce easily with -Og (up through gcc-10). Most of them don't trigger with -O1; just the one in ref-filter.c.

That one's interesting. We have:

  int ret = 0;
  ...
  if (...)
         ...
  else
         ret = for_each_fullref_in_pattern(...);
  ...
  return ret;

So we'd either have 0 or an assigned return. But the bug is actually in for_each_fullref_in_pattern(), which does this:

  int ret; /* uninitialized! */

  /* a bunch of early return conditionals */
  if (...)
    return ...;

  for_each_string_list_item(...) {
    ret = for_each_fullref_in(...);
  }

  return ret;

but that will return an uninitialized value when there are no patterns. I doubt we have such a case, but that may explain why -O0 does not complain (it assumes "in_pattern" will return a useful value) and -O2 does not (it is able to figure out that it always does), but -O1 only inlines part of it.

Curiously, -Og _does_ find the correct function.

Your patch silences it, but is it doing the right thing? It sets "ret = 0", but we haven't actually iterated anything. Should it be an error instead?

-Peff