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

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

[James Y Knight]

[-- Attachment #1.1: Type: text/plain, Size: 958 bytes --]

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. Thus, depending on which header you include
last, you'll end up with a different definition of NULL.

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]

If you're using Clang's modules support, it can also break
compilation. In that case, the conflicting definitions may be detected
as a module incompatibility.

A different (potentially better) fix would be to always retrieve the
definition of NULL from the compiler's stddef.h (via #define
__need_NULL #include <stddef.h>). It may also be best to delete the
musl stddef.h entirely for clarity, since it's currently ignored,
anyhow.

But, this seemed the more minimal fix.

[-- Attachment #1.2: Type: text/html, Size: 1084 bytes --]

[-- Attachment #2: 0001-Define-NULL-as-__null-in-C-mode-when-using-GCC-or-Cl.patch --]
[-- Type: text/x-patch, Size: 3931 bytes --]

From 3d898d4825c28f08ade92c40822fa5bfa2ef1f1f Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight@google.com>
Date: Tue, 9 Jul 2019 15:03:03 -0400
Subject: [PATCH] Define NULL as __null in C++ mode when using GCC or Clang.

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. Thus, depending on which header you include
last, you'll end up with a different definition of NULL.

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]

If you're using Clang's modules support, it can also break
compilation. In that case, the conflicting definitions may be detected
as a module incompatibility.

A different (potentially better) fix would be to always retrieve the
definition of NULL from the compiler's stddef.h (via #define
__need_NULL #include <stddef.h>). It may also be best to delete the
musl stddef.h entirely for clarity, since it's currently ignored,
anyhow.

But, this seemed the more minimal fix.
---
 include/locale.h | 4 ++++
 include/stddef.h | 4 ++++
 include/stdio.h  | 4 ++++
 include/stdlib.h | 4 ++++
 include/string.h | 4 ++++
 include/time.h   | 4 ++++
 include/unistd.h | 4 ++++
 include/wchar.h  | 4 ++++
 8 files changed, 32 insertions(+)

diff --git a/include/locale.h b/include/locale.h
index ce384381..80c2d2db 100644
--- a/include/locale.h
+++ b/include/locale.h
@@ -8,7 +8,11 @@ extern "C" {
 #include <features.h>
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
diff --git a/include/stddef.h b/include/stddef.h
index bd753853..415a2d91 100644
--- a/include/stddef.h
+++ b/include/stddef.h
@@ -2,7 +2,11 @@
 #define _STDDEF_H
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
diff --git a/include/stdio.h b/include/stdio.h
index 3604198c..9e30d1ad 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -26,7 +26,11 @@ extern "C" {
 #include <bits/alltypes.h>
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
diff --git a/include/stdlib.h b/include/stdlib.h
index 42ca8336..a272a4f4 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -8,7 +8,11 @@ extern "C" {
 #include <features.h>
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
diff --git a/include/string.h b/include/string.h
index 795a2abc..4d344d72 100644
--- a/include/string.h
+++ b/include/string.h
@@ -8,7 +8,11 @@ extern "C" {
 #include <features.h>
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
diff --git a/include/time.h b/include/time.h
index 672b3fc3..0eec373c 100644
--- a/include/time.h
+++ b/include/time.h
@@ -8,7 +8,11 @@ extern "C" {
 #include <features.h>
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
diff --git a/include/unistd.h b/include/unistd.h
index 9485da7a..391b58ba 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -16,7 +16,11 @@ extern "C" {
 #define SEEK_END 2
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
diff --git a/include/wchar.h b/include/wchar.h
index 88eb55b1..bfdbaebb 100644
--- a/include/wchar.h
+++ b/include/wchar.h
@@ -39,7 +39,11 @@ extern "C" {
 #endif
 
 #ifdef __cplusplus
+#ifdef __GNUC__
+#define NULL __null
+#else
 #define NULL 0L
+#endif
 #else
 #define NULL ((void*)0)
 #endif
-- 
2.22.0.410.gd8fdbe21b5-goog

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

[Rich Felker]

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.

> Thus, depending on which header you include
> last, you'll end up with a different definition of NULL.

musl doesn't support use of the compiler versions of these headers,
for several reasons that have nothing to do with NULL.

> 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.

> If you're using Clang's modules support, it can also break
> compilation. In that case, the conflicting definitions may be detected
> as a module incompatibility.

I'm not sure what this means. The conflicting definition should never
appear in code that's consistent with compiling and linking with musl.

> A different (potentially better) fix would be to always retrieve the
> definition of NULL from the compiler's stddef.h (via #define
> __need_NULL #include <stddef.h>). It may also be best to delete the
> musl stddef.h entirely for clarity, since it's currently ignored,
> anyhow.

We intentionally don't do things that way.

Rich


> But, this seemed the more minimal fix.

> From 3d898d4825c28f08ade92c40822fa5bfa2ef1f1f Mon Sep 17 00:00:00 2001
> From: James Y Knight <jyknight@google.com>
> Date: Tue, 9 Jul 2019 15:03:03 -0400
> Subject: [PATCH] Define NULL as __null in C++ mode when using GCC or Clang.
> 
> 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. Thus, depending on which header you include
> last, you'll end up with a different definition of NULL.
> 
> 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]
> 
> If you're using Clang's modules support, it can also break
> compilation. In that case, the conflicting definitions may be detected
> as a module incompatibility.
> 
> A different (potentially better) fix would be to always retrieve the
> definition of NULL from the compiler's stddef.h (via #define
> __need_NULL #include <stddef.h>). It may also be best to delete the
> musl stddef.h entirely for clarity, since it's currently ignored,
> anyhow.
> 
> But, this seemed the more minimal fix.
> ---
>  include/locale.h | 4 ++++
>  include/stddef.h | 4 ++++
>  include/stdio.h  | 4 ++++
>  include/stdlib.h | 4 ++++
>  include/string.h | 4 ++++
>  include/time.h   | 4 ++++
>  include/unistd.h | 4 ++++
>  include/wchar.h  | 4 ++++
>  8 files changed, 32 insertions(+)
> 
> diff --git a/include/locale.h b/include/locale.h
> index ce384381..80c2d2db 100644
> --- a/include/locale.h
> +++ b/include/locale.h
> @@ -8,7 +8,11 @@ extern "C" {
>  #include <features.h>
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> diff --git a/include/stddef.h b/include/stddef.h
> index bd753853..415a2d91 100644
> --- a/include/stddef.h
> +++ b/include/stddef.h
> @@ -2,7 +2,11 @@
>  #define _STDDEF_H
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> diff --git a/include/stdio.h b/include/stdio.h
> index 3604198c..9e30d1ad 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -26,7 +26,11 @@ extern "C" {
>  #include <bits/alltypes.h>
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> diff --git a/include/stdlib.h b/include/stdlib.h
> index 42ca8336..a272a4f4 100644
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -8,7 +8,11 @@ extern "C" {
>  #include <features.h>
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> diff --git a/include/string.h b/include/string.h
> index 795a2abc..4d344d72 100644
> --- a/include/string.h
> +++ b/include/string.h
> @@ -8,7 +8,11 @@ extern "C" {
>  #include <features.h>
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> diff --git a/include/time.h b/include/time.h
> index 672b3fc3..0eec373c 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -8,7 +8,11 @@ extern "C" {
>  #include <features.h>
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> diff --git a/include/unistd.h b/include/unistd.h
> index 9485da7a..391b58ba 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -16,7 +16,11 @@ extern "C" {
>  #define SEEK_END 2
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> diff --git a/include/wchar.h b/include/wchar.h
> index 88eb55b1..bfdbaebb 100644
> --- a/include/wchar.h
> +++ b/include/wchar.h
> @@ -39,7 +39,11 @@ extern "C" {
>  #endif
>  
>  #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
>  #define NULL 0L
> +#endif
>  #else
>  #define NULL ((void*)0)
>  #endif
> -- 
> 2.22.0.410.gd8fdbe21b5-goog
> 

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

[James Y Knight]

[-- Attachment #1: Type: text/plain, Size: 4897 bytes --]

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.

> Thus, depending on which header you include
> > last, you'll end up with a different definition of NULL.
>
> musl doesn't support use of the compiler versions of these headers,
> for several reasons that have nothing to do with NULL


That's rather non-obvious. Usually, the compiler's builtin headers are
intended to be before libc in the search path. Invoking the compiler with
--sysroot= pointed to a musl install directory results in that ordering. I
would've expected that the behavior you get from `clang --sysroot=X -target
x86_64-linux-musl` should actually work -- and, well, it does indeed appear
to work -- but uses the compiler's stddef.h.

I'm also not really sure what the value is for musl to provide its own
stddef.h -- can you be more specific as to why it's bad to use the
compiler's copy? The compiler already has to know the correct types for
ptrdiff_t, size_t, and wchar_t internally, whether or not you use its
stddef.h. Having the compiler also be responsible for exposing those names
seems like a good way to ensure that they're defined in a consistent manner.

Of course, as long as musl does define them to the same types, it's also
fine for musl to provide the header...but it seems less than ideal for the
compiler and the libc to be fighting about who should be providing the
header.

> 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 you're using Clang's modules support, it can also break
>
> compilation. In that case, the conflicting definitions may be detected
> > as a module incompatibility.
>
> I'm not sure what this means. The conflicting definition should never
> appear in code that's consistent with compiling and linking with musl


The issue occurs when the compiler's stddef.h and musl's locale.h (or other
such headers) are both used -- then you can observe conflicting definitions
within one translation unit, which is what can trigger the issue. (All the
details of what "modules' is, and when and why such conflicting definitions
can trigger an error probably isn't really important here).


> > A different (potentially better) fix would be to always retrieve the
> > definition of NULL from the compiler's stddef.h (via #define
> > __need_NULL #include <stddef.h>). It may also be best to delete the
> > musl stddef.h entirely for clarity, since it's currently ignored,
> > anyhow.
>
> We intentionally don't do things that way.

[-- Attachment #2: Type: text/html, Size: 6672 bytes --]

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

[Szabolcs Nagy]

* 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.

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

[Florian Weimer]

* Szabolcs Nagy:

> 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__.

Why wouldn't something like this be valid for current C++ versions?

static inline constexpr decltype(nullptr) __null{};
#define NULL __null

I don't see a requirement that NULL must be an expression that can be
evaluated by the preprocessor.

Thanks,
Florian

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

[Szabolcs Nagy]

* Florian Weimer <fweimer@redhat.com> [2019-07-10 08:34:12 +0200]:
> * Szabolcs Nagy:
> > 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__.
> 
> Why wouldn't something like this be valid for current C++ versions?

if you aim for current c++ only then you need
even more ifdefs (#if __cplusplus > ..).

> static inline constexpr decltype(nullptr) __null{};
> #define NULL __null
> 
> I don't see a requirement that NULL must be an expression that can be
> evaluated by the preprocessor.

c++03 requires an integral const expr.,
integral const expr must be usable in #if.
c++11 quietly changed that.

in this case that works out though: undefined
identifiers are 0 in #if, so #if __null works
as expected.

stringification is probably not a valid argument.

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

[James Y Knight]

[-- Attachment #1: Type: text/plain, Size: 5696 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 8029 bytes --]

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

[James Y Knight]

[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]

On Wed, Jul 10, 2019 at 2:34 AM Florian Weimer <fweimer@redhat.com> wrote:

> * Szabolcs Nagy:
>
> > 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__.
>
> Why wouldn't something like this be valid for current C++ versions?
>
> static inline constexpr decltype(nullptr) __null{};
> #define NULL __null
>
> I don't see a requirement that NULL must be an expression that can be
> evaluated by the preprocessor.


Is this just for the purposes of argument, or, why would you want to do
that? More sanely, yes, `#define NULL nullptr` would now also be a
conforming definition, but that is an undesirable change in practice for a
variety of compatibility reasons. Toolchains have remained with `#define
NULL __null`, and seem likely to continue to do so indefinitely.

I feel like this thread is kinda going off on a tangent now. Inventing new
and unique ways to define NULL in C++ doesn't really seem a useful thing to
do be doing here...

[-- Attachment #2: Type: text/html, Size: 1486 bytes --]

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

[Rich Felker]

On Wed, Jul 10, 2019 at 12:18:11PM -0400, James Y Knight wrote:
> On Wed, Jul 10, 2019 at 2:34 AM Florian Weimer <fweimer@redhat.com> wrote:
> 
> > * Szabolcs Nagy:
> >
> > > 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__.
> >
> > Why wouldn't something like this be valid for current C++ versions?
> >
> > static inline constexpr decltype(nullptr) __null{};
> > #define NULL __null
> >
> > I don't see a requirement that NULL must be an expression that can be
> > evaluated by the preprocessor.
> 
> 
> Is this just for the purposes of argument, or, why would you want to do
> that? More sanely, yes, `#define NULL nullptr` would now also be a
> conforming definition, but that is an undesirable change in practice for a
> variety of compatibility reasons. Toolchains have remained with `#define
> NULL __null`, and seem likely to continue to do so indefinitely.
> 
> I feel like this thread is kinda going off on a tangent now. Inventing new
> and unique ways to define NULL in C++ doesn't really seem a useful thing to
> do be doing here...

Well "do it the same way GCC's headers did it" isn't useful because
we're not using GCC's headers. If there's a definition that's
preferable to what we have now, it should stand on its own as a better
(or at least better in some ways that we decide are good trade-offs)
choice. The current definition was chosen that way, not just at
random, as you can see from the commit history.

musl explicitly does not use, require, or support the use of
standard-namespace-conflicting headers (as opposed to extensions like
vector intrinsics) shipped with the compiler, and supports as wide a
range as possible of compilers -- for building musl itself, ones
implementing a minimal subset of "GNU C", and for compiling against
musl, an even more minimal subset that can be "faked" with just some
macros if needed. So whether there are or may be "GNU C++" compilers
that lack __null is a valid question. My leaning would kinda be to use
nullptr in recent C++ versions and retain 0L for old ones if nullptr
is a valid definition in new C++ versions, but I still wonder if
having use of NULL "break maximally" isn't a better behavior with
respect to ending its use...

Rich

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

[James Y Knight]

[-- Attachment #1: Type: text/plain, Size: 3540 bytes --]

On Wed, Jul 10, 2019 at 12:45 PM Rich Felker <dalias@libc.org> wrote:

> On Wed, Jul 10, 2019 at 12:18:11PM -0400, James Y Knight wrote:
> > On Wed, Jul 10, 2019 at 2:34 AM Florian Weimer <fweimer@redhat.com>
> wrote:
> >
> > > * Szabolcs Nagy:
> > >
> > > > 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__.
> > >
> > > Why wouldn't something like this be valid for current C++ versions?
> > >
> > > static inline constexpr decltype(nullptr) __null{};
> > > #define NULL __null
> > >
> > > I don't see a requirement that NULL must be an expression that can be
> > > evaluated by the preprocessor.
> >
> >
> > Is this just for the purposes of argument, or, why would you want to do
> > that? More sanely, yes, `#define NULL nullptr` would now also be a
> > conforming definition, but that is an undesirable change in practice for
> a
> > variety of compatibility reasons. Toolchains have remained with `#define
> > NULL __null`, and seem likely to continue to do so indefinitely.
> >
> > I feel like this thread is kinda going off on a tangent now. Inventing
> new
> > and unique ways to define NULL in C++ doesn't really seem a useful thing
> to
> > do be doing here...
>
> Well "do it the same way GCC's headers did it" isn't useful because
> we're not using GCC's headers. If there's a definition that's
> preferable to what we have now, it should stand on its own as a better
> (or at least better in some ways that we decide are good trade-offs)
> choice. The current definition was chosen that way, not just at
> random, as you can see from the commit history


Neither was GCC's choice random, as I'm sure you'll agree.

musl explicitly does not use, require, or support the use of
> standard-namespace-conflicting headers (as opposed to extensions like
> vector intrinsics) shipped with the compiler, and supports as wide a
> range as possible of compilers -- for building musl itself, ones
> implementing a minimal subset of "GNU C", and for compiling against
> musl, an even more minimal subset that can be "faked" with just some
> macros if needed. So whether there are or may be "GNU C++" compilers
> that lack __null is a valid question.


It's a question which is impossible to ever answer in the negative -- there
always _may be_ any sort of terrible software implemented out there,
somewhere. But, I do doubt any such relevant compilers actually exist.

My leaning would kinda be to use
> nullptr in recent C++ versions and retain 0L for old ones if nullptr
> is a valid definition in new C++ versions, but I still wonder if
> having use of NULL "break maximally" isn't a better behavior with
> respect to ending its use...
>

#define NULL nullptr is standards-valid in c++11 and later, but would be an
unfortunate choice to make. Both in terms of breaking working code (code
which is making unportable assumptions, granted), but also in terms of
breaking ABIs on valid code: changing the type from long to
decltype(nullptr) changes mangling, etc.

If you really wanted to push for that, I'd recommend starting a discussion
on the gcc or clang lists -- the libc list doesn't really seem a place most
likely to find C++ experts to help evaluate the pros and cons of making
such a change. Certainly that all such C++ experts had previously decided
it would be a bad idea to do so doesn't *necessarily* mean it's still the
wrong thing, but it should at least be considered a strong signal...

[-- Attachment #2: Type: text/html, Size: 4710 bytes --]

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

[A. Wilcox]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

On Jul 10, 2019, at 12:35 PM, James Y Knight <jyknight@google.com> wrote:
> 
> It's a question which is impossible to ever answer in the negative -- there always _may be_ any sort of terrible software implemented out there, somewhere. But, I do doubt any such relevant compilers actually exist.

Or, put another way, it has always seemed to me that one of musl's tenets is to "fail fast and break hard" on egregiously invalid code. I'd argue "pretending to be GNU C++ and not having __null" is much more egregious than "code still using NULL in C++".  Therefore it's better to break the invalid compiler (which could have any number of other bugs) than break the C++ code.

Sincerely,
--someone who actually enjoys using musl and also actually enjoys using C++, a seemingly rare breed

--
A. Wilcox (Sent from my iPhone - not signed)
Project Lead, Adélie Linux
https://adelielinux.org

[-- Attachment #2: Type: text/html, Size: 1629 bytes --]

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

[Michael Everitt]

[-- Attachment #1.1.1: Type: text/plain, Size: 1155 bytes --]

On 10/07/19 21:11, A. Wilcox wrote:
> On Jul 10, 2019, at 12:35 PM, James Y Knight <jyknight@google.com
> <mailto:jyknight@google.com>> wrote:
>
>> It's a question which is impossible to ever answer in the negative --
>> there always _may be_ any sort of terrible software implemented out
>> there, somewhere. But, I do doubt any such relevant compilers actually
>> exist.
>
> Or, put another way, it has always seemed to me that one of musl's tenets
> is to "fail fast and break hard" on egregiously invalid code. I'd argue
> "pretending to be GNU C++ and not having __null" is much more egregious
> than "code still using NULL in C++".  Therefore it's better to break the
> invalid compiler (which could have any number of other bugs) than break
> the C++ code.
>
> Sincerely,
> --someone who actually enjoys using musl and also actually enjoys using
> C++, a seemingly rare breed
>
> --
> A. Wilcox (Sent from my iPhone - not signed)
> Project Lead, Adélie Linux
> https://adelielinux.org
You need to be cloned before you become a target of some weird computing
museum somewhere ...

[but this isn't news, and very OT...]

[-- Attachment #1.1.2: Type: text/html, Size: 2486 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Rich Felker]

On Wed, Jul 10, 2019 at 03:11:30PM -0500, A. Wilcox wrote:
> On Jul 10, 2019, at 12:35 PM, James Y Knight <jyknight@google.com> wrote:
> > 
> > It's a question which is impossible to ever answer in the negative
> > -- there always _may be_ any sort of terrible software implemented
> > out there, somewhere. But, I do doubt any such relevant compilers
> > actually exist.
> 
> Or, put another way, it has always seemed to me that one of musl's
> tenets is to "fail fast and break hard" on egregiously invalid code.

Code != compilers. We do judge egregiously bad compilers on failing to
conform to the documented standards, but there really is no standard
for what is "GNU C" (or "GNU C++") except "GCC". The intent has always
been clear to require the minimal subset of this needed to work, so
that other current and future compilers can be usable without having
to clone all of GCC. It's a matter of reducing the barrier to entry,
which is already way too high.

> I'd argue "pretending to be GNU C++ and not having __null" is much
> more egregious than "code still using NULL in C++". Therefore it's
> better to break the invalid compiler (which could have any number of
> other bugs) than break the C++ code.

"Still using NULL in C++" is bad style but valid. Making assumptions
that it's valid for terminating variadic lists or that it will have a
type that causes particular overloads to get called is what's not
valid.

Rich

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

[Rich Felker]

On Wed, Jul 10, 2019 at 01:35:35PM -0400, James Y Knight wrote:
> My leaning would kinda be to use
> > nullptr in recent C++ versions and retain 0L for old ones if nullptr
> > is a valid definition in new C++ versions, but I still wonder if
> > having use of NULL "break maximally" isn't a better behavior with
> > respect to ending its use...
> >
> 
> #define NULL nullptr is standards-valid in c++11 and later, but would be an
> unfortunate choice to make. Both in terms of breaking working code (code
> which is making unportable assumptions, granted), but also in terms of
> breaking ABIs on valid code: changing the type from long to
> decltype(nullptr) changes mangling, etc.

Could you clarify how it "breaks ABI"? NULL is not a type but a macro
expanding to an expression. Does its type somehow leak into mangled
symbol names via templates or something? If so, this is a complication
to any proposed change of the type.

Rich

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

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2019-07-10 16:48:12 -0400]:
> On Wed, Jul 10, 2019 at 01:35:35PM -0400, James Y Knight wrote:
> > My leaning would kinda be to use
> > > nullptr in recent C++ versions and retain 0L for old ones if nullptr
> > > is a valid definition in new C++ versions, but I still wonder if
> > > having use of NULL "break maximally" isn't a better behavior with
> > > respect to ending its use...
> > >
> > 
> > #define NULL nullptr is standards-valid in c++11 and later, but would be an
> > unfortunate choice to make. Both in terms of breaking working code (code
> > which is making unportable assumptions, granted), but also in terms of
> > breaking ABIs on valid code: changing the type from long to
> > decltype(nullptr) changes mangling, etc.
> 
> Could you clarify how it "breaks ABI"? NULL is not a type but a macro
> expanding to an expression. Does its type somehow leak into mangled
> symbol names via templates or something? If so, this is a complication
> to any proposed change of the type.

void f(int);
void f(long);
void f(void*);
...
f(NULL); // if NULL is 0 vs 0L then the int vs long version is called.

so the dispatch (and called symbol) depends on the definition of NULL

__null behaves like 0L, nullptr would dispatch to the void* version.

i think modern c++ code should use nullptr in the code.

definition of NULL should be imo kept as 0L (that's what
you would get on older unix systems or on openbsd anyway)
apparently some ppl prefer __null.

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

[Rich Felker]

On Wed, Jul 10, 2019 at 11:11:55PM +0200, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2019-07-10 16:48:12 -0400]:
> > On Wed, Jul 10, 2019 at 01:35:35PM -0400, James Y Knight wrote:
> > > My leaning would kinda be to use
> > > > nullptr in recent C++ versions and retain 0L for old ones if nullptr
> > > > is a valid definition in new C++ versions, but I still wonder if
> > > > having use of NULL "break maximally" isn't a better behavior with
> > > > respect to ending its use...
> > > >
> > > 
> > > #define NULL nullptr is standards-valid in c++11 and later, but would be an
> > > unfortunate choice to make. Both in terms of breaking working code (code
> > > which is making unportable assumptions, granted), but also in terms of
> > > breaking ABIs on valid code: changing the type from long to
> > > decltype(nullptr) changes mangling, etc.
> > 
> > Could you clarify how it "breaks ABI"? NULL is not a type but a macro
> > expanding to an expression. Does its type somehow leak into mangled
> > symbol names via templates or something? If so, this is a complication
> > to any proposed change of the type.
> 
> void f(int);
> void f(long);
> void f(void*);
> ....
> f(NULL); // if NULL is 0 vs 0L then the int vs long version is called.
> 
> so the dispatch (and called symbol) depends on the definition of NULL
> 
> __null behaves like 0L, nullptr would dispatch to the void* version.

I see. I don't see this as ABI breakage, but rather as a change in the
behavior produced by non-portable code. But I wonder if it's also
possible to see ABI breakage from a change.

> i think modern c++ code should use nullptr in the code.
> 
> definition of NULL should be imo kept as 0L (that's what
> you would get on older unix systems or on openbsd anyway)
> apparently some ppl prefer __null.

Do you know if OpenBSD has a reason they do it this way?

Rich

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

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2019-07-10 17:16:24 -0400]:
> > i think modern c++ code should use nullptr in the code.
> > 
> > definition of NULL should be imo kept as 0L (that's what
> > you would get on older unix systems or on openbsd anyway)
> > apparently some ppl prefer __null.
> 
> Do you know if OpenBSD has a reason they do it this way?

hm i misremembered and they changed it anyway
https://github.com/openbsd/src/blob/master/sys/sys/_null.h

openbsd used to define NULL as 0L for both c and c++,
then they changed it to __null if __GNUG__ and 0L and
(void*)0 for c++ and c, now it's even more complicated.

maybe it was some other bsd or solaris that used 0L
i remember looking at it last time we changed NULL.