#264 Build fails with strict-aliasing violations.

[Eli Schwartz]

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:

-e  [x86_64-pc-linux-gnu-gcc] loader_xsb.c using -fPIC -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 
In file included from cell_xsb.h:77,
                 from loader_xsb.c:56:
loader_xsb.c: In function ‘float_val_to_hash’:
box_defines.h:56:51: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   56 | #define FLOAT_HIGH_16_BITS(float) ((((UInteger)(*((UInteger *)((void *)(& float)))))>>48) & LOW_16_BITS_MASK)
      |                                                  ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:238:49: note: in expansion of macro ‘FLOAT_HIGH_16_BITS’
  238 |   return ((ID_BOXED_FLOAT << BOX_ID_OFFSET ) | (FLOAT_HIGH_16_BITS(Flt))) ^
      |                                                 ^~~~~~~~~~~~~~~~~~
box_defines.h:59:25: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   59 |   (     (((UInteger)(*(((UInteger *)((void *)(& float))))))>>24) & LOW_24_BITS_MASK)
      |                       ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:239:5: note: in expansion of macro ‘FLOAT_MIDDLE_24_BITS’
  239 |     FLOAT_MIDDLE_24_BITS(Flt) ^ FLOAT_LOW_24_BITS(Flt);
      |     ^~~~~~~~~~~~~~~~~~~~
box_defines.h:61:50: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   61 | #define FLOAT_LOW_24_BITS(float) (((UInteger)(*(((UInteger *)((void *)(& float)))))) & LOW_24_BITS_MASK)
      |                                                ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:239:33: note: in expansion of macro ‘FLOAT_LOW_24_BITS’
  239 |     FLOAT_MIDDLE_24_BITS(Flt) ^ FLOAT_LOW_24_BITS(Flt);
      |                                 ^~~~~~~~~~~~~~~~~
In file included from loader_xsb.c:54:
loader_xsb.c: In function ‘get_index_tab’:
loader_xsb.h:87:25: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   87 | #define fix_bb4(lptr) (*(unsigned int *)(lptr) = \
      |                         ^~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:129:45: note: in expansion of macro ‘fix_bb4’
  129 |   {Integer dummy; dummy = get_obj_word(x) ; fix_bb4(x) ;      \
      |                                             ^~~~~~~
loader_xsb.c:270:15: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  270 |     case 'i': get_obj_word_bbsig_notag(&ival);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:130:24: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  130 |     *(Integer *)(x) = *(int *)(x);                                      \
      |                        ^~~~~~~~~~
loader_xsb.c:270:15: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  270 |     case 'i': get_obj_word_bbsig_notag(&ival);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.h:87:25: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   87 | #define fix_bb4(lptr) (*(unsigned int *)(lptr) = \
      |                         ^~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:129:45: note: in expansion of macro ‘fix_bb4’
  129 |   {Integer dummy; dummy = get_obj_word(x) ; fix_bb4(x) ;      \
      |                                             ^~~~~~~
loader_xsb.c:275:7: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  275 |       get_obj_word_bbsig_notag(&ival);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:130:24: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  130 |     *(Integer *)(x) = *(int *)(x);                                      \
      |                        ^~~~~~~~~~
loader_xsb.c:275:7: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  275 |       get_obj_word_bbsig_notag(&ival);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:278:32: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  278 |       val = float_val_to_hash(*(float *)(&ival));
      |                                ^~~~~~~~~~~~~~~~
loader_xsb.h:87:25: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   87 | #define fix_bb4(lptr) (*(unsigned int *)(lptr) = \
      |                         ^~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:129:45: note: in expansion of macro ‘fix_bb4’
  129 |   {Integer dummy; dummy = get_obj_word(x) ; fix_bb4(x) ;      \
      |                                             ^~~~~~~
loader_xsb.c:330:5: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  330 |     get_obj_word_bbsig_notag(&label);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:130:6: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  130 |     *(Integer *)(x) = *(int *)(x);                                      \
      |      ^~~~~~~~~~~~~~
loader_xsb.c:330:5: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  330 |     get_obj_word_bbsig_notag(&label);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:130:24: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  130 |     *(Integer *)(x) = *(int *)(x);                                      \
      |                        ^~~~~~~~~~
loader_xsb.c:330:5: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  330 |     get_obj_word_bbsig_notag(&label);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[1]: *** [Makefile:268: loader_xsb.o] Error 1

Previously reported downstream at https://bugs.gentoo.org/855659
Build log is attached.

[David S. Warren]

So I guess the accepted way to do these things would be to use union types? And storing under one type and retrieving through the other? Hmmm. If that would work, that would require having an extra memory location for the union variable and the copy. If this would work, and it's worth the trouble, I could look into it.

[Eli Schwartz]

The standards conformant way to do this without invoking UB is to use either that (union type access) or a memcpy.

Both are commonly used in this scenario. Both will produce, on gcc or clang with optimizations enabled, the same code (in fact, the same code that the strict-aliasing violation produces, except the aliasing could trigger UB and explode instead). It's a well known pattern that the compiler has been taught to recognize.

The memcpy guarantees there are no alignment issues whereas the union provides no guarantees and it's the job of the code author to verify that isn't a problem. So there's an argument to be made that the memcpy is always the best approach since you are anyways trusting -O2 to do its job for anyone that wants efficient codegen.

[David S. Warren]

Thanks very much for that information. I will look into seeing what can be done.
(I haven't understood that code for handling byte-backward issues for at least 35 years, if I ever did, even though I'm quite sure I wrote it. I hope I'm not too old to relearn it....)

[Eli Schwartz]

I see you committed a fix, unfortunately attempting to backport it emits a different fatal error instead:

In file included from tst_utils.c:45:
cell_xsb_i.h: In function ‘bld_boxedfloat’:
cell_xsb_i.h:42:5: error: implicit declaration of function ‘memcpy’ [-Werror=implicit-function-declaration]
   42 |     memcpy(&tempUIFloat,&value,sizeof(tempUIFloat));
      |     ^~~~~~
cell_xsb_i.h:1:1: note: include ‘<string.h>’ or provide a declaration of ‘memcpy’
  +++ |+#include <string.h>
    1 | #include "register.h"
cell_xsb_i.h:42:5: warning: incompatible implicit declaration of built-in function ‘memcpy’ [-Wbuiltin-declaration-mismatch]
   42 |     memcpy(&tempUIFloat,&value,sizeof(tempUIFloat));
      |     ^~~~~~
cell_xsb_i.h:42:5: note: include ‘<string.h>’ or provide a declaration of ‘memcpy’
cc1: some warnings being treated as errors
make[1]: *** [Makefile:277: tst_utils.o] Error 1

[Eli Schwartz]

Note that implicit-function-declaration is tied to https://sourceforge.net/p/xsb/bugs/265/

This is an easy case because you can just include the header you forgot. Some Modern C errors are a lot harder to fix.

[Eli Schwartz]

Oh hrm right, fixed by edce993d1a837707781dbd4b5025e22e694c9053.

[Eli Schwartz]

Unfortunately after backporting the patches, it still fails:

-e  [x86_64-pc-linux-gnu-gcc] loader_xsb.c using -fPIC -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -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 -Wno-error=format-security 
loader_xsb.c: In function ‘get_index_tab’:
loader_xsb.c:136:16: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  136 |     tempInt = *(int *)(x);                                              \
      |                ^~~~~~~~~~
loader_xsb.c:280:15: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  280 |     case 'i': get_obj_word_bbsig_notag(&ival);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:136:16: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  136 |     tempInt = *(int *)(x);                                              \
      |                ^~~~~~~~~~
loader_xsb.c:285:7: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  285 |       get_obj_word_bbsig_notag(&ival);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~
loader_xsb.c:288:32: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  288 |       val = float_val_to_hash(*(float *)(&ival));
      |                                ^~~~~~~~~~~~~~~~
loader_xsb.c:136:16: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  136 |     tempInt = *(int *)(x);                                              \
      |                ^~~~~~~~~~
loader_xsb.c:340:5: note: in expansion of macro ‘get_obj_word_bbsig_notag’
  340 |     get_obj_word_bbsig_notag(&label);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[1]: *** [Makefile:268: loader_xsb.o] Error 1

Looks like the fix wasn't quite enough, some but not all issues were solved.

float_val_to_hash was overlooked. tempInt is an issue in the replacement code instead.