From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id C482E3840029 for ; Tue, 27 Jul 2021 10:44:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C482E3840029 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (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 EF5741FEFD; Tue, 27 Jul 2021 10:44:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1627382650; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hhP4gcAUFZjTwpZDeZVILUsul2z6fekdvVckni56YZg=; b=jO0Q0cptylQslENO77BqjA/POLQ8YfCuMEHZS2hcXmV8l8QGTTpH7typ+wOoku2BH7haGg 39+F6VnBrvP8I1jLCVwRAB6oFu9w2RwL5xFFJ3VffHI+HvfNVCa2PPGp3FpwdisTPia5LR 656KzEzJN8/YWZ9EPFssCvw4ohPrsns= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1627382650; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hhP4gcAUFZjTwpZDeZVILUsul2z6fekdvVckni56YZg=; b=vWepHy6tvh/PW0A2H/ainTH2za0HF1LgYd6DwURyFtDoKFptNZbGm4paYdTjkyEMAvMaf4 PK6uCyXqP8NuF+BQ== Received: from imap1.suse-dmz.suse.de (imap1.suse-dmz.suse.de [192.168.254.73]) (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 imap1.suse-dmz.suse.de (Postfix) with ESMTPS id D01BA13CF4; Tue, 27 Jul 2021 10:44:10 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id hHzMMHrj/2CaRwAAGKfGzw (envelope-from ); Tue, 27 Jul 2021 10:44:10 +0000 Subject: Re: Building with recent GCC versions: gdbsupport/gdb_assert.h:35:4: error: 'nonnull' argument 'filename' compared to NULL [-Werror=nonnull-compare] To: Andrew Burgess , Jan-Benedict Glaw Cc: gdb@sourceware.org References: <20210726211101.ivychvbfgaafxjtz@lug-owl.de> <20210727100354.GB4037238@embecosm.com> From: Tom de Vries Message-ID: Date: Tue, 27 Jul 2021 12:44:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210727100354.GB4037238@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_ASCII_DIVIDERS, KAM_SHORT, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Jul 2021 10:44:13 -0000 On 7/27/21 12:03 PM, Andrew Burgess wrote: > * Jan-Benedict Glaw [2021-07-26 23:11:01 +0200]: > >> Hi! >> >> I'm running some CI builds and noticed that, when building GDB with >> quite recent GCC versions, it breaks. >> >> With ie. this "gcc-snapshot" GCC from Debian: >> >> /usr/lib/gcc-snapshot/bin/gcc --version >> gcc (Debian 20210630-1) 12.0.0 20210630 (experimental) [master revision 6bf383c37e6:93c270320bb:35da8a98026849bd20d16bbf9210ac1d0b44ea6a] >> >> we see: >> >> ./configure --target=i686-linux --prefix=/tmp/gdb-i686-linux >> [...] >> all make V=1 all-gdb >> [...] >> [all 2021-07-26 20:39:22] /usr/lib/gcc-snapshot/bin/g++ -x c++ -I. -I. -I./config -DLOCALEDIR="\"/tmp/gdb-i686-linux/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../readline/readline/.. -I./../zlib -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./../gnulib/import -I../gnulib/import -I./.. -I.. -DTUI=1 -I./.. -pthread -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-variable -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-compare -Wno-error=maybe-uninitialized -Wno-mismatched-tags -Wsuggest-override -Wimplicit-fallthrough=3 -Wduplicated-cond -Wshadow=local -Wdeprecated-copy -Wdeprecated-copy-dtor -Wredundant-move -Wmissing-declarations -Wstrict-null-sentinel -Wformat -Wformat-nonliteral -Werror -g -O2 -c -o compile/compile.o -MT compile/compile.o -MMD -MP -MF compile/.deps/compile.Tpo compile/compile.c >> [all 2021-07-26 20:39:26] In file included from ./../gdbsupport/common-defs.h:126, >> [all 2021-07-26 20:39:26] from ./defs.h:28, >> [all 2021-07-26 20:39:26] from compile/compile.c:20: >> [all 2021-07-26 20:39:26] ./../gdbsupport/gdb_unlinker.h: In constructor 'gdb::unlinker::unlinker(const char*)': >> [all 2021-07-26 20:39:26] ./../gdbsupport/gdb_assert.h:35:4: error: 'nonnull' argument 'filename' compared to NULL [-Werror=nonnull-compare] >> [all 2021-07-26 20:39:26] 35 | ((void) ((expr) ? 0 : \ >> [all 2021-07-26 20:39:26] | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> [all 2021-07-26 20:39:26] 36 | (gdb_assert_fail (#expr, __FILE__, __LINE__, FUNCTION_NAME), 0))) >> [all 2021-07-26 20:39:26] | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> [all 2021-07-26 20:39:26] ./../gdbsupport/gdb_unlinker.h:38:5: note: in expansion of macro 'gdb_assert' >> [all 2021-07-26 20:39:26] 38 | gdb_assert (filename != NULL); >> [all 2021-07-26 20:39:26] | ^~~~~~~~~~ >> [all 2021-07-26 20:39:27] cc1plus: all warnings being treated as errors >> [all 2021-07-26 20:39:27] make[1]: *** [Makefile:1642: compile/compile.o] Error 1 >> [all 2021-07-26 20:39:27] make[1]: Leaving directory '/var/lib/laminar/run/gdb-i686-linux/4/binutils-gdb/gdb' >> [all 2021-07-26 20:39:27] make: *** [Makefile:11410: all-gdb] Error 2 >> >> >> I also discussed this on the GCC patches mailing list >> (https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575568.html), >> where Martin suggested that this should be fixed here in GDB. >> >> Any thoughts about this? > > As I understand it the nonnull attribute only provides compile time > protection against explicitly passing NULL, there's no compiled in > non-null check (well, maybe with -fisolate-erroneous-paths-attribute, > but the assert might give a better error message). > > This means its still possible to pass NULL to a nonnull function, its > just the behaviour of the program is undefined in that case. > > So, it doesn't seem crazy that we might want to both (a) have a > function declared nonnull, to prevent explicitly passing NULL, and (b) > have a NULL check inside the function to catch logic bugs that result > in NULL being passed. > > We could, of course, push the assert outside of the function, but that > would really suck due to code duplication, and the risk of missing an > assert, so that seems like a non-starter. > > We could drop either the assert, or the nonnull attribute, but that > would suck as both give a valuable, but different form of protection. > > After some experimenting, I suspect that the assert is being optimised > away anyway, which kind of makes sense, as we're telling the compiler > it can assume that the pointer is non-null. > Yes, in fact that's what the nonnull-compare warning specifically warns against: there's some code that may be optimized away, due to the nonnull attribute. > So, what we probably want is someway to tell (or trick) GCC into > including the null check even in the nonnull function.... > > ... here's what I came up with, add this somewhere: > > template > bool __attribute__ ((noinline)) > nonnull_arg_is_really_not_null (const T *ptr) > { > return ptr != nullptr; > } > > then change the assert to: > > gdb_assert (nonnull_arg_is_really_not_null (filename)); > > Seems to keep the assert, and silence the warning. Thoughts? > I understand why it works, but it seems fragile to me. At some point some compiler may get smart enough to also optimize this case, and then we're back in the same situation. I wonder whether using volatile is a better idea (can't try this out right now). Thanks, - Tom