This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[pushed Initialize gdb::optional empty payload to quiet false -Wmaybe-uninitialized warnings (Re: Oh dear. I regret to inform you that commit 'dwarf2read.c: Make dir_index and file_name_index strong typedefs' might be unfortunate)


On 04/04/2017 10:49 PM, gdb-buildbot@sergiodj.net wrote:
> My lords, ladies, gentlemen, members of the public.

Fun!

I've pushed in the fix below.

>From c053b65441eb70ac78a514fabc3431b857a30d2e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 4 Apr 2017 23:49:27 +0100
Subject: [PATCH] Initialize gdb::optional empty payload to quiet false
 -Wmaybe-uninitialized warnings

Commit ecfb656c37b982 ("dwarf2read.c: Make dir_index and
file_name_index strong typedefs") added a use of gdb::optional that
triggers bogus -Wmaybe-uninitialized warnings:

GCC trunk is complaining like this:

  ../../binutils-gdb/gdb/dwarf2read.c: In function void read_formatted_entries(bfd*, const gdb_byte**, line_header*, const comp_unit_head*, void (*)(line_header*, const char*, dir_index, unsigned int, unsigned int)):
  ../../binutils-gdb/gdb/dwarf2read.c:17779:65: error: fe.file_entry::length may be used uninitialized in this function [-Werror=maybe-uninitialized]
	 callback (lh, fe.name, fe.d_index, fe.mod_time, fe.length);
								   ^
  ../../binutils-gdb/gdb/dwarf2read.c:17779:65: error: *((void*)& fe +8) may be used uninitialized in this function [-Werror=maybe-uninitialized]
  ../../binutils-gdb/gdb/dwarf2read.c:17779:65: error: fe.file_entry::mod_time may be used uninitialized in this function [-Werror=maybe-uninitialized]
  ../../binutils-gdb/gdb/dwarf2read.c:17779:65: error: fe.file_entry::name may be used uninitialized in this function [-Werror=maybe-uninitialized]

While some older GCCs are complaining like this:

  ../../binutils-gdb/gdb/dwarf2read.c: In function void read_formatted_entries(bfd*, const gdb_byte**, line_header*, const comp_unit_head*, void (*)(line_header*, const char*, dir_index, unsigned int, unsigned int)):
  ../../binutils-gdb/gdb/dwarf2read.c:17779:65: error: uint may be used uninitialized in this function [-Werror=maybe-uninitialized]
	 callback (lh, fe.name, fe.d_index, fe.mod_time, fe.length);

Looking around the web, I see that boost's optional implementation
triggers this kind of issue often too.  See:

  http://www.boost.org/doc/libs/1_63_0/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html

I noticed that replacing the gdb::optional uses with real C++17
std::optional uses against GCC 7/trunk makes the warnings go away.
Looking at the implementation, AFAICS, libstdc++ always initializes
its "empty" union payload member (_M_empty, which is defined as an
empty class, like ours).  I.e., all payload types have this ctor:

    struct _Optional_payload.....
    {
      constexpr _Optional_payload()
	: _M_empty() {}

The constexpr makes a diference too.  Without it, GCC7 still warns.

So I'm applying the same treatment to our gdb::optional.

gdb/ChangeLog:
2017-04-05  Pedro Alves  <palves@redhat.com>

	* common/gdb_optional.h (optional::optional): Make constexpr and
	initialize m_dummy.
---
 gdb/ChangeLog             | 5 +++++
 gdb/common/gdb_optional.h | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7fcfebb..93d20b5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-04-05  Pedro Alves  <palves@redhat.com>
+
+	* common/gdb_optional.h (optional::optional): Make constexpr and
+	initialize m_dummy.
+
 2017-04-04  John Baldwin  <jhb@FreeBSD.org>
 
 	* amd64-fbsd-tdep.c: Remove "bsd-uthread.h" include.
diff --git a/gdb/common/gdb_optional.h b/gdb/common/gdb_optional.h
index fef7a73..ad1119f 100644
--- a/gdb/common/gdb_optional.h
+++ b/gdb/common/gdb_optional.h
@@ -34,8 +34,9 @@ class optional
 {
 public:
 
-  optional ()
-    : m_instantiated (false)
+  constexpr optional ()
+    : m_dummy (),
+      m_instantiated (false)
   {
   }
 
-- 
2.5.5



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]