From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by sourceware.org (Postfix) with ESMTPS id 9F5A33858439 for ; Tue, 27 Jul 2021 11:35:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9F5A33858439 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wm1-x329.google.com with SMTP id h24-20020a1ccc180000b029022e0571d1a0so2101230wmb.5 for ; Tue, 27 Jul 2021 04:35:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=eRvcME354rT6vmXXv0Ty4kHqfMaDLNfOrciS41efkDM=; b=UIpN1mzplztAuNOkrgAmgNP9IohU4Z8+ufVYEiG/y4WVrb+4FhV+D7kY0cVPouLHlI zzvQlzI+hCSW7ZEGsIvPebOFvx9Gw0ELtvnnb7+gZ/hWE4kBk5+EtbVvGc/kfCwTmHIs DmweeGai+O3xwgnfyW1b+/Pt4Dgn9TtdTLTsz525m1NoAZf5GdE1zzt3DtMnA3M5sIXk zoNPNnX4Xe+O97/Uqep26N0AuoOIX2Jiv7oBumBsBNEqeuI4Mkjz3oiG02FbgWTX1XUk NRjU9NtJtGaE264YKOyjAQBKH36hUHKUXu6sD79yVmk40j02IzAf704g8zwH+EgcSw1p dPQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=eRvcME354rT6vmXXv0Ty4kHqfMaDLNfOrciS41efkDM=; b=uGBj2yF1OOIpE+7h5Wde0B46FM71bAVZIbI2DJquQjpQi11Tso4+TyEqPQiSXW1rZF AntRkkgcLXPFaEzAV4yQUkynV4M1UpLzs+FWkTf5TNoqua35gJ4icsoUUUDOFyFjHEwm 0xmBWvkmLVXlqaRhgY7ZjBnMs7py9RXURsf6NlrxA87e/GVIwQrLI0H0MPtpWwVWpTGl HbFBjuMQRoH0nFKlHB6KgkID0PL+adWZUNCh+BCHKhG8abJeGjZWThA2DTL92vovoBlw +mSativZ+y5aPbrrONa5fKuZtRrjFJ9qLI4/45q94tEXkPMAeT4kL4zExIdv7+kHqZfE giqw== X-Gm-Message-State: AOAM531muCZZgSBMHDhBqjJ7PnAhsLv7yCUmHfqDnNf2S5MGjfztk3UC AXtLREDJD1H4FqvI4B+JP4eznw== X-Google-Smtp-Source: ABdhPJzQHvJaz44dO9IiuokYpve+1QqxJ96I8J8q672h3ir6r+Hq+X4fMUkh0cInzZ5jwO4RHtwj8Q== X-Received: by 2002:a1c:980e:: with SMTP id a14mr529591wme.123.1627385713750; Tue, 27 Jul 2021 04:35:13 -0700 (PDT) Received: from localhost (host86-161-16-194.range86-161.btcentralplus.com. [86.161.16.194]) by smtp.gmail.com with ESMTPSA id f194sm2555822wmf.23.2021.07.27.04.35.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Jul 2021 04:35:13 -0700 (PDT) Date: Tue, 27 Jul 2021 12:35:11 +0100 From: Andrew Burgess To: Tom de Vries Cc: Jan-Benedict Glaw , gdb@sourceware.org Subject: Re: Building with recent GCC versions: gdbsupport/gdb_assert.h:35:4: error: 'nonnull' argument 'filename' compared to NULL [-Werror=nonnull-compare] Message-ID: <20210727113511.GC4037238@embecosm.com> References: <20210726211101.ivychvbfgaafxjtz@lug-owl.de> <20210727100354.GB4037238@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 12:31:46 up 10 days, 22:05, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, 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 11:35:18 -0000 * Tom de Vries [2021-07-27 12:44:10 +0200]: > 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. Good point. The GCC documentation for noinline[1] suggests we can avoid the call being removed by adding 'asm ("");' into the function: template bool __attribute__ ((noinline)) nonnull_arg_is_really_not_null (const T *ptr) { asm (""); return ptr != nullptr; } I'm not really arguing for this approach over any other, just sharing what I discovered. Thanks, Andrew [1] https://gcc.gnu.org/onlinedocs/gcc-11.1.0/gcc/Common-Function-Attributes.html#Common-Function-Attributes > > I wonder whether using volatile is a better idea (can't try this out > right now). > > Thanks, > - Tom