On Tue, Jul 9, 2019 at 10:04 PM Szabolcs Nagy <nsz@port70.net> wrote:
* 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__.

Sure, it's theoretically possible that some unknown compiler pretends to be GCC, but then implements __null incorrectly. But if it does, why does musl care? A conforming C++ implementation certainly requires a great many components to be implemented correctly, not least of which is the compiler.

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

The warning when you're using NULL as a "long" instead of a pointer is a very important warning -- so important it's on by default without any warning flags. It's _extremely_ surprising behavior for most developers that foo(NULL) can call a foo(long) overload in preference to a foo(int*) overload. This is why the compiler went to the trouble of implementing __null in the first place. It's only somewhat less important, now that 'nullptr' is becoming more prevalent.
 
> > 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++.

While it's not defined by C++, I do not believe there are any platforms GCC or Clang supports where passing __null to a varargs function instead of (void*)0 causes a problem. The intent is certainly that it's in-practice safe to use as a varargs-terminator, by being defined to be the correct size.
 
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.

There's always a trade-off between which warnings should be enabled by default or in -Wall. If this were an _actual_ codegen problem, for sure -Wstrict-null-sentinel should be enabled by default. However, as a very-common idiom which does not cause an actual problem, having this not emit a warning in the default flag sets seems justifiable.
 
(the warnings __null can provide are rarely time bombs)

Without the warning, developers may not realize that the wrong overload is being called. And maybe their software is working anyways, for whatever reason -- until someone converts NULL to nullptr, causing an unexpected behavior change.
 
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.

I think everyone agrees NULL is broken in C++, which is why we now have nullptr.