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

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

[asmwarrior]

Hi, when building gdb git head(which is 875e539851bb2702f3292f819e220545a8776242 as 2018-10-04) under msys+gcc 5.4, I see such warning:

  CXX    infrun.o
In file included from ../../binutils-gdb/gdb/inferior.h:49:0,
                 from ../../binutils-gdb/gdb/infrun.c:26:
../../binutils-gdb/gdb/progspace.h: In function 'void handle_vfork_child_exec_or_exit(int)':
../../binutils-gdb/gdb/progspace.h:285:47: warning: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' may be used uninitialized in this function [-Wmaybe-uninitialized]
   { set_current_program_space (m_saved_pspace); }
                                               ^
../../binutils-gdb/gdb/infrun.c:931:6: note: '*((void*)(& maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace' was declared here
      maybe_restore_inferior;
      ^
  CXX    inline-frame.o

Hope you devs can fix this. Thanks.

Asmwarrior

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

[Simon Marchi]

On 2018-10-04 02:28, asmwarrior wrote:
> Hi, when building gdb git head(which is
> 875e539851bb2702f3292f819e220545a8776242 as 2018-10-04) under msys+gcc
> 5.4, I see such warning:
> 
>   CXX    infrun.o
> In file included from ../../binutils-gdb/gdb/inferior.h:49:0,
>                  from ../../binutils-gdb/gdb/infrun.c:26:
> ../../binutils-gdb/gdb/progspace.h: In function 'void
> handle_vfork_child_exec_or_exit(int)':
> ../../binutils-gdb/gdb/progspace.h:285:47: warning: '*((void*)(&
> maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace'
> may be used uninitialized in this function [-Wmaybe-uninitialized]
>    { set_current_program_space (m_saved_pspace); }
>                                                ^
> ../../binutils-gdb/gdb/infrun.c:931:6: note: '*((void*)(&
> maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace'
> was declared here
>       maybe_restore_inferior;
>       ^
>   CXX    inline-frame.o
> 
> Hope you devs can fix this. Thanks.
> 
> Asmwarrior

I also see this from time to time.  I think it is a false positive, but 
I may be wrong.  Do you see a code path that could actually be 
problematic?

Simon

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

[Pedro Alves]

On 10/04/2018 01:00 PM, Simon Marchi wrote:
> On 2018-10-04 02:28, asmwarrior wrote:
>> Hi, when building gdb git head(which is
>> 875e539851bb2702f3292f819e220545a8776242 as 2018-10-04) under msys+gcc
>> 5.4, I see such warning:
>>
>>   CXX    infrun.o
>> In file included from ../../binutils-gdb/gdb/inferior.h:49:0,
>>                  from ../../binutils-gdb/gdb/infrun.c:26:
>> ../../binutils-gdb/gdb/progspace.h: In function 'void
>> handle_vfork_child_exec_or_exit(int)':
>> ../../binutils-gdb/gdb/progspace.h:285:47: warning: '*((void*)(&
>> maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace'
>> may be used uninitialized in this function [-Wmaybe-uninitialized]
>>    { set_current_program_space (m_saved_pspace); }
>>                                                ^
>> ../../binutils-gdb/gdb/infrun.c:931:6: note: '*((void*)(&
>> maybe_restore_inferior)+16).scoped_restore_current_program_space::m_saved_pspace'
>> was declared here
>>       maybe_restore_inferior;
>>       ^
>>   CXX    inline-frame.o
>>
>> Hope you devs can fix this. Thanks.
>>
>> Asmwarrior
> 
> I also see this from time to time.  I think it is a false positive, but I may be wrong.  Do you see a code path that could actually be problematic?

That warning is documented as producing false positives.
And those preexisting warnings are hard/ugly to disable
with #pragma GCC diagnostic push/pop.
That's why we disable -Werror for that warning.

There's been mild talking about moving -Wmaybe-uninitialized out
of -Wall in gcc, which I think would make sense.  Over time,
GCC's VRP etc. technology will improve and those same bugs
will be warned by -Wuninitialized instead (I'd hope).

See:

 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

If the warnings confuse people too much, I'd be OK with
disabling -Wmaybe-uninitlized completely.  I left it as a
-Wno-error warning because even though it produces false positives,
it also helps catch bugs earlier in the compile-edit cycle,
when you're hacking some code, when you're introducing
uninitialized uses, and "make" ends up compiling just a few
files.

Thanks,
Pedro Alves

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

[Tom Tromey]

>>>>> "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).

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.

Tom

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

[Pedro Alves]

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

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

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> 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.

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

For gdb::optional, I think it would be good enough if we could simply
suppress the warning and make operator* assert that the object was
instantiated.  Perhaps std::optional could enforce this in debug mode as
well.

Tom

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

[Pedro Alves]

On 10/09/2018 08:33 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
>>> 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.
> 
> Pedro> Really not sure whether that is possible.  I think there's hope
> Pedro> that GCC value tracking becomes smart enough that these
> Pedro> std::optional-related warnings end up disappearing (which usually
> Pedro> means the code will optimize better too).  Fingers crossed, at least.
> 
> For gdb::optional, I think it would be good enough if we could simply
> suppress the warning and make operator* assert that the object was
> instantiated.  Perhaps std::optional could enforce this in debug mode as
> well.

Last I tried, I couldn't find a way to suppress the warning from
gdb::optional.  The warning triggers in code that belongs to
T in gdb::optional<T>.

Thanks,
Pedro Alves

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

[Pedro Alves]

On 10/09/2018 08:54 PM, Pedro Alves wrote:
> On 10/09/2018 08:33 PM, Tom Tromey wrote:
>>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>>
>>>> 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.
>>
>> Pedro> Really not sure whether that is possible.  I think there's hope
>> Pedro> that GCC value tracking becomes smart enough that these
>> Pedro> std::optional-related warnings end up disappearing (which usually
>> Pedro> means the code will optimize better too).  Fingers crossed, at least.
>>
>> For gdb::optional, I think it would be good enough if we could simply
>> suppress the warning and make operator* assert that the object was
>> instantiated.  Perhaps std::optional could enforce this in debug mode as
>> well.
> 
> Last I tried, I couldn't find a way to suppress the warning from
> gdb::optional.  The warning triggers in code that belongs to
> T in gdb::optional<T>.

Maybe we could make gdb::optional's ctor initialize the payload
only in devel mode, leave it uninitialized in release mode,
and add back -Werror for that warning.  We'd see the warnings in
release mode, but releases don't use -Werror so it's less of
an issue.

Doesn't really fix the issue though, just papers over it.
Not sure that really helps.

Thanks,
Pedro Alves

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

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Last I tried, I couldn't find a way to suppress the warning from
Pedro> gdb::optional.  The warning triggers in code that belongs to
Pedro> T in gdb::optional<T>.

I was hoping we could get a gcc extension for this -- say, some
attribute we could apply somewhere to suppress the warning.

Tom

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

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> I also see this from time to time.  I think it is a false positive,
Simon> but I may be wrong.  Do you see a code path that could actually be
Simon> problematic?

I think this is just that gcc doesn't know how to deal with
gdb::optional, and we aren't willing to add an initializer that would
slow down the generated code.  IIUC this is a problem with std::optional
as well.

Tom