OSGeo / grass

GRASS GIS - free and open-source geospatial processing engine
https://grass.osgeo.org
Other
800 stars 293 forks source link

Build fails with strict-aliasing violations #3506

Open eli-schwartz opened 4 months ago

eli-schwartz commented 4 months ago

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

wenzeslaus commented 4 months ago

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)) {