From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id 50AC93850427 for ; Tue, 27 Jul 2021 10:03:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 50AC93850427 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-wr1-x429.google.com with SMTP id b7so14492138wri.8 for ; Tue, 27 Jul 2021 03:03:57 -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=xrdvwFRcVl4a7K9i+kRcs4gHd6ltF1fV1tXjRaBcMIg=; b=AEsn/Lnb4TTEr1ABFewCJIzMsGf4t2ml5+CT2GqbGx3NQNfjy1R6mCx112T87WqK01 Kx7YehgP0SYWH9S3nOqull0gnqHaxWTsbebQSlPyrLvrrOZgz6uAN5Mlzlf/naK75sEy RGhMyHONl6F/+R2oLYiB3ytlxNx+YEb7WKQaaPC3j/ma5WwG2FbqO75BTmHgpvsNEpKe IZgnUKsijewl7o+FqqlV758qrdOAPNrMzdP9JKAaFyIMP11V6IJwKe6+h7XRwsy/TfZJ ULCEGGGblUHhAA/5piLISxXu00J2sD+Vq9AP+IT+co4PwCAxdn2Xosc4uHyB+auaxZTL OoJg== 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=xrdvwFRcVl4a7K9i+kRcs4gHd6ltF1fV1tXjRaBcMIg=; b=iUmMEQ6o0+6fTniCjj7y/g/gom2fCj+N8cQdpB6xCrvCcaNlzv44L9tD4Q+1btroKD tL8BwDM6CR8RUzvFa1MeAkgrUZUMs5E0XGqgNuJXWu/ZPNSpBFLzqAdxFvJzj2wMYE3F bigawPCB1tRQ+DW9T2krC+Q0T19H0jizG8TVwNbvqvRCx9Jmrc0OIp3kX4e/nXGVFHCd 7QWfRBdLYXAIzDHsy5qvtHL9zLL3IN9Oz9B5CNP809ZehTgZnP3dqZO/8T4Tvsdgxid3 uigU51Jazzvp8IxyWPfAJlgHgoBGBHnZ3Zy65hGpdupRdIx83CtUQkaBId/GxJ3L2sjR S4HA== X-Gm-Message-State: AOAM532kzBuGLi/5F/9DoRi7vWd/NW/AlZZq/h8iOKfraX+iawYhLjL2 xnexkIFx142XSRQLk46pogQK1Q== X-Google-Smtp-Source: ABdhPJyKTajxeRvIv4J+wItBfW7pKMIxM+wgNt0YN2XAbLn+TYAffU0V5DdFKwQeDS99eZ3H8O0CWg== X-Received: by 2002:adf:ed45:: with SMTP id u5mr7678865wro.203.1627380236383; Tue, 27 Jul 2021 03:03:56 -0700 (PDT) Received: from localhost (host86-161-16-194.range86-161.btcentralplus.com. [86.161.16.194]) by smtp.gmail.com with ESMTPSA id z6sm2598895wmp.1.2021.07.27.03.03.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Jul 2021 03:03:56 -0700 (PDT) Date: Tue, 27 Jul 2021 11:03:54 +0100 From: Andrew Burgess To: Jan-Benedict Glaw Cc: 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: <20210727100354.GB4037238@embecosm.com> References: <20210726211101.ivychvbfgaafxjtz@lug-owl.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210726211101.ivychvbfgaafxjtz@lug-owl.de> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 09:40:53 up 10 days, 19:14, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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:03:59 -0000 * 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. 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? Thanks, Andrew --- diff --git a/gdbsupport/gdb_assert.h b/gdbsupport/gdb_assert.h index 00553a78613..478bf10ec24 100644 --- a/gdbsupport/gdb_assert.h +++ b/gdbsupport/gdb_assert.h @@ -58,4 +58,26 @@ internal_error (__FILE__, __LINE__, _(message)) #endif +/* If a pointer argument to a function is marked as nonnull (using the + nonnull attribute) then GCC will warn about any attempts to compare the + pointer to nullptr. Even if you can silence the warning GCC will + likely optimize out the check (and any associated code block) + completely. + + Normally this would be what you want, but, marking an argument nonnull + doesn't guarantee that nullptr can't be passed. So it's not + unreasonable that we might want to add an assert that the argument is + not nullptr. + + This function should be used for this purpose, like: + + gdb_assert (nonnull_arg_is_not_nullptr (arg_name)); */ + +template +bool _GL_ATTRIBUTE_NOINLINE +nonnull_arg_is_not_nullptr (const T *ptr) +{ + return ptr != nullptr; +} + #endif /* COMMON_GDB_ASSERT_H */ diff --git a/gdbsupport/gdb_unlinker.h b/gdbsupport/gdb_unlinker.h index bda6fe7ab54..ec159c2166a 100644 --- a/gdbsupport/gdb_unlinker.h +++ b/gdbsupport/gdb_unlinker.h @@ -35,7 +35,7 @@ class unlinker unlinker (const char *filename) ATTRIBUTE_NONNULL (2) : m_filename (filename) { - gdb_assert (filename != NULL); + gdb_assert (nonnull_arg_is_not_nullptr (filename)); } ~unlinker ()