[Bug build/26281] New: [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[Bug build/26281] New: [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

            Bug ID: 26281
           Summary: [Wmaybe-uninitialized] Warning in gdb/record-btrace.c
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: build
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

...
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;
      |                                   ^~~~~~~~
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Hmm, this looks related:
...
commit e13cb306f099a8cd450c9fba0dfa22521aaa3c95
Author: Pedro Alves <palves@redhat.com>
Date:   Fri May 5 01:03:28 2017 +0100

    gdb: Disable -Werror for -Wmaybe-uninitialized

    Newer GCCs are triggering false-positive -Wmaybe-uninitialized
    warnings around code that uses gdb::optional:
      https://sourceware.org/ml/gdb-patches/2017-05/msg00118.html

    Using std::optional wouldn't help, it triggers the same warnings:
      https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635

    Initializing the variables to quiet the warning would defeat the
    purpose of gdb::optional.  Making the optional ctor memset its storage
    would be a pessimization.  Wrapping gdb::optional's internals with
    "#pragma GCC diagnostic push/ignored/pop" doesn't work, we'd have to
    wrap uses of gdb::optional instead, which I think would get unwieldy
    and ugly as we start using gdb::optional more and more.

    The -Wmaybe-uninitialized warning is documented as producing false
    positives (unlike -Wuninialized), so until we find a better
    workaround, disable -Werror for this warning.  You'll still see the
    warning when building gdb, but it won't cause a build failure.
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at sourceware dot org

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
Yeah, this is the known std::optional problem.
One fix would be to memset the union, but I think
we prefer not to, since that's a pessimization.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
I presume you mean(In reply to Tom Tromey from comment #2)
> Yeah, this is the known std::optional problem.
> One fix would be to memset the union, but I think
> we prefer not to, since that's a pessimization.

This patch does the memset only for the variable causing the warning:
...
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 718de62f28..29076e3756 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -794,6 +794,9 @@ btrace_insn_history (struct ui_out *uiout,
   gdb::optional<ui_out_emit_tuple> src_and_asm_tuple;
   gdb::optional<ui_out_emit_list> asm_list;

+  /* Work around Wmaybe-uninitialized warning.  */
+  asm_list.clobber ();
+
   gdb_pretty_print_disassembler disasm (gdbarch, uiout);

   for (btrace_insn_iterator it = *begin; btrace_insn_cmp (&it, end) != 0;
diff --git a/gdbsupport/gdb_optional.h b/gdbsupport/gdb_optional.h
index 02a87f6ee4..f3fdf9c900 100644
--- a/gdbsupport/gdb_optional.h
+++ b/gdbsupport/gdb_optional.h
@@ -189,6 +189,10 @@ class optional
       this->destroy ();
   }

+  void clobber () {
+    memset ((void *)&m_item, 0, sizeof (T));
+  }
+
 private:

   /* Destroy the object.  */
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
I'm not sure how I feel about that patch.

It silences the bug but requires us to do something
that we ordinarily wouldn't do.

What about 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635#c53
instead?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #4)
> I'm not sure how I feel about that patch.
> 
> It silences the bug but requires us to do something
> that we ordinarily wouldn't do.
> 
> What about 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80635#c53
> instead?

That works:
...
diff --git a/gdbsupport/gdb_optional.h b/gdbsupport/gdb_optional.h
index 02a87f6ee4..851d7b2502 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.  */
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
Submitted patch:
https://sourceware.org/pipermail/gdb-patches/2020-July/170837.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=866b34a12df543caae10dd5a8de988e40ee012cd

commit 866b34a12df543caae10dd5a8de988e40ee012cd
Author: Tom de Vries <tdevries@suse.de>
Date:   Tue Jul 28 15:07:44 2020 +0200

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

    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.

    gdbsupport/ChangeLog:

    2020-07-28  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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug build/26281] [Wmaybe-uninitialized] Warning in gdb/record-btrace.c

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26281

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |10.1

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
Patch adding workaround committed, marking resolved-fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.