Re: gcc warning with "some variable may be used uninitialized in this function [-Wmaybe-uninitialized]" when building under msys

[Pedro Alves <palves@redhat.com>]

On 10/05/2018 06:08 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> If the warnings confuse people too much, I'd be OK with
> Pedro> disabling -Wmaybe-uninitlized completely.  I left it as a
> Pedro> -Wno-error warning because even though it produces false positives,
> Pedro> it also helps catch bugs earlier in the compile-edit cycle,
> Pedro> when you're hacking some code, when you're introducing
> Pedro> uninitialized uses, and "make" ends up compiling just a few
> Pedro> files.
> 
> It caught a bug in the -Wshadow=local series; and I think in most cases
> the false reports are easily handled with an initialization.  I suppose
> in theory these initializations could themselves mask bugs, but I don't
> recall that ever actually happening (or at least being noticed).

The sort of bug not-initializing prevents is that kind that would be
caught during development, via more -Wmaybe-unitialized/-Wuninitialized
warnings, or simply GDB crashes/regressions.  I.e., the bug caused by
reworking the code creating a new path that leads to the variable not
being initialized.  I do recall that happening to me, but it's of course
hard to measure.

If we can avoid the forced-initialization, say, by restructuring code,
I tend to prefer that.  The usual case that leads to false positives
is around TRY/CATCH, exception flow.  For example, in the guile hunk
at <https://sourceware.org/ml/gdb-patches/2018-10/msg00101.html>,
I think the problem is that GDBSCM_HANDLE_GDB_EXCEPTION
is defined as:

#define GDBSCM_HANDLE_GDB_EXCEPTION(exception)          \
  do {                                                  \
    if (exception.reason < 0)                           \
      {                                                 \
        gdbscm_throw_gdb_exception (exception);         \
        /*NOTREACHED */                                 \
      }                                                 \
  } while (0)

while the code that is using it is:

  TRY
    {
      gdb::unique_xmalloc_ptr<gdb_byte> buffer;
      LA_GET_STRING (value, &buffer, &length, &char_type, &la_encoding);
      buffer_contents = buffer.release ();
    }
  CATCH (except, RETURN_MASK_ALL)
    {
      xfree (encoding);
      GDBSCM_HANDLE_GDB_EXCEPTION (except);
    }
  END_CATCH

Note how GDBSCM_HANDLE_GDB_EXCEPTION is used inside a CATCH
block, where we know that exception.reason is definitely < 0.
GCC doesn't know that, so it thinks there could be a path
where the catch block doesn't rethrow, leaving buffer_contents
uninitialized.

So replacing that GDBSCM_HANDLE_GDB_EXCEPTION call
with a direct call to gdbscm_throw_gdb_exception makes the
warning would go away.

So for these types of bugs / warnings, I agree, the warning
is useful.

It's for the tricker cases, like std::optional, where a variable's
initialization depends on the value of some other state (like
another variable), where the warning ends up producing
false positives.

> 
> It would be good if gcc could recognize std::optional and not issue the
> warning when it is used.  Perhaps gdb could then just always use
> optional for the maybe-not-initialized cases.

Really not sure whether that is possible.  I think there's hope
that GCC value tracking becomes smart enough that these
std::optional-related warnings end up disappearing (which usually
means the code will optimize better too).  Fingers crossed, at least.

Thanks,
Pedro Alves