Bug 77513 - -Wzero-as-null-pointer-constant vs 0, nullptr, NULL and __null
Summary: -Wzero-as-null-pointer-constant vs 0, nullptr, NULL and __null
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: 9.0
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: 78989
  Show dependency treegraph
 
Reported: 2016-09-07 11:22 UTC by petschy
Modified: 2024-04-03 10:59 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 9.1.0
Known to fail:
Last reconfirmed: 2021-11-21 00:00:00


Attachments
Preprocessed source, generated with g++-7.0.0 -std=c++14 -Wzero-as-null-pointer-constant 20160907-null.cpp -E > 20160907-null.ii (341 bytes, text/plain)
2016-09-07 11:22 UTC, petschy
Details

Note You need to log in before you can comment on or make changes to this bug.
Description petschy 2016-09-07 11:22:47 UTC
Created attachment 39580 [details]
Preprocessed source, generated with g++-7.0.0 -std=c++14 -Wzero-as-null-pointer-constant 20160907-null.cpp -E > 20160907-null.ii

Yesterday I switched on the warning for a ~250kloc codebase to clean it up. Used 7.0, it was tedious but it was done. I had to replace NULLs also, not just 0s, but at that time I wasn't suspecting anything, though it seemed a bit strange.

Then, tried to build on another machine with 5.4.1, and to my surprise, tons of warnings appeared. Then tried to build on my machine with 5.4.1, the same results. It turned out that NULLs are frowned upon, quite inconsistently.

5.4.1 has problems with 7 cpp files, 6.2.1 and 7.0 with just a single one. Did a grep for NULL, and as expected for a large and aging codebase, there were lots of them, but they are not treated equally. All files are c++, and compiled with the same flags.

Preprocessed 2 problematic files with all three gcc versions mentioned. Diffing them revealed that there is no difference in the actual code, only what gets included due to the differing gcc versions. All NULLs were replaced with __null's by the preprocessor, which is defined in the gcc version specific stddef.h include.

Crafted a test case:

#include <stddef.h>
char* a = 0;
char* b = nullptr;
char* c = __null;
char* d = NULL;
int main()
{
}

$ g++-5.4.1 -std=c++14 -Wzero-as-null-pointer-constant 20160907-null.cpp
20160907-null.cpp:2:11: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
 char* a = 0;
           ^
20160907-null.cpp:4:11: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
 char* c = __null;
           ^
6.2.1 and 7.0 print exactly the same warnings.

So NULL is ok, but __null is not? The end of the preprocessed source looks like this:

# 2 "20160907-null.cpp"
char* a = 0;
char* b = nullptr;
char* c = __null;
char* d = 
# 5 "20160907-null.cpp" 3 4
         __null
# 5 "20160907-null.cpp"
             ;
int main()
{
}

c and d initialized the same except for whitespace and the two "'# 6" lines around d's __null. I naively thought that these are only to communicate line info to the compiler, but if I delete the first one:

$ g++-7.0.0 -std=c++14 -Wzero-as-null-pointer-constant 20160907-null.ii
20160907-null.cpp:2:11: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
 char* a = 0;
           ^
20160907-null.cpp:4:11: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
 char* c = __null;
           ^~~~~~
20160907-null.cpp:6:10: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
 int main()
          ^

The interpretation of __null at d changed for some reason. What is going on? It seems that the interpretation can change unpredictably, and in the problematic source files __null's are misdiagnosed even when the "# ..." lines are around them.

For c++11 and later code, why is NULL defined as __null, rather than nullptr?

I put a fast bandaid on my code by redefining NULL to be nullptr after the last include in the problematic files, but since the number of problematic files seems to change from gcc version to gcc version, this is rather fragile, let alone unelegant.

Platform is Debian Jessie AMD64, the gcc versions:

$ g++-5.4.1 -v
Using built-in specs.
COLLECT_GCC=g++-5.4.1
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-unknown-linux-gnu/5.4.1/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../configure --enable-languages=c,c++ --disable-multilib --program-suffix=-5.4.1 --disable-bootstrap CFLAGS='-O2 -march=native' CXXFLAGS='-O2 -march=native'
Thread model: posix
gcc version 5.4.1 20160829 (GCC)

$ g++-6.2.1 -v
Using built-in specs.
COLLECT_GCC=g++-6.2.1
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-pc-linux-gnu/6.2.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --enable-languages=c,c++ --disable-multilib --program-suffix=-6.2.1 --disable-bootstrap CFLAGS='-O2 -march=native' CXXFLAGS='-O2 -march=native'
Thread model: posix
gcc version 6.2.1 20160831 (GCC)

$ g++-7.0.0 -v
Using built-in specs.
COLLECT_GCC=g++-7.0.0
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/x86_64-pc-linux-gnu/7.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --enable-languages=c,c++ --disable-multilib --program-suffix=-7.0.0 --disable-bootstrap CFLAGS='-O2 -march=native' CXXFLAGS='-O2 -march=native'
Thread model: posix
gcc version 7.0.0 20160831 (experimental) (GCC)
Comment 1 Andreas Schwab 2016-09-07 12:21:33 UTC
The "3" flag on the line marker marks the following lines as originating from a system header where warnings are suppressed.  Use -Wsystem-headers to enable them.
Comment 2 petschy 2016-09-07 12:37:54 UTC
I don't want to enable them. The problem is not with too little but too many warnings.

A snippet from one of the problematic files:

	{ NULL, NULL, false, false }

is preprocessed to 

 { 
# 62 "AdsPlugin.cpp" 3 4
  __null
# 62 "AdsPlugin.cpp"
      , 
# 62 "AdsPlugin.cpp" 3 4
        __null
# 62 "AdsPlugin.cpp"
            , false, false }
};

Here I see the same flags, yet for these two NULLs gcc warns.
Comment 3 Jonathan Wakely 2016-09-07 15:37:37 UTC
(In reply to petschy from comment #0)
> For c++11 and later code, why is NULL defined as __null, rather than nullptr?

Because defining NULL as nullptr would violate the requirements of the standard, which very intentionally says that NULL is an integral constant expression, not nullptr.
Comment 4 Manuel López-Ibáñez 2016-09-07 19:18:34 UTC
(In reply to petschy from comment #2)
> Here I see the same flags, yet for these two NULLs gcc warns.

prog.cc:6:28: warning: zero as null pointer constant [-Wzero-as-null-pointer-constant]
             , false, false };
                            ^

because the location given is wrong, so the location in this case is not within the system-header flags. If the location was right (under __null), it would not warn. That is the bug.
Comment 5 petschy 2016-09-16 10:04:41 UTC
Some more details, hope this helps. Preprocessed one of the oddly behaving files with 5.4.1, 6.2.1 and 7.0.0, then tried to compile each preprocessed file with each compiler version. 5.4.1 warned for all preprocessed files, 6.2.1 and 7.0.0 didn't warn for any of them. This doesn't mean that 6.2.1 and 7.0.0 is not affected, just for this particular file they didn't warn. 

Diffing the files revealed that there is no difference in the user code, only in the system headers included, eg <string> comes from /usr/local/include/c++/5.4.1/string for 5.4.1, etc.

All of the problematic lines are assignments of the form
  var = NULL;
or
  var1 = var2 = NULL;

ALL of the NULL assignments in the file are reported.
NONE of the == or != tests are reported.

Eg: this code warns:

AbstractImage::AbstractImage()
{
 pixels=
# 124 "common/src/AbstractImage.cpp" 3 4
       __null
# 124 "common/src/AbstractImage.cpp"
           ;
 channels_n=0;
 mode=255;
 width=-1;
 height=-1;
}

But if I replace 'pixels =' with 'bool b = pixels ==' above, the warning disappears, which is strange, I think.

The flags are the same for the != and == tests, ie 
# <n> "common/src/AbstractImage.cpp" 3 4
is everywhere.
Comment 6 Fabio Alemagna 2018-05-24 12:37:01 UTC
(In reply to Jonathan Wakely from comment #3)
> (In reply to petschy from comment #0)
> > For c++11 and later code, why is NULL defined as __null, rather than nullptr?
> 
> Because defining NULL as nullptr would violate the requirements of the
> standard, which very intentionally says that NULL is an integral constant
> expression, not nullptr.

I don't know how autoritative is it, but cppreference.com says that since C++11,  NULL is

    an integer literal with value zero, or a prvalue of type std::nullptr_t

See: http://en.cppreference.com/w/cpp/types/NULL
Comment 7 Ville Voutilainen 2018-05-30 19:51:01 UTC
"The macro NULL is an implementation-defined null pointer constant.", says the C++ standard draft.
Comment 8 Ville Voutilainen 2018-05-30 20:00:27 UTC
See r260973
Comment 9 Jonathan Wakely 2019-05-20 07:59:51 UTC
(In reply to Ville Voutilainen from comment #7)
> "The macro NULL is an implementation-defined null pointer constant.", says
> the C++ standard draft.

So it does, maybe I was misremembering something from an older C++0x draft.

Either way, 0 is still a valid definition of NULL, and I suspect that defining NULL as nullptr would break a lot of code (albeit bad code that is misusing NULL).
Comment 10 Andrew Pinski 2021-08-27 18:50:55 UTC
*** Bug 77299 has been marked as a duplicate of this bug. ***
Comment 11 Andrew Pinski 2021-08-27 18:51:49 UTC
Confirmed.
Comment 12 Andrew Pinski 2021-11-21 14:54:31 UTC
*** Bug 103347 has been marked as a duplicate of this bug. ***
Comment 13 Jonathan Wakely 2021-11-21 17:05:13 UTC
An intereting case from PR 103347 where the pedwarn about the NSDMI is suppressed because GCC thinks the initializer is in a system header:

#include <cstddef>
struct test {
    void *x = NULL; //invalid in C++03 mode
};
int main() {}

This should be rejected with -pedantic-errors, but g++ is silent unless you also add -Wsystem-headers.
Comment 14 Jonathan Wakely 2021-11-22 09:25:13 UTC
(In reply to Jonathan Wakely from comment #13)
> An intereting case from PR 103347 where the pedwarn about the NSDMI is
> suppressed because GCC thinks the initializer is in a system header:
> 
> #include <cstddef>
> struct test {
>     void *x = NULL; //invalid in C++03 mode
> };
> int main() {}
> 
> This should be rejected with -pedantic-errors, but g++ is silent unless you
> also add -Wsystem-headers.

And this is a regression, because 4.8 silently accepts this code in C++98 mode, whereas 4.7 warns about it as expected:

null.C:3:15: warning: non-static data member initializers only available with -std=c++11 or -std=gnu++11 [enabled by default]
Comment 15 Andrew Pinski 2021-11-22 09:41:18 UTC
Note the original issue with -Wzero-as-null-pointer-constant is fixed in GCC 9 by r9-873. The system header issue is now listed as PR 77299.