Bug 103848 - [11 Regression] std::deque<>::iterator operator- uses "0" for nullptr check, triggers "zero-as-null-pointer-constant" warning
Summary: [11 Regression] std::deque<>::iterator operator- uses "0" for nullptr check, ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 11.2.1
: P3 normal
Target Milestone: 11.4
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2021-12-28 02:24 UTC by Stefan Brüns
Modified: 2022-04-21 12:39 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-12-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Brüns 2021-12-28 02:24:23 UTC
Since https://github.com/gcc-mirror/gcc/commit/f6be5d6ee31b76838e242704782938bc9745659c

using `operator-` may fail:

```
[  346s] In file included from /usr/include/c++/11/deque:67,
[  346s]                  from /usr/include/c++/11/queue:60,
...
[  346s] /usr/include/c++/11/bits/stl_deque.h:356:58: error: zero as null pointer constant [-Werror=zero-as-null-pointer-constant]
[  346s]   356 |           * (__x._M_node - __y._M_node - int(__x._M_node != 0))
[  346s]       |                                              ~~~~~~~~~~~~^~~~
[  346s] cc1plus: some warnings being treated as errors
```
Comment 1 Andrew Pinski 2021-12-28 02:39:18 UTC
Confirmed.
Comment 2 Jonathan Wakely 2022-01-04 14:31:06 UTC
This is not a bug.

Firstly, there's no testcase provided (as https://gcc.gnu.org/bugs says is needed). Here's the missing testcase:

#include <deque>
std::deque<int> d;
auto n = d.begin() - d.begin();

Secondly, the command to reproduce the error is not provided, as the link also says is needed. This "error" only happens with -Werror=zero-as-null-pointer-constant -Wsystem-headers, so don't do that. This is not even a warning unless you explicitly ask for it (it's not in -Wall or -Wextra) and if you choose to make it an error, that's your fault.

We can't nullptr because the code needs to compile as C++98. We could use NULL or __null, or Jakub suggested just _Map_pointer. I find all of those inferior to 0 but I suppose we can change it.
Comment 3 Jakub Jelinek 2022-01-04 15:15:44 UTC
I said - int(__x._M_node != _Map_pointer())
but indeed one doesn't know from looking at the code that it is comparison against NULL.
Other options perhaps could be - (__x._M_node ? 1 : 0) or - 1 + !__x._M_node
or - !!__x._M_node
I'd hope the compiler treats all those the same, if not, we have something to fix.
Comment 4 Jonathan Wakely 2022-01-04 15:42:18 UTC
(In reply to Jakub Jelinek from comment #3)
> Other options perhaps could be - (__x._M_node ? 1 : 0)

That produces worse code (with a jump) at -O1

> or - 1 + !__x._M_node

Isn't that undefined for (x - y - 1 + !x) if x and y are both null?
We get (T*)0 - 1 + 1 which overflows twice.

> or - !!__x._M_node

I think that's what I'll go with, thanks.
Comment 5 Jonathan Wakely 2022-01-04 15:58:48 UTC
(In reply to Jonathan Wakely from comment #4)
> We get (T*)0 - 1 + 1 which overflows twice.

GCC's ubsan doesn't diagnose this, but Clang's does.
Comment 6 Jakub Jelinek 2022-01-04 16:02:22 UTC
(In reply to Jonathan Wakely from comment #4)
> > or - 1 + !__x._M_node
> 
> Isn't that undefined for (x - y - 1 + !x) if x and y are both null?
> We get (T*)0 - 1 + 1 which overflows twice.

You're right, it would need to be + (!__x.M_node - 1).
 
> > or - !!__x._M_node
> 
> I think that's what I'll go with, thanks.
Comment 7 Jonathan Wakely 2022-01-04 16:18:17 UTC
(In reply to Jonathan Wakely from comment #4)
> (In reply to Jakub Jelinek from comment #3)
> > Other options perhaps could be - (__x._M_node ? 1 : 0)
> 
> That produces worse code (with a jump) at -O1

Oops, no it doesn't, I testsed this intead:

(__x._M_node ? __x._M_node - __y._M_node - 1 : 0)

And that has a jump, because the compiler doesn't know that __x._M_node being null implies __y._M_node is null too (because the standard says it's undefined otherwise).
Comment 8 Stefan Brüns 2022-01-05 03:08:41 UTC
(In reply to Jonathan Wakely from comment #2)
> This is not a bug.
> 
> Firstly, there's no testcase provided (as https://gcc.gnu.org/bugs says is
> needed). Here's the missing testcase:
> 
> #include <deque>
> std::deque<int> d;
> auto n = d.begin() - d.begin();
> 
> Secondly, the command to reproduce the error is not provided, as the link
> also says is needed. This "error" only happens with
> -Werror=zero-as-null-pointer-constant -Wsystem-headers, so don't do that.
> This is not even a warning unless you explicitly ask for it (it's not in
> -Wall or -Wextra) and if you choose to make it an error, that's your fault.
> 
> We can't nullptr because the code needs to compile as C++98. We could use
> NULL or __null, or Jakub suggested just _Map_pointer. I find all of those
> inferior to 0 but I suppose we can change it.

Full GCC command line:

```
[  826s] g++ -c -o test_allheaders_allheaders.o  -I/home/abuild/rpmbuild/BUILD/wxWidgets-3.1.5/lib/wx/include/gtk3-unicode-3.1 -I../include -D_FILE_OFFSET_BITS=64 -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/fribidi -I/usr/include/uuid -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/gio-unix-2.0 -I/usr/include/wayland -I/usr/include/libxkbcommon -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -I/usr/include/at-spi-2.0 -pthread  -D__WXGTK__      -I. -DWXUSINGDLL -I./../samples -I../3rdparty/catch/include -pthread -Wall -Wundef -Wunused-parameter -Wno-ctor-dtor-privacy -Woverloaded-virtual -Wno-deprecated-declarations -I/usr/include/freetype2 -I/usr/include/uuid -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/fribidi -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/gtk-3.0/unix-print -I/usr/include/gtk-3.0 -I/usr/include/gio-unix-2.0 -I/usr/include/wayland -I/usr/include/libxkbcommon -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -I/usr/include/at-spi-2.0 -I/usr/include/SDL2 -D_REENTRANT -O2 -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g -I/usr/include/webkitgtk-4.0 -I/usr/include/libsoup-2.4 -I/usr/include/libxml2 -I/usr/include/gstreamer-1.0 -I/usr/include/orc-0.4  -fvisibility=hidden -fvisibility-inlines-hidden -fPIC -DPIC  -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g ./allheaders.cpp
[  826s] In file included from /usr/include/c++/11/deque:67,
[  826s]                  from /usr/include/c++/11/queue:60,
[  826s]                  from ../include/wx/msgqueue.h:24,
[  826s]                  from ./allheaders.h:239,
[  826s]                  from ./allheaders.cpp:435:
```

So definitely neither -Werror=zero-as-null-pointer-constant nor -Wsystem-headers.


GCC -v
```
Using built-in specs.
Reading specs from /usr/lib64/gcc/x86_64-suse-linux/11/defaults.spec
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib64/gcc/x86_64-suse-linux/11/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-suse-linux
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,objc,fortran,obj-c++,ada,go,d,jit --enable-offload-targets=nvptx-none,amdgcn-amdhsa, --without-cuda-driver --enable-host-shared --enable-checking=release --disable-werror --with-gxx-include-dir=/usr/include/c++/11 --enable-ssp --disable-libssp --disable-libvtv --enable-cet=auto --disable-libcc1 --enable-plugin --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-libphobos --enable-version-specific-runtime-libs --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex --enable-gnu-indirect-function --program-suffix=-11 --without-system-libunwind --enable-multilib --with-arch-32=x86-64 --with-tune=generic --with-build-config=bootstrap-lto-lean --enable-link-mutex --build=x86_64-suse-linux --host=x86_64-suse-linux
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.2.1 20211124 [revision 7510c23c1ec53aa4a62705f0384079661342ff7b] (SUSE Linux) 
```
Comment 9 Jonathan Wakely 2022-01-05 09:22:54 UTC
Either that isn't the correct g++ command, or something in the code you're compiling uses "#pragma GCC diagnostic error ..." to enable the warning.

There is no way to get an *error* from a non-default warning in a system header otherwise.
Comment 10 Jakub Jelinek 2022-01-05 09:31:32 UTC
Please rerun that command with -c -o test_allheaders_allheaders.o replaced with
-E -dD -o test_allheaders_allheaders.ii
Then compress it and attach here.
Comment 11 GCC Commits 2022-01-05 13:47:54 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:76a45931ab7c831e32cebf13a6317e5e142f8151

commit r12-6261-g76a45931ab7c831e32cebf13a6317e5e142f8151
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 4 21:57:16 2022 +0000

    libstdc++: Avoid -Wzero-as-null-pointer-constant warning [PR103848]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/103848
            * include/bits/stl_deque.h (operator-): Do not use 0 as null
            pointer constant.
Comment 12 Richard Biener 2022-04-21 07:51:01 UTC
GCC 11.3 is being released, retargeting bugs to GCC 11.4.
Comment 13 GCC Commits 2022-04-21 12:32:23 UTC
The releases/gcc-11 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:b59435ff6069583290c8e0953a58044f598a348c

commit r11-9899-gb59435ff6069583290c8e0953a58044f598a348c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 4 21:57:16 2022 +0000

    libstdc++: Avoid -Wzero-as-null-pointer-constant warning [PR103848]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/103848
            * include/bits/stl_deque.h (operator-): Do not use 0 as null
            pointer constant.
    
    (cherry picked from commit 76a45931ab7c831e32cebf13a6317e5e142f8151)
Comment 14 Jonathan Wakely 2022-04-21 12:39:24 UTC
Fixed for 11.4