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

Pedro Alves
2018-10-09T10:34:10Z

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