[Bug c/115552] New: wrong code at -O2 and above when strict-aliasing with uint128 and double

[Bug c/115552] New: wrong code at -O2 and above when strict-aliasing with uint128 and double

[davidhu0903ex3 at gmail dot com]

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

            Bug ID: 115552
           Summary: wrong code at -O2 and above when strict-aliasing with
                    uint128 and double
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Keywords: alias, wrong-code
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: davidhu0903ex3 at gmail dot com
  Target Milestone: ---

Created attachment 58469
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58469&action=edit
preproccessed file

=== Summary ===

When turning on -O2 optimization or above, the produced hash output is
different when without -O2, unless adding `-fno-strict-aliasing`. To my
investigation, the issue arise only when either:
  1. output to a uint128_t*
  2. input with a double*

=== Detail Explanation ===

The program produced the following output when compiled with -O1

  60d8f0889f4b7d88
  3bcf47a7eecabc5d
  1b153029d058b5115490463dd8be0468
  1b153029d058b5115490463dd8be0468

However, it produced different output with -O2

  e09546989572a77c
  95fe807a3fab3ad5
  b8b1ef1e717bc0d4818be1e9343867f6
  00000000000000000000000000000000

The first two lines are the case where the inputs are double* pointers. The
third line is the case where the output are written to a uint128_t* pointer.
The last line is the case of passing a uint128_t by value to `foo` before dump.

The warning didn't complain about strict-aliasing, but rather an ambiguous
message:

  'd1' may be used uninitialized [-Wmaybe-uninitialized]

However, those variables are already initialized in `main`:

  double d1 = 1;
  double d2 = 2;

From the generated assembly, the initialization of the inputs seems missing, so
perhaps that's the reason of the warning message. However, in production our
code didn't emit "-Wmaybe-uninitialized" at all, so that's what surprises us.

With Compiler Explorer (godbolt.org), I can roughly locate that this behavior
appear after version 10.5.0 and until 14.1.0.

=== Environment ===

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/12/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='Debian 12.2.0-14'
--with-bugurl=file:///usr/share/doc/gcc-12/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-12
--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-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-12-bTRWOB/gcc-12-12.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-12-bTRWOB/gcc-12-12.2.0/debian/tmp-gcn/usr
--enable-offload-defaulted --without-cuda-driver --enable-checking=release
--build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.0 (Debian 12.2.0-14) 
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc'
'-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -E -quiet -v -imultiarch x86_64-linux-gnu
poc.c -mtune=generic -march=x86-64 -Wall -Wextra -O2 -fpch-preprocess
-fasynchronous-unwind-tables -o poc.i
ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"
ignoring nonexistent directory "/usr/lib/gcc/x86_64-linux-gnu/12/include-fixed"
ignoring nonexistent directory
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/x86_64-linux-gnu/12/include
 /usr/local/include
 /usr/include/x86_64-linux-gnu
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc'
'-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-linux-gnu/12/cc1 -fpreprocessed poc.i -quiet -dumpbase
poc.c -dumpbase-ext .c -mtune=generic -march=x86-64 -O2 -Wall -Wextra -version
-fasynchronous-unwind-tables -o poc.s
GNU C17 (Debian 12.2.0-14) version 12.2.0 (x86_64-linux-gnu)
        compiled by GNU C version 12.2.0, GMP version 6.2.1, MPFR version
4.1.1-p1, MPC version 1.3.1, isl version isl-0.25-GMP

warning: MPFR header version 4.1.1-p1 differs from library version 4.2.0.
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C17 (Debian 12.2.0-14) version 12.2.0 (x86_64-linux-gnu)
        compiled by GNU C version 12.2.0, GMP version 6.2.1, MPFR version
4.1.1-p1, MPC version 1.3.1, isl version isl-0.25-GMP

warning: MPFR header version 4.1.1-p1 differs from library version 4.2.0.
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: cc72d2b9b5048fedc2be9051c917b40b
In function ‘spookyhash_128’,
    inlined from ‘main’ at poc.c:398:5:
poc.c:322:9: warning: ‘d1’ may be used uninitialized [-Wmaybe-uninitialized]
  322 |         spookyhash_short(message, length, hash1, hash2);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
poc.c: In function ‘main’:
poc.c:229:20: note: by argument 1 of type ‘const void *’ to
‘spookyhash_short.constprop’ declared here
  229 | static inline void spookyhash_short(const void *message, size_t length,
uint64_t *hash1, uint64_t *hash2) {
      |                    ^~~~~~~~~~~~~~~~
poc.c:393:12: note: ‘d1’ declared here
  393 |     double d1 = 1;
      |            ^~
In function ‘spookyhash_128’,
    inlined from ‘main’ at poc.c:399:5:
poc.c:322:9: warning: ‘d2’ may be used uninitialized [-Wmaybe-uninitialized]
  322 |         spookyhash_short(message, length, hash1, hash2);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
poc.c: In function ‘main’:
poc.c:229:20: note: by argument 1 of type ‘const void *’ to
‘spookyhash_short.constprop’ declared here
  229 | static inline void spookyhash_short(const void *message, size_t length,
uint64_t *hash1, uint64_t *hash2) {
      |                    ^~~~~~~~~~~~~~~~
poc.c:394:12: note: ‘d2’ declared here
  394 |     double d2 = 2;
      |            ^~
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc'
'-mtune=generic' '-march=x86-64'
 as -v --64 -o poc.o poc.s
GNU assembler version 2.40 (x86_64-linux-gnu) using BFD version (GNU Binutils
for Debian) 2.40
COMPILER_PATH=/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/
LIBRARY_PATH=/usr/lib/gcc/x86_64-linux-gnu/12/:/usr/lib/gcc/x86_64-linux-gnu/12/../../../x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/12/../../../../lib/:/lib/x86_64-linux-gnu/:/lib/../lib/:/usr/lib/x86_64-linux-gnu/:/usr/lib/../lib/:/usr/lib/gcc/x86_64-linux-gnu/12/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc'
'-mtune=generic' '-march=x86-64' '-dumpdir' 'poc.'
 /usr/lib/gcc/x86_64-linux-gnu/12/collect2 -plugin
/usr/lib/gcc/x86_64-linux-gnu/12/liblto_plugin.so
-plugin-opt=/usr/lib/gcc/x86_64-linux-gnu/12/lto-wrapper
-plugin-opt=-fresolution=poc.res -plugin-opt=-pass-through=-lgcc
-plugin-opt=-pass-through=-lgcc_s -plugin-opt=-pass-through=-lc
-plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s --build-id
--eh-frame-hdr -m elf_x86_64 --hash-style=gnu --as-needed -dynamic-linker
/lib64/ld-linux-x86-64.so.2 -pie -o poc
/usr/lib/gcc/x86_64-linux-gnu/12/../../../x86_64-linux-gnu/Scrt1.o
/usr/lib/gcc/x86_64-linux-gnu/12/../../../x86_64-linux-gnu/crti.o
/usr/lib/gcc/x86_64-linux-gnu/12/crtbeginS.o -L/usr/lib/gcc/x86_64-linux-gnu/12
-L/usr/lib/gcc/x86_64-linux-gnu/12/../../../x86_64-linux-gnu
-L/usr/lib/gcc/x86_64-linux-gnu/12/../../../../lib -L/lib/x86_64-linux-gnu
-L/lib/../lib -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib
-L/usr/lib/gcc/x86_64-linux-gnu/12/../../.. poc.o -lgcc --push-state
--as-needed -lgcc_s --pop-state -lc -lgcc --push-state --as-needed -lgcc_s
--pop-state /usr/lib/gcc/x86_64-linux-gnu/12/crtendS.o
/usr/lib/gcc/x86_64-linux-gnu/12/../../../x86_64-linux-gnu/crtn.o
COLLECT_GCC_OPTIONS='-v' '-save-temps' '-O2' '-Wall' '-Wextra' '-o' 'poc'
'-mtune=generic' '-march=x86-64' '-dumpdir' 'poc.'

[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---

    union {
        const uint8_t *p8;
        uint32_t *p32;
        uint64_t *p64;
        size_t i;
    } u;


Does not get around aliasing rules of C/C++.

[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So basically since main writes via a double type but then spookyhash_short
reads via a uint64_t type there is an alias violation.

Using an union to change the pointer type does not change there is an alias
violation happening. You need to use memcpy for copying from one type to
another in this case. 

Instead of doing:
c += u.p64[0];

You could do load_64(&u.p64[0]) and have load_64 defined to be:
```
static inline uint64_t load_64(void *a)
{
  uint64_t t;
  memcpy(&t, a, sizeof(t));
  return t;
}
```

[Bug c/115552] wrong code at -O2 and above when strict-aliasing with uint128 and double

[davidhu0903ex3 at gmail dot com]

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

--- Comment #3 from davidhcefx <davidhu0903ex3 at gmail dot com> ---
Thanks for your explanation, we now understand the problem.