From patchwork Thu May 4 18:22:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 20280 Received: (qmail 1269 invoked by alias); 4 May 2017 18:23:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 1216 invoked by uid 89); 4 May 2017 18:23:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy=H*r:192.168.43, folks, regret X-HELO: mail-wm0-f52.google.com Received: from mail-wm0-f52.google.com (HELO mail-wm0-f52.google.com) (74.125.82.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 04 May 2017 18:23:03 +0000 Received: by mail-wm0-f52.google.com with SMTP id u65so4514634wmu.1 for ; Thu, 04 May 2017 11:23:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=ESDbEi+Jf84plkukgbSpDCaFraBoKw1p5kODUXlN3aw=; b=Fp320lGN4pEYvjizIfynuiCTP1gbcbF2TmKdH2fr2TaZjCVAcZUq3/UUdP01wbKcig CbCw7x1/7u3e4FihkKxdYndmFzYcfu0s7h/5/iVEta8uVDbVJJdU/PMgI56lpwAsXdHJ wkO3qSaUVI1xaPz6pxtD//mgDK4WT34htC+bnZjn73jmvXZKHOeTNCzqmDgZujOVenbk iD7bz2y3xlj8KIOFOUWMjRasA+iw57SrP50i87ByiJp/OsMH99zrr3XiHB3jdU5roun6 XxVioTH/NX9UBEyz2lZRNuS3bjZHp3we6LKn80gRzk7/JPlg3Q26yv6GDApMgsOcyyDQ bByg== X-Gm-Message-State: AN3rC/7EzNHhA71Aq8WD16gL5bjFUmRhqaGf/67OgnQp1KAE7OdZLpZT 2V8ACmXbPsUjsrLx1RJgq242 X-Received: by 10.28.91.77 with SMTP id p74mr2785087wmb.54.1493922183894; Thu, 04 May 2017 11:23:03 -0700 (PDT) Received: from [192.168.43.236] ([95.69.93.66]) by smtp.gmail.com with ESMTPSA id t85sm2368373wmt.23.2017.05.04.11.23.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 May 2017 11:23:02 -0700 (PDT) Subject: Re: Oh dear. I regret to inform you that commit 'RAII-fy make_cleanup_restore_current_thread & friends' might be unfortunate To: Simon Marchi References: <34792dd0-088c-a1d1-9125-70c8585c21bd@redhat.com> <6bf88edee0fb17451d44b85bb00fb0d0@polymtl.ca> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <79511435-a3a8-48f9-2e16-bca8adb1909d@redhat.com> Date: Thu, 4 May 2017 19:22:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <6bf88edee0fb17451d44b85bb00fb0d0@polymtl.ca> On 05/04/2017 07:15 PM, Simon Marchi wrote: > On 2017-05-04 12:42, Pedro Alves wrote: >> On 05/04/2017 05:06 PM, gdb-buildbot@sergiodj.net wrote: >>> g++ -g -O2 -I. -I../../binutils-gdb/gdb >>> -I../../binutils-gdb/gdb/common -I../../binutils-gdb/gdb/config >>> -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H >>> -I../../binutils-gdb/gdb/../include/opcode >>> -I../../binutils-gdb/gdb/../opcodes/.. >>> -I../../binutils-gdb/gdb/../readline/.. >>> -I../../binutils-gdb/gdb/../zlib -I../bfd >>> -I../../binutils-gdb/gdb/../bfd -I../../binutils-gdb/gdb/../include >>> -I../libdecnumber -I../../binutils-gdb/gdb/../libdecnumber >>> -I../../binutils-gdb/gdb/gnulib/import -Ibuild-gnulib/import >>> -DTUI=1 -pthread -I/usr/include/guile/2.0 -I/usr/include/python2.7 >>> -I/usr/include/python2.7 -Wall -Wpointer-arith -Wno-unused >>> -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts >>> -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable >>> -Wno-sign-compare -Wno-narrowing -Wformat-nonliteral -Werror -c -o >>> interps.o -MT interps.o -MMD -MP -MF .deps/interps.Tpo >>> ../../binutils-gdb/gdb/interps.c >>> In file included from ../../binutils-gdb/gdb/infrun.c:26:0: >>> ../../binutils-gdb/gdb/inferior.h: In function void >>> handle_vfork_child_exec_or_exit(int): >>> ../../binutils-gdb/gdb/inferior.h:553:39: error: *((void*)(& >>> maybe_restore_inferior)+40).scoped_restore_current_inferior::m_saved_inf >>> may be used uninitialized in this function [-Werror=maybe-uninitialized] >>> { set_current_inferior (m_saved_inf); } >>> ^ >>> ../../binutils-gdb/gdb/infrun.c:940:6: note: *((void*)(& >>> maybe_restore_inferior)+40).scoped_restore_current_inferior::m_saved_inf >>> was declared here >>> maybe_restore_inferior; >>> ^~~~~~~~~~~~~~~~~~~~~~ >>> In file included from ../../binutils-gdb/gdb/inferior.h:46:0, >>> from ../../binutils-gdb/gdb/infrun.c:26: >>> ../../binutils-gdb/gdb/progspace.h:274:47: error: *((void*)(& >>> maybe_restore_inferior)+32).scoped_restore_current_program_space::m_saved_pspace >>> may be used uninitialized in this function [-Werror=maybe-uninitialized] >>> { set_current_program_space (m_saved_pspace); } >>> ^ >>> ../../binutils-gdb/gdb/infrun.c:940:6: note: *((void*)(& >>> maybe_restore_inferior)+32).scoped_restore_current_program_space::m_saved_pspace >>> was declared here >>> maybe_restore_inferior; >>> ^~~~~~~~~~~~~~~~~~~~~~ >> >> Fun, looks like gdb::optional related ... >> >> Thanks, >> Pedro Alves > > What do you need to reproduce this? GCC 7? The above is g++ 6.3.1, if I can trust the buildslave description page. I didn't try that release, but gcc master warns the same way. And, if I hack the code to use std::optional instead of gdb::optional : and compile with -std=gnu++17, I get the exact same warning again... src/gdb/infrun.c In file included from /home/pedro/gdb/mygit/cxx-convertion/src/gdb/infrun.c:26:0: /home/pedro/gdb/mygit/cxx-convertion/src/gdb/inferior.h: In function ‘void handle_vfork_child_exec_or_exit(int)’: /home/pedro/gdb/mygit/cxx-convertion/src/gdb/inferior.h:553:26: error: ‘*((void*)(& maybe_restore_inferior)+40).scoped_restore_current_inferior::m_saved_inf’ may be used uninitialized in this function [-Werror=maybe-uninitialized] { set_current_inferior (m_saved_inf); } ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~ /home/pedro/gdb/mygit/cxx-convertion/src/gdb/infrun.c:942:6: note: ‘*((void*)(& maybe_restore_inferior)+40).scoped_restore_current_inferior::m_saved_inf’ was declared here maybe_restore_inferior; ^~~~~~~~~~~~~~~~~~~~~~ In file included from /home/pedro/gdb/mygit/cxx-convertion/src/gdb/inferior.h:46:0, from /home/pedro/gdb/mygit/cxx-convertion/src/gdb/infrun.c:26: /home/pedro/gdb/mygit/cxx-convertion/src/gdb/progspace.h:274:31: error: ‘*((void*)(& maybe_restore_inferior)+32).scoped_restore_current_program_space::m_saved_pspace’ may be used uninitialized in this function [-Werror=maybe-uninitialized] { set_current_program_space (m_saved_pspace); } ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~ /home/pedro/gdb/mygit/cxx-convertion/src/gdb/infrun.c:942:6: note: ‘*((void*)(& maybe_restore_inferior)+32).scoped_restore_current_program_space::m_saved_pspace’ was declared here maybe_restore_inferior; ^~~~~~~~~~~~~~~~~~~~~~ cc1plus: all warnings being treated as errors AFAICS so far, this is a false positive. Not sure what to do. I wouldn't want to force-memset the optional's storage to work around it, which would be a pessimization to quiet a warning. From above, we see that that wouldn't work when we later start using std::optional. There's a bug open about this (for boost::optional, but most probably the exact same): https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78044 Comment #2 makes me think that we should really disable the warning, or at least make it "-Wno-error=maybe-uninitialized". I've seen other similar comments around the interwebs when looking for this warning + optional. Maybe I could check if the gcc folks plan on doing something to std::optional to work around this. Thanks, Pedro Alves diff --git c/gdb/infrun.c w/gdb/infrun.c index d0504de..11196ee 100644 --- c/gdb/infrun.c +++ w/gdb/infrun.c @@ -910,6 +910,8 @@ private: scoped_restore_current_inferior m_inferior; }; +#include + /* Called whenever we notice an exec or exit event, to handle detaching or resuming a vfork parent. */ @@ -936,9 +938,9 @@ handle_vfork_child_exec_or_exit (int exec) inf->vfork_parent->pending_detach = 0; - gdb::optional + std::optional maybe_restore_inferior; - gdb::optional + std::optional maybe_restore_thread; /* If we're handling a child exit, then inferior_ptid points