grass icon indicating copy to clipboard operation
grass copied to clipboard

Build fails with strict-aliasing violations

Open eli-schwartz opened this issue 1 year ago • 3 comments

I tried to compile with LTO: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing

The -Werror=* flags are important to detect cases where the compiler can try to optimize based on assuming UB cannot happen, and miscompile code that has UB in it. strict-aliasing issues are always bad but LTO can make them even worse.

I got this error:

x86_64-pc-linux-gnu-gcc  -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-clash-protection -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types  -fPIC  -I/var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include -I/var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include   -fopenmp -DPACKAGE=\""grasslibs"\"   -I/var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include -I/var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/gpde\" -o OBJ.x86_64-pc-linux-gnu/n_arrays_io.o -c n_arrays_io.c
In file included from /var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include/grass/raster.h:239,
                 from /var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include/grass/raster3d.h:5,
                 from /var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include/grass/N_pde.h:18,
                 from n_arrays_io.c:20:
n_arrays_io.c: In function ‘N_read_rast3d_to_array_3d’:
/var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include/grass/defs/raster.h:404:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  404 |     (*(const FCELL *)(fcellVal) != *(const FCELL *)(fcellVal))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~
n_arrays_io.c:321:25: note: in expansion of macro ‘Rast_is_f_null_value’
  321 |                     if (Rast_is_f_null_value((void *)&f1)) {
      |                         ^~~~~~~~~~~~~~~~~~~~
/var/tmp/portage/sci-geosciences/grass-8.3.1/work/grass-8.3.1/dist.x86_64-pc-linux-gnu/include/grass/defs/raster.h:404:37: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  404 |     (*(const FCELL *)(fcellVal) != *(const FCELL *)(fcellVal))
      |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~
n_arrays_io.c:321:25: note: in expansion of macro ‘Rast_is_f_null_value’
  321 |                     if (Rast_is_f_null_value((void *)&f1)) {
      |                         ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[4]: *** [../../include/Make/Compile.make:32: OBJ.x86_64-pc-linux-gnu/n_arrays_io.o] Error 1

Downstream report: https://bugs.gentoo.org/862579 Full build log: build.log.txt

eli-schwartz avatar Mar 15 '24 06:03 eli-schwartz

The variable is double, but Rast_is_f_null_value is used for the null check. It looks like the following should fix it (I would not mind at all if someone uses this to create a PR):

- if (Rast_is_f_null_value((void *)&f1)) {
+ if (Rast_is_d_null_value(&f1)) {

wenzeslaus avatar Mar 15 '24 16:03 wenzeslaus

There's still another issue there, unfortunately:

[ 80%] Built target r.series
In file included from /tmp/grass/build/output/lib64/grass85/include/grass/raster.h:248,
                 from /tmp/grass/build/output/lib64/grass85/include/grass/raster3d.h:5,
                 from /tmp/grass/build/output/lib64/grass85/include/grass/N_pde.h:18,
                 from /tmp/grass/lib/gpde/n_arrays_io.c:20:
/tmp/grass/lib/gpde/n_arrays_io.c: In function 'N_read_rast3d_to_array_3d':
/tmp/grass/build/output/lib64/grass85/include/grass/defs/raster.h:413:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  413 |     (*(const FCELL *)(fcellVal) != *(const FCELL *)(fcellVal))
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/grass/lib/gpde/n_arrays_io.c:325:25: note: in expansion of macro 'Rast_is_f_null_value'
  325 |                     if (Rast_is_f_null_value(&f1)) {
      |                         ^~~~~~~~~~~~~~~~~~~~
/tmp/grass/build/output/lib64/grass85/include/grass/defs/raster.h:413:37: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  413 |     (*(const FCELL *)(fcellVal) != *(const FCELL *)(fcellVal))
      |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/grass/lib/gpde/n_arrays_io.c:325:25: note: in expansion of macro 'Rast_is_f_null_value'
  325 |                     if (Rast_is_f_null_value(&f1)) {
      |                         ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

i.e. &f1 is a double* but it gets accessed as a const FCELL*.

thesamesam avatar May 18 '25 14:05 thesamesam

Hi @thesamesam! Can you help us and write down the compiler/linker versions and which ones you were using, and the flags you used (or the full command you used to build from source)?

If it is not the same exact environment as the original report that dates a year ago, sometimes it can be helpful too.

We use somewhat strict compiler flags in CI, and check with llvm and gcc, and even very recent toolchain versions with MSYS on windows. I'm surprised we didn't see it fail yet.

echoix avatar May 18 '25 14:05 echoix