From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id x7R1MeESqGHvLwAAWB0awg (envelope-from ) for ; Wed, 01 Dec 2021 19:27:13 -0500 Received: by simark.ca (Postfix, from userid 112) id B93701F0CE; Wed, 1 Dec 2021 19:27:13 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 1F31F1EDF0 for ; Wed, 1 Dec 2021 19:27:13 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 978E3385803F for ; Thu, 2 Dec 2021 00:27:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 978E3385803F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1638404832; bh=RmIyK0eeqxncUobny4cf7bb5nMqv6lDyLr0/uXAqNnE=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ToptctQlaNy+raep1ufFj0Qo2lsLkAr04dO9Ouxw4tHEglFvPwntbNUXWl21kTi2L Kq16bGZiyEhn42yHEoGyG0hywAD65TGwTZuBTnHe5AHtqd5htYA0Tz+9NX9mgrcolA Pekj/D231TpqjrL6m2aSzvLpW+4KNeAzdIX1WJJ4= Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 0BEE53858414; Thu, 2 Dec 2021 00:25:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0BEE53858414 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 193641FDFE; Thu, 2 Dec 2021 00:25:52 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id EE7A013D84; Thu, 2 Dec 2021 00:25:51 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id /TMUOY8SqGEfQAAAMHmgww (envelope-from ); Thu, 02 Dec 2021 00:25:51 +0000 Subject: Re: [PATCH 2/2] include, gdb: fix -Wswitch build errors with gcc 4.8 To: Simon Marchi , gdb-patches@sourceware.org, binutils@sourceware.org References: <20211123211437.3783065-1-simon.marchi@efficios.com> <20211123211437.3783065-2-simon.marchi@efficios.com> Message-ID: <2278406c-c8cd-abf5-a0e5-69e367e3c385@suse.de> Date: Thu, 2 Dec 2021 01:25:51 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20211123211437.3783065-2-simon.marchi@efficios.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 11/23/21 10:14 PM, Simon Marchi via Gdb-patches wrote: > I started seeing these strange errors when building with gcc 4.8: > > CXX ada-tasks.o > /home/smarchi/src/binutils-gdb/gdb/ada-tasks.c: In function ‘void read_known_tasks()’: > /home/smarchi/src/binutils-gdb/gdb/ada-tasks.c:998:10: error: enumeration value ‘ADA_TASKS_UNKNOWN’ not handled in switch [-Werror=switch] > switch (data->known_tasks_kind) > ^ > > It is caused by commit 06de25b7af21 ("gdb: introduce > target_waitkind_str, use it in target_waitstatus::to_string"), which > introduced some pragmas to enable -Wswitch locally. That shouldn't > affect the rest of the code, since we are using push and pop around > that. It looks like gcc 4.8 (and 4.9)'s diagnostic push/pop is not > reliable, as it doesn't cause a problem with later versions. > > Work around this by not enabling -Wswitch for GCC < 5. This makes the > code a bit ugly, it would be nice to find a good way to factor this out, > especially if we want to re-use it elsewhere. But for now, I just want > to un-break the build. > > Note that this code (already as it exists in master today) enables > -Wswitch at the error level even if --disable-werror is passed. It > shouldn't be a problem, as it's not like a new enumerator will appear > out of nowhere. So it shouldn't cause a spurious build error in the > future. Still, for correctness, we would ideally want to ask the > compiler to enable -Wswitch at its default level (as if the user had > passed -Wswitch on the command-line). There doesn't seem to be a way to > do this. > > Change-Id: I50d66b348bf83de099c3c0879e2eb352d60540ba > --- > gdb/target/waitstatus.c | 9 ++++++++- > gdb/target/waitstatus.h | 9 ++++++++- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/gdb/target/waitstatus.c b/gdb/target/waitstatus.c > index 0fbcec5b7c8..e2388aa80b8 100644 > --- a/gdb/target/waitstatus.c > +++ b/gdb/target/waitstatus.c > @@ -29,9 +29,14 @@ target_waitstatus::to_string () const > ("status->kind = %s", target_waitkind_str (this->kind ())); > > /* Make sure the compiler warns if a new TARGET_WAITKIND enumerator is added > - but not handled here. */ > + but not handled here. > + > + GCC 4.8's "diagnostic push/pop" seems broken, it leaves -Werror=switch > + enabled after the pop. Skip it for GCC < 5. */ > DIAGNOSTIC_PUSH > +#if (defined(__GNUC__) && __GNUC__ >= 5) || defined(__clang__) > DIAGNOSTIC_ERROR_SWITCH > +#endif > switch (this->kind ()) > { > case TARGET_WAITKIND_EXITED: > @@ -63,7 +68,9 @@ DIAGNOSTIC_ERROR_SWITCH > case TARGET_WAITKIND_THREAD_CREATED: > return str; > } > +#if (defined(__GNUC__) && __GNUC__ >= 5) || defined(__clang__) > DIAGNOSTIC_POP > +#endif > It looks a bit funny to do the push unconditionally, and the pop conditionally. Is this not better fixed at the def site, once, rather than at two use sites? How about this instead: ... diff --git a/include/diagnostics.h b/include/diagnostics.h index 5ab43a85e9c..43f0458bd8c 100644 --- a/include/diagnostics.h +++ b/include/diagnostics.h @@ -79,8 +79,10 @@ # define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \ DIAGNOSTIC_IGNORE ("-Wformat-nonliteral") +#if __GNUC__ >= 5 # define DIAGNOSTIC_ERROR_SWITCH \ DIAGNOSTIC_ERROR ("-Wswitch") +#endif #endif ... ? Thanks, - Tom > gdb_assert_not_reached ("invalid target_waitkind value: %d", > (int) this->kind ()); > diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h > index 5b537354184..59014a37362 100644 > --- a/gdb/target/waitstatus.h > +++ b/gdb/target/waitstatus.h > @@ -108,9 +108,14 @@ static inline const char * > target_waitkind_str (target_waitkind kind) > { > /* Make sure the compiler warns if a new TARGET_WAITKIND enumerator is added > - but not handled here. */ > + but not handled here. > + > + GCC 4.8's "diagnostic push/pop" seems broken, it leaves -Werror=switch > + enabled after the pop. Skip it for GCC < 5. */ > DIAGNOSTIC_PUSH > +#if (defined(__GNUC__) && __GNUC__ >= 5) || defined(__clang__) > DIAGNOSTIC_ERROR_SWITCH > +#endif > switch (kind) > { > case TARGET_WAITKIND_EXITED: > @@ -146,7 +151,9 @@ DIAGNOSTIC_ERROR_SWITCH > case TARGET_WAITKIND_THREAD_EXITED: > return "THREAD_EXITED"; > }; > +#if (defined(__GNUC__) && __GNUC__ >= 5) || defined(__clang__) > DIAGNOSTIC_POP > +#endif > > gdb_assert_not_reached ("invalid target_waitkind value: %d\n", (int) kind); > } >