* Martin Sebor <mse...@gmail.com> [2021-07-28 10:16:59 -0600]:
> On 7/28/21 5:14 AM, Andrew Burgess wrote:
> > * Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> [2021-07-19
> > 09:08:35 -0600]:
> >
> > > On 7/17/21 2:36 PM, Jan-Benedict Glaw wrote:
> > > > Hi Martin!
> > > >
> > > > On Fri, 2021-06-04 15:27:04 -0600, Martin Sebor <mse...@gmail.com>
> > > > wrote:
> > > > > This is a revised patch series to add warning control by group and
> > > > > location, updated based on feedback on the initial series.
> > > > [...]
> > > >
> > > > My automated checking (in this case: Using Debian's "gcc-snapshot"
> > > > package) indicates that between versions 1:20210527-1 and
> > > > 1:20210630-1, building GDB breaks. Your patch is a likely candidate.
> > > > It's a case where a method asks for a nonnull argument and later on
> > > > checks for NULLness again. The build log is currently available at
> > > > (http://wolf.lug-owl.de:8080/jobs/gdb-vax-linux/5), though obviously
> > > > breaks for any target:
> > > >
> > > > configure --target=vax-linux --prefix=/tmp/gdb-vax-linux
> > > > make all-gdb
> > > >
> > > > [...]
> > > > [all 2021-07-16 19:19:25] CXX compile/compile.o
> > > > [all 2021-07-16 19:19:30] In file included from
> > > > ./../gdbsupport/common-defs.h:126,
> > > > [all 2021-07-16 19:19:30] from ./defs.h:28,
> > > > [all 2021-07-16 19:19:30] from compile/compile.c:20:
> > > > [all 2021-07-16 19:19:30] ./../gdbsupport/gdb_unlinker.h: In
> > > > constructor 'gdb::unlinker::unlinker(const char*)':
> > > > [all 2021-07-16 19:19:30] ./../gdbsupport/gdb_assert.h:35:4: error:
> > > > 'nonnull' argument 'filename' compared to NULL [-Werror=nonnull-compare]
> > > > [all 2021-07-16 19:19:30] 35 | ((void) ((expr) ? 0 :
> > > > \
> > > > [all 2021-07-16 19:19:30] |
> > > > ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > [all 2021-07-16 19:19:30] 36 | (gdb_assert_fail (#expr,
> > > > __FILE__, __LINE__, FUNCTION_NAME), 0)))
> > > > [all 2021-07-16 19:19:30] |
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > [all 2021-07-16 19:19:30] ./../gdbsupport/gdb_unlinker.h:38:5: note: in
> > > > expansion of macro 'gdb_assert'
> > > > [all 2021-07-16 19:19:30] 38 | gdb_assert (filename != NULL);
> > > > [all 2021-07-16 19:19:30] | ^~~~~~~~~~
> > > > [all 2021-07-16 19:19:31] cc1plus: all warnings being treated as errors
> > > > [all 2021-07-16 19:19:31] make[1]: *** [Makefile:1641:
> > > > compile/compile.o] Error 1
> > > > [all 2021-07-16 19:19:31] make[1]: Leaving directory
> > > > '/var/lib/laminar/run/gdb-vax-linux/5/binutils-gdb/gdb'
> > > > [all 2021-07-16 19:19:31] make: *** [Makefile:11410: all-gdb] Error 2
> > > >
> > > >
> > > > Code is this:
> > > >
> > > > 31 class unlinker
> > > > 32 {
> > > > 33 public:
> > > > 34
> > > > 35 unlinker (const char *filename) ATTRIBUTE_NONNULL (2)
> > > > 36 : m_filename (filename)
> > > > 37 {
> > > > 38 gdb_assert (filename != NULL);
> > > > 39 }
> > > >
> > > > I'm quite undecided whether this is bad behavior of GCC or bad coding
> > > > style in Binutils/GDB, or both.
> > >
> > > A warning should be expected in this case. Before the recent GCC
> > > change it was inadvertently suppressed in gdb_assert macros by its
> > > operand being enclosed in parentheses.
> >
> > This issue was just posted to the GDB list, and I wanted to clarify my
> > understanding a bit.
> >
> > I believe that (at least by default) adding the nonnull attribute
> > allows GCC to assume (in the above case) that filename will not be
> > NULL and generate code accordingly.
> >
> > Additionally, passing an explicit NULL (i.e. 'unlinker obj (NULL)')
> > would cause a compile time error.
> >
> > But, there's nothing to actually stop a NULL being passed due to, say,
> > a logic bug in the program. So, something like this would compile
> > fine:
> >
> > extern const char *ptr;
> > unlinker obj (ptr);
> >
> > And in a separate compilation unit:
> >
> > const char *ptr = NULL;
> >
> > Obviously, the run time behaviour of such a program would be
> > undefined.
> >
> > Given the above then, it doesn't seem crazy to want to do something
> > like the above, that is, add an assert to catch a logic bug in the
> > program.
> >
> > Is there an approved mechanism through which I can tell GCC that I
> > really do want to do a comparison to NULL, without any warning, and
> > without the check being optimised out?
>
Thanks for your feedback.
> The manual says -fno-delete-null-pointer-checks is supposed to
> prevent the removal of the null function argument test so I'd
> expect adding attribute optimize ("no-delete-null-pointer-checks")
> to the definition of the function to have that effect but in my
> testing it didn't work (and didn't give a warning for the two
> attributes on the same declarataion). That seems worth filing
> a bug for.
I've since been pointed at this:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100404
Comment #3 of which discusses exactly this issue.
Thanks,
Andrew
