Compilation fails with -Werror=strict-aliasing

[tastytea]

Hello! Automated checks over at Gentoo have found that simutrans does not compile with -Werror=strict-aliasing. I've been told that it indicates possible runtime issues and may be even worse when combined with LTO. Since -fstrict-aliasing is implied by -O2 (and therefore -O3), it seems this could be a problem with OPTIMISE := 1 in config.default?

Code Select Expand

dataobj/translator.cc: In function 'char* recode(const char*, bool, bool, bool)':
dataobj/translator.cc:162:107: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
  162 |         *dst++ = c = (uint8)utf8_decoder_t::decode((utf8 const *&)src);
      |                                                                   ^~~

dataobj/translator.cc:165:118: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing[https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing]]
  165 | ++ = c = unicode_to_latin2(utf8_decoder_t::decode((utf8 const *&)src));
      |                                                                  ^~~

The build was done on x86_64, Linux, GCC 12.1. The original bug report (including full log) is here: https://bugs.gentoo.org/859229

[prissi]

That must be a compiler error (or like to document said a false positive), since char is defined in the C standard as the smallest addressable unit. (So I had a signal processor once where one char was 40 bits size ... ) Hence, char cannot have any alignment issues by definition, it is allowed to point to any type. (Same for utf8, which is just defined as

Code Select Expand

typedef unsigned char  utf8;
utf8 and src are both unsigned and signed char * ...

To my knowledge, strict-aliasing rules apply for pointing to structures or anything larger than a char. It should compile with -fstrict-aliasing The warning is included in -Wall.

Anyway, Simutrans (and especially the x64 builds) are not made to be compiled without warnings. Moreover, -Wstrict-aliasing can and does create false positives. Thus treating this as error is a really bad idea on 3rd party working code.

[Ters]

I don't understand this error. The documentation for GCC states that a "character type may alias any other type".

Maybe Gentoo has changed the utf8 type to be something other than a char, but then they have created the error themselves.

[tastytea]

Sorry for the late reply, i misconfigured the e-mail notification settings.

To my knowledge, strict-aliasing rules apply for pointing to structures or anything larger than a char. It should compile with -fstrict-aliasing The warning is included in -Wall.

Yes, it compiles fine without -Werror* and it runs fine, from what i can tell.

Anyway, Simutrans (and especially the x64 builds) are not made to be compiled without warnings. Moreover, -Wstrict-aliasing can and does create false positives. Thus treating this as error is a really bad idea on 3rd party working code.

Agreed. It's not normally compiled with -Werror* in Gentoo, just for occasional test runs. I couldn't tell if it's a bug or a false positive, so i thought it's best to report it.

Maybe Gentoo has changed the utf8 type to be something other than a char, but then they have created the error themselves.

No, utf8 is defined as unsigned char in simutrans, we did not override it.

[ceeac]

Interestingly, the warning is only emitted for GCC 6.1+ with -O2 and higher: https://godbolt.org/z/P334jT1eb. Moving the cast out of the function call silences the warning.

I could not get Clang/MSVC to emit a warning and I cannot think of a reason why this should not work so I am starting to think this may be a GCC bug?

[prissi]

@tastytea
I think notification emails were broken recently (together with all other emails).

@ceeac
A very interesting website. Wished I had something like this 20 years ago when compiler errors were much more frequent.

[ceeac]

Moving the cast out of the function call silences the warning.

Implemented in r10752: Converting char * to unsigned char *& directly is Undefined Behaviour.

[tastytea]

Implemented in r10752: Converting char * to unsigned char *& directly is Undefined Behaviour.

I found another one:

Code Select Expand

squirrel/squirrel/sqcompiler.cc: In member function 'void SQCompiler::EmitLoadConstFloat(SQFloat, SQInteger)':
squirrel/squirrel/sqcompiler.cc:896:69: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  896 |                         _fs->AddInstruction(_OP_LOADFLOAT, target,*(reinterpret_cast<SQInt32 *>(&value)));
      |                                                                    ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'm not familiar with this coding style so i'm not sure i got it right, but this patch gets rid of the warning: fix-undefined-behaviour-when-converting-float-to-int.patch

[ceeac]

I'm not familiar with this coding style so i'm not sure i got it right, but this patch gets rid of the warning

This still invokes Undefined Behaviour; using memcpy does not. Since the bug is also present in the upstream Squirrel library, I have opened a pull request there: https://github.com/albertodemichelis/squirrel/pull/257.

[Dwachs]

Thanks! Seems the fix was already in our code but accidentally removed by an update of the squirrel sources....