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 0B80B3858439 for ; Tue, 27 Jul 2021 11:49:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0B80B3858439 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 1E8F820107; Tue, 27 Jul 2021 11:49:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1627386555; 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=vMz687NJ1QPK3zXiiHP9Nt7cMEYvhGTK535y1mlXvVg=; b=VHgEg/EHuAVhL0Vc71dnhvUx4RP9EX3sJ0gI34eDRXHBj4hPeG3hsr1DjdCnOtEjro4+cd TxntjrPRWjX34gP9HrrICa5vYxPMDGn/ynSRaWTmrLxVUohL0lgU8rVbDhjb6rNEHr5y1N qxor0osoCQ5jWT1c7pbHGqzO57dLXxE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1627386555; 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=vMz687NJ1QPK3zXiiHP9Nt7cMEYvhGTK535y1mlXvVg=; b=G/yCTkTms5sTEG4AW6WE2/VPhoYqt1QrSmDb/m77QMFQ4RGhoMOIiyNmBCyiN8vTvp3P2k fSxj7/fyVdJwmhCA== 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 EC811133DE; Tue, 27 Jul 2021 11:49:14 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap1.suse-dmz.suse.de with ESMTPSA id o/EaOLry/2CEWgAAGKfGzw (envelope-from ); Tue, 27 Jul 2021 11:49:14 +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 Cc: Jan-Benedict Glaw , gdb@sourceware.org References: <20210726211101.ivychvbfgaafxjtz@lug-owl.de> <20210727100354.GB4037238@embecosm.com> <20210727113511.GC4037238@embecosm.com> From: Tom de Vries Message-ID: <6cf80ba9-b010-bb42-c92d-84e4f396813c@suse.de> Date: Tue, 27 Jul 2021 13:49:14 +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: <20210727113511.GC4037238@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.6 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 11:49:18 -0000 On 7/27/21 1:35 PM, Andrew Burgess wrote: > * 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. > Ack, understood. Note that the added asm doesn't stop a compiler from doing: ... gdb_assert (nonnull_arg_is_really_not_null (filename)); ... -> ... nonnull_arg_is_really_not_null (filename); gdb_assert (true); ... Thanks, - Tom > 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