[Bug c/111615] New: NULL check incorrectly skipped at O2 and O3

[Bug c/111615] New: NULL check incorrectly skipped at O2 and O3

[gardner.ben at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111615

            Bug ID: 111615
           Summary: NULL check incorrectly skipped at O2 and O3
           Product: gcc
           Version: 11.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gardner.ben at gmail dot com
  Target Milestone: ---

Created attachment 56003
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56003&action=edit
Source file that produces the issue.

The attached source code has a function (pr_str()) that prints something
different if the parameter is NULL.
When passed a NULL (const char *) value from a static const structure, the NULL
check is skipped and the first printf() is executed.


static void pr_str(const char *s)
{
   /* BUG: this NULL check is skipped/wrong at O2 and O3 for
    * vec->haystack and vec->needle.
    */
   if (s != NULL)
   {
      printf("'%s' %p %d", s, s, (int)(intptr_t)s);
   }
   else
   {
      printf("(nil)");
   }
}


This occurs at O2 and O3, but not at O0, O1, or Os.
If the program prints "h=(nil)" on the 2nd to last line when executed, then it
worked.
The the program prints "h='(null)' (nil) 0" on the 2nd to last line, then if
failed.


Build script:
#!/bin/sh
build_it() {
        OP=$1
        gcc -g -Wall -O$OP -c -o memmem_test.O$OP.o memmem_test.c
        gcc memmem_test.O$OP.o -o memmem_test.O$OP
}
build_it 0
build_it 1
build_it 2
build_it 3
build_it s


GCC detailed info:
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/11/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
11.4.0-1ubuntu1~22.04' --with-bugurl=file:///usr/share/doc/gcc-11/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-11
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib
--enable-libphobos-checking=release --with-target-system-zlib=auto
--enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none=/build/gcc-11-XeT9lY/gcc-11-11.4.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-11-XeT9lY/gcc-11-11.4.0/debian/tmp-gcn/usr
--without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
--with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)

[Bug c/111615] NULL check incorrectly skipped at O2 and O3

[gardner.ben at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111615

--- Comment #1 from Ben Gardner <gardner.ben at gmail dot com> ---
Created attachment 56004
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56004&action=edit
Build script.

[Bug middle-end/111615] NULL check incorrectly skipped at O2 and O3

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111615

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't think this is a bug. memmem is defined such that a null pointer
argument is undefined even if the len is 0.

[Bug middle-end/111615] NULL check incorrectly skipped at O2 and O3

[gardner.ben at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111615

--- Comment #3 from Ben Gardner <gardner.ben at gmail dot com> ---
The issue isn't with memmem(). It is with the value passed into pr_str() from
the structure. I suspect memmem() is a distraction.
I'll try to further reduce the test case to eliminate memmem(), if possible.

[Bug middle-end/111615] NULL check incorrectly skipped at O2 and O3

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111615

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Anything passed to memmem (or memcmpy, or memcpy, etc.) is considered to be a
non-null pointer, because that's a requirement of those functions. And so if
it's a non-null pointer, any null checks for it can be removed.

[Bug middle-end/111615] NULL check incorrectly skipped at O2 and O3

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111615

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
extern void *memmem (const void *__haystack, size_t __haystacklen,
       const void *__needle, size_t __needlelen)
     __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__pure__))
__attribute__ ((__nonnull__ (1, 3)));


memmem is declared with nonnull for the 1st and 3rd argument. If those
arguments are null, the behavior is undefined and the values of those arguments
can be assumed as not null afterwards too.

If you don't want that behavior you can use -fno-delete-null-pointer-checks .

Otherwise the behavior you are seeing is correct behavior based on well defined
code.

[Bug middle-end/111615] NULL check incorrectly skipped at O2 and O3

[gardner.ben at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111615

--- Comment #6 from Ben Gardner <gardner.ben at gmail dot com> ---
(In reply to Andrew Pinski from comment #5)
> extern void *memmem (const void *__haystack, size_t __haystacklen,
>        const void *__needle, size_t __needlelen)
>      __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__pure__))
> __attribute__ ((__nonnull__ (1, 3)));
> 
> 
> memmem is declared with nonnull for the 1st and 3rd argument. If those
> arguments are null, the behavior is undefined and the values of those
> arguments can be assumed as not null afterwards too.
> 
> If you don't want that behavior you can use -fno-delete-null-pointer-checks .
> 
> Otherwise the behavior you are seeing is correct behavior based on well
> defined code.

Thanks for the info. That makes sense. I didn't check the header file, so I
didn't know that memmem() was declared with nonnull.

Also, thanks for the tip about -fno-delete-null-pointer-checks.