Re: [PATCH] Define NULL as __null in C++ mode when using GCC or Clang.

[Szabolcs Nagy <nsz@port70.net>]

* James Y Knight <jyknight@google.com> [2019-07-09 19:04:13 -0400]:
> On Tue, Jul 9, 2019 at 3:38 PM Rich Felker <dalias@libc.org> wrote:
> > On Tue, Jul 09, 2019 at 03:19:00PM -0400, James Y Knight wrote:
> > > Both GCC and Clang ship their own stddef.h which does this (musl's
> > > stddef.h is simply ignored). But, musl also defines the macro in a
> > > number of other headers.
> >
> > This was intentional at the time it was done. At least in the present
> > version of C++ at the time, __null was not a conforming definition.
> > For example, conforming applications could parse the stringification
> > of NULL and expect to get something that's a valid null pointer
> > constant. This could possibly be revisited, if it's changed, and if it
> > can be done in a way that doesn't involve compiler-specific stuff.
> >
> 
> The C++11 standard simply says "The macro NULL is an implementation-defined
> C++ null pointer constant in this International Standard." (and links to
> the definition of null pointer constant -- "A null pointer constant is an
> integral constant expression (5.19) prvalue of integer type that evaluates
> to zero or a prvalue of type std::nullptr_t").
> 
> If the implementation defines __null to be a null pointer constant, which
> is an integral constant expression of integer type that evaluates to zero
> (which it does), that seems entirely valid. I see nothing that restricts
> what it may stringify to.

it is clear that 0L is a conforming definition for all
conforming c++ compilers.

it is less clear if __null is so in all compilers that
define __GNUC__.

so normally one would expect a compelling reason to do
such change.

> > Mostly, getting musl's definition simply degrades warning diagnostics
> 
> > in C++ slightly -- e.g. GCC can no longer emit this warning:
> > >   warning: passing NULL to non-pointer argument 1 of 'int foo(long int)'
> > > [-Wconversion-null]
> >
> > The current definition however catches bugs where NULL is wrongly used
> > to terminate a variadic argument list where (void*)0 or (char*)0 is
> > actually needed. See commits 41d7c77d6a2e74294807d35062e4cd1d48ab72d3
> > and c8a9c22173f485c8c053709e1dfa0a617cb6be1a. So both choices are a
> > tradeoff in diagnostic capability.
> 
> 
> Ehh. Actually musl's definition actually breaks the intended semantics of
> that warning as well. The following program:
> #include <unistd.h>
> int main() {
>   execl("foo", "bar", NULL);
> }
> does indeed not warn with "g++ -Wall foo.cc" when NULL is __null, and it
> does warn if NULL is 0L.
> 
> However, it was an intentional choice, not an accident, to suppress the
> warning in the former case. This kind of usage is widespread,, and in
> practice it does not trigger buggy behavior (because __null is guaranteed
> to be the same size as a pointer). GCC does have a separate
> "-Wstrict-null-sentinel" warning flag, if you want to be more pedantic
> about this not-standards-compliant code.

if a variadic function implementation calls va_arg(ap,char*)
on a NULL argument then the behaviour is undefined in c++,
but it is well-defined in posix! so the pattern is legitimately
widespread but users often mistakenly compile c code as c++.

how can we ensure that an optimizing c++ compiler won't
silently break such code? gcc rutinely eliminates code if
it can spot that a null pointer is passed to a library
function where that is undefined.

not diagnosing such correctness issues when you can is bad
practice: it leaves around ticking time bombs. (the warnings
__null can provide are rarely time bombs)

but i think NULL is dangerously broken in c++ either way,
so both the current and the proposed definition is fine by me,
i just slightly prefer the current definition.