Re: [PATCH][gdb/build] Fix Wmaybe-uninitialized in gdb_optional.h

[Simon Marchi <simark@simark.ca>]

On 2020-07-24 8:42 a.m., Tom de Vries wrote:
> Hi,
> 
> When building with CFLAGS/CXXFLAGS="-O2 -g -Wall", we run into:
> ...
> In file included from src/gdb/exceptions.h:23,
>                  from src/gdb/utils.h:24,
>                  from src/gdb/defs.h:630,
>                  from src/gdb/record-btrace.c:22:
> src/gdb/ui-out.h: In function 'void btrace_insn_history(ui_out*, \
>   const btrace_thread_info*, const btrace_insn_iterator*, \
>   const btrace_insn_iterator*, gdb_disassembly_flags)':
> src/gdb/ui-out.h:352:18: warning: \
>   'asm_list.ui_out_emit_type<ui_out_type_list>::m_uiout' may be used \
>   uninitialized in this function [-Wmaybe-uninitialized]
>   352 |     m_uiout->end (Type);
>       |     ~~~~~~~~~~~~~^~~~~~
> src/gdb/record-btrace.c:795:35: note: \
>   'asm_list.ui_out_emit_type<ui_out_type_list>::m_uiout' was declared here
>   795 |   gdb::optional<ui_out_emit_list> asm_list;
>       |                                   ^~~~~~~~
> ...
> 
> This is reported as PR gcc/80635 - "[8/9/10/11 regression] std::optional and
> bogus -Wmaybe-uninitialized warning".
> 
> Silence the warning by using the workaround suggested here (
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635#c53 ):
> ...
>    union
>    {
>      struct { } m_dummy;
>      T m_item;
> +    volatile char dont_use; // Silences -Wmaybe-uninitialized warning.
>    };
> ...
> 
> Build on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb/build] Fix Wmaybe-uninitialized in gdb_optional.h
> 
> gdbsupport/ChangeLog:
> 
> 2020-07-24  Tom de Vries  <tdevries@suse.de>
> 
> 	PR build/26281
> 	* gdb_optional.h (class optional): Add volatile member to union
> 	contaning m_dummy and m_item.
> 
> ---
>  gdbsupport/gdb_optional.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gdbsupport/gdb_optional.h b/gdbsupport/gdb_optional.h
> index 02a87f6ee4..221b4f75b7 100644
> --- a/gdbsupport/gdb_optional.h
> +++ b/gdbsupport/gdb_optional.h
> @@ -208,6 +208,7 @@ class optional
>    {
>      struct { } m_dummy;
>      T m_item;
> +    volatile char dont_use; // Silences -Wmaybe-uninitialized warning.
>    };
>  
>    /* True if the object was ever emplaced.  */
> 

This is fine with me, but I'd use /* */ style comments as usual, and maybe
point to the specific gcc PR in the comment, as you did in the other patch.
People can always use git blame to find it, but having it right there is handy.

Simon