albertodemichelis / squirrel

Official repository for the programming language Squirrel
http://www.squirrel-lang.org
MIT License
894 stars 148 forks source link

Multiple build errors: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] #261

Open band-a-prend opened 1 year ago

band-a-prend commented 1 year ago

When trying to build squirrel 3.1 or 3.2 with -flto -Werror=strict-aliasing enabled (without passing -fno-strict-aliasing) the multiple errors dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] occur (example for quirrel 3.2 with other warnings):

/usr/bin/x86_64-pc-linux-gnu-g++ -D_SQ64 -Dsqstdlib_EXPORTS -I/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/include  -march=core2 -O2 -pipe -fPIC -fno-rtti -fno-exceptions -Wall -Wextra -pedantic -Wcast-qual -flto -Werror=strict-aliasing -O3 -g -std=gnu++11 -MD -MT sqstdlib/CMakeFiles/sqstdlib.dir/sqstdblob.cpp.o -MF sqstdlib/CMakeFiles/sqstdlib.dir/sqstdblob.cpp.o.d -o sqstdlib/CMakeFiles/sqstdlib.dir/sqstdblob.cpp.o -c /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/sqstdlib/sqstdblob.cpp
FAILED: sqstdlib/CMakeFiles/sqstdlib.dir/sqstdblob.cpp.o
/usr/bin/x86_64-pc-linux-gnu-g++ -D_SQ64 -Dsqstdlib_EXPORTS -I/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/include  -march=core2 -O2 -pipe -fPIC -fno-rtti -fno-exceptions -Wall -Wextra -pedantic -Wcast-qual -flto -Werror=strict-aliasing -O3 -g -std=gnu++11 -MD -MT sqstdlib/CMakeFiles/sqstdlib.dir/sqstdblob.cpp.o -MF sqstdlib/CMakeFiles/sqstdlib.dir/sqstdblob.cpp.o.d -o sqstdlib/CMakeFiles/sqstdlib.dir/sqstdblob.cpp.o -c /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/sqstdlib/sqstdblob.cpp
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/sqstdlib/sqstdblob.cpp: In function ‘SQInteger _g_blob_casti2f(HSQUIRRELVM)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/sqstdlib/sqstdblob.cpp:192:22: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  192 |     sq_pushfloat(v,*((const SQFloat *)&i));
      |                     ~^~~~~~~~~~~~~~~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/sqstdlib/sqstdblob.cpp: In function ‘SQInteger _g_blob_castf2i(HSQUIRRELVM)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/sqstdlib/sqstdblob.cpp:200:24: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  200 |     sq_pushinteger(v,*((const SQInteger *)&f));
      |                       ~^~~~~~~~~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors
/usr/bin/x86_64-pc-linux-gnu-g++ -D_SQ64 -Dsquirrel_EXPORTS -I/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/include  -march=core2 -O2 -pipe -fPIC -fno-rtti -fno-exceptions -Wall -Wextra -pedantic -Wcast-qual -flto -Werror=strict-aliasing -O3 -g -std=gnu++11 -MD -MT squirrel/CMakeFiles/squirrel.dir/sqcompiler.cpp.o -MF squirrel/CMakeFiles/squirrel.dir/sqcompiler.cpp.o.d -o squirrel/CMakeFiles/squirrel.dir/sqcompiler.cpp.o -c /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp
FAILED: squirrel/CMakeFiles/squirrel.dir/sqcompiler.cpp.o
/usr/bin/x86_64-pc-linux-gnu-g++ -D_SQ64 -Dsquirrel_EXPORTS -I/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/include  -march=core2 -O2 -pipe -fPIC -fno-rtti -fno-exceptions -Wall -Wextra -pedantic -Wcast-qual -flto -Werror=strict-aliasing -O3 -g -std=gnu++11 -MD -MT squirrel/CMakeFiles/squirrel.dir/sqcompiler.cpp.o -MF squirrel/CMakeFiles/squirrel.dir/sqcompiler.cpp.o.d -o squirrel/CMakeFiles/squirrel.dir/sqcompiler.cpp.o -c /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp
In file included from /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp:14:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.h: In copy constructor ‘SQExceptionTrap::SQExceptionTrap(const SQExceptionTrap&)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.h:23:60: warning: implicitly-declared ‘SQExceptionTrap& SQExceptionTrap::operator=(const SQExceptionTrap&)’ is deprecated [-Wdeprecated-copy]
   23 |     SQExceptionTrap(const SQExceptionTrap &et) { (*this) = et;  }
      |                                                            ^~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.h:23:5: note: because ‘SQExceptionTrap’ has user-provided ‘SQExceptionTrap::SQExceptionTrap(const SQExceptionTrap&)’
   23 |     SQExceptionTrap(const SQExceptionTrap &et) { (*this) = et;  }
      |     ^~~~~~~~~~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp: In member function ‘void SQCompiler::EmitLoadConstFloat(SQFloat, SQInteger)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp:896:57: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  896 |             _fs->AddInstruction(_OP_LOADFLOAT, target,*((SQInt32 *)&value));
      |                                                        ~^~~~~~~~~~~~~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp: In member function ‘void SQCompiler::ParseTableOrClass(SQInteger, SQInteger)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp:1008:17: warning: this statement may fall through [-Wimplicit-fallthrough=]
 1008 |                 if(separator == ',') { //only works for tables
      |                 ^~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqcompiler.cpp:1013:13: note: here
 1013 |             default :
      |             ^~~~~~~
cc1plus: some warnings being treated as errors
/usr/bin/x86_64-pc-linux-gnu-g++ -D_SQ64 -Dsquirrel_EXPORTS -I/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/include  -march=core2 -O2 -pipe -fPIC -fno-rtti -fno-exceptions -Wall -Wextra -pedantic -Wcast-qual -flto -Werror=strict-aliasing -O3 -g -std=gnu++11 -MD -MT squirrel/CMakeFiles/squirrel.dir/sqvm.cpp.o -MF squirrel/CMakeFiles/squirrel.dir/sqvm.cpp.o.d -o squirrel/CMakeFiles/squirrel.dir/sqvm.cpp.o -c /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp
FAILED: squirrel/CMakeFiles/squirrel.dir/sqvm.cpp.o
/usr/bin/x86_64-pc-linux-gnu-g++ -D_SQ64 -Dsquirrel_EXPORTS -I/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/include  -march=core2 -O2 -pipe -fPIC -fno-rtti -fno-exceptions -Wall -Wextra -pedantic -Wcast-qual -flto -Werror=strict-aliasing -O3 -g -std=gnu++11 -MD -MT squirrel/CMakeFiles/squirrel.dir/sqvm.cpp.o -MF squirrel/CMakeFiles/squirrel.dir/sqvm.cpp.o.d -o squirrel/CMakeFiles/squirrel.dir/sqvm.cpp.o -c /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp
In file included from /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:8:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.h: In copy constructor ‘SQExceptionTrap::SQExceptionTrap(const SQExceptionTrap&)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.h:23:60: warning: implicitly-declared ‘SQExceptionTrap& SQExceptionTrap::operator=(const SQExceptionTrap&)’ is deprecated [-Wdeprecated-copy]
   23 |     SQExceptionTrap(const SQExceptionTrap &et) { (*this) = et;  }
      |                                                            ^~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.h:23:5: note: because ‘SQExceptionTrap’ has user-provided ‘SQExceptionTrap::SQExceptionTrap(const SQExceptionTrap&)’
   23 |     SQExceptionTrap(const SQExceptionTrap &et) { (*this) = et;  }
      |     ^~~~~~~~~~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp: In member function ‘bool SQVM::Execute(SQObjectPtr&, SQInteger, SQInteger, SQObjectPtr&, SQBool, SQVM::ExecutionType)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:746:44: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  746 |             case _OP_LOADFLOAT: TARGET = *((const SQFloat *)&arg1); continue;
      |                                           ~^~~~~~~~~~~~~~~~~~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:948:43: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
  948 |                     val._unVal.fFloat = *((const SQFloat *)&arg1);
      |                                          ~^~~~~~~~~~~~~~~~~~~~~~~
In file included from /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqobject.h:5,
                 from /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqpcheader.h:17,
                 from /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:4:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/squtils.h: In instantiation of ‘void sqvector<T>::remove(SQUnsignedInteger) [with T = SQObjectPtr; SQUnsignedInteger = long long unsigned int]’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqarray.h:83:23:   required from here
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/squtils.h:97:20: warning: ‘void* memmove(void*, const void*, size_t)’ writing to an object of type ‘struct SQObjectPtr’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
   97 |             memmove(&_vals[idx], &_vals[idx+1], sizeof(T) * (_size - idx - 1));
      |             ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqpcheader.h:17,
                 from /var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:4:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqobject.h:205:8: note: ‘struct SQObjectPtr’ declared here
  205 | struct SQObjectPtr : public SQObject
      |        ^~~~~~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp: In member function ‘bool SQVM::ObjCmp(const SQObjectPtr&, const SQObjectPtr&, SQInteger&)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:236:13: warning: this statement may fall through [-Wimplicit-fallthrough=]
  236 |             }
      |             ^
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:238:9: note: here
  238 |         default:
      |         ^~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp: In member function ‘bool SQVM::ToString(const SQObjectPtr&, SQObjectPtr&)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:317:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
  317 |         }
      |         ^
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:318:5: note: here
  318 |     default:
      |     ^~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp: In member function ‘bool SQVM::FOREACH_OP(SQObjectPtr&, SQObjectPtr&, SQObjectPtr&, SQObjectPtr&, SQInteger, int, int&)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:576:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
  576 |         }
      |         ^
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:577:5: note: here
  577 |     default:
      |     ^~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp: In member function ‘bool SQVM::Execute(SQObjectPtr&, SQInteger, SQInteger, SQObjectPtr&, SQBool, SQVM::ExecutionType)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:825:23: warning: this statement may fall through [-Wimplicit-fallthrough=]
  825 |                       }
      |                       ^
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:826:21: note: here
  826 |                     default:
      |                     ^~~~~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:762:31: warning: this statement may fall through [-Wimplicit-fallthrough=]
  762 |                               }
      |                               ^
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:763:13: note: here
  763 |             case _OP_CALL: {
      |             ^~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp: In member function ‘SQInteger SQVM::FallBackGet(const SQObjectPtr&, const SQObjectPtr&, SQObjectPtr&)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:1338:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
 1338 |         if(_delegable(self)->_delegate) {
      |         ^~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:1345:5: note: here
 1345 |     case OT_INSTANCE: {
      |     ^~~~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp: In member function ‘SQInteger SQVM::FallBackSet(const SQObjectPtr&, const SQObjectPtr&, const SQObjectPtr&)’:
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:1409:9: warning: this statement may fall through [-Wimplicit-fallthrough=]
 1409 |         if(_table(self)->_delegate) {
      |         ^~
/var/tmp/portage/dev-lang/squirrel-3.2/work/squirrel3/squirrel/sqvm.cpp:1413:5: note: here
 1413 |     case OT_INSTANCE:
      |     ^~~~
cc1plus: some warnings being treated as errors

This results in compilation errors where squirrel is used as subproject (see Code::Blocks issue: https://sourceforge.net/p/codeblocks/tickets/1303/).

Steps to reproduce:

  1. Patch CmakeLists.txt (example for squirrel 3.2) to provide -flto -Werror=strict-aliasing flags and remove -fno-strict-aliasing:
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -18,11 +18,12 @@
 if(CMAKE_COMPILER_IS_GNUCXX)
   add_compile_options(
     "$<$<COMPILE_LANGUAGE:CXX>:-fno-rtti;-fno-exceptions>"
-    -fno-strict-aliasing
     -Wall
     -Wextra
     -pedantic
     -Wcast-qual
+    -flto
+    -Werror=strict-aliasing
     "$<$<CONFIG:Release>:-O3>"
     "$<$<CONFIG:RelWithDebInfo>:-O3;-g>"
     "$<$<CONFIG:MinSizeRel>:-Os>"
  1. Compile in several jobs (or one job but only one of error will appear).
  2. Appropriate errors result in compilation failures.
zeromus commented 1 year ago

Workaround: compile files without the offending options.

band-a-prend commented 1 year ago

Workaround: compile files without the offending options.

Already temporary workarounded (https://bugs.gentoo.org/858338) but several distributives currently prefer to build packages with LTO enabled by default or planning to do this.

It seems that https://github.com/albertodemichelis/squirrel/pull/257 fixes one of error for sqcompiler.cpp

zeromus commented 1 year ago

LTO necessarily implies no-strict-aliasing?

band-a-prend commented 1 year ago

To reproduce issue the -fno-strict-aliasing is disabled and -Werror=strict-aliasing is enabled. I'm not sure but if this error shows that to avoid UB the proper/more accurate type casting should be used? Sorry, my knowledge of C++ are too superficial and I easily could be mislead of the compiler option interpretation.

rversteegen commented 1 year ago

It's incorrect to compile Squirrel without -fno-strict-aliasing, the compiler might perform incorrect optimisations. If Squirrel is embedded in another project which you want to compile without -fno-strict-aliasing, then use different compiler arguments for Squirrel. Note that -fstrict-aliasing just enables some optimisations, it's not important.

Squirrel compiles fine (except the static library which produces ar: plugin needed to handle lto object) with -flto -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing added to the existing flags without removing -fno-strict-aliasing, I just tried it. So don't remove that.

band-a-prend commented 1 year ago

@rversteegen Thank you for clarifications. As a maintainer of the affected package I was needed to report the observed issue. How to handle it is up to the upstream developers decision as they know the prehistory of project and it's specificity.

rversteegen commented 1 year ago

Some changes to the code could be made to avoid breaking the strict aliasing rules, by using unions in SQInstruction, not the ugly solution used in #257. I might do that soon.

It would be a good idea to do so, to avoid depending on compiler flags, which can get lost when embedded in another project.