[PATCH 0/13] v2 warning control by group and location (PR 74765)

[Andrew Burgess]

* 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 <msebor@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,
Andrew