Segfaults with gcc 13 and `-fstrict-aliasing` about lua-protobuf HOT 12 CLOSED

If I compile with -fstrict-aliasing -Wstrict-aliasing=2, I get some useful warnings - see https://gist.github.com/gszr/84b12df6c5401b4e64fe5e5739405722.

Also, I've tested with this changeset, that refactors some of the "aliasing", and verified it fixes the issue: https://github.com/gszr/lua-protobuf/pull/1/files
(I am not familiar with the library's code, so this was just a proof of concept to verify some assumptions)

from lua-protobuf.

It seems that caused by the lack of Lua stack space, you could try the latest HEAD version of git to see whether it fixed.

from lua-protobuf.

If I compile with -fstrict-aliasing -Wstrict-aliasing=2, I get some useful warnings - see https://gist.github.com/gszr/84b12df6c5401b4e64fe5e5739405722.

Also, I've tested with this changeset, that refactors some of the "aliasing", and verified it fixes the issue: https://github.com/gszr/lua-protobuf/pull/1/files (I am not familiar with the library's code, so this was just a proof of concept to verify some assumptions)

Oh, I see this fix, so it's caused by the optimization of gcc? a pr is welcome :-)

from lua-protobuf.

@starwing - can you take a look at the PR?
FWIW, we might want to disable the -fstrict-aliasing, since that same pattern I remvoed in the PR appears in other places. Doing -O2 without disabling strict-aliasing seems unsafe, and the segfault can happen in other places.

from lua-protobuf.

@gszr it seems the gcc feature, not bug. To compete with clang for performance ... I will accept this pr

from lua-protobuf.

I have made a new commit, please try it out and see whether the warnings still exist.

from lua-protobuf.

@starwing - I will give this a shot soon and report back.

from lua-protobuf.

@starwing I can confirm that fixes the specific aliasing issue I reported. No segfaults with the same testing.
Also, it'd be a good exercise to go through other aliasing issues as reported by https://gist.github.com/gszr/84b12df6c5401b4e64fe5e5739405722 and fix them. These result in undefined behavior if the -fstrict-aliasing is on.

from lua-protobuf.

Also, could you draft a new patch release of the library?

from lua-protobuf.

I have made a new commit which fix the strict aliasing issue, could you check it out and test whether it works?

from lua-protobuf.

@gszr new version drafted.

from lua-protobuf.

@starwing The issue is gone (https://gist.github.com/gszr/05b43652556e0a1f8dd570806a8e914e) - thanks for the fix and new version (could you also update the GH releases page? :)

from lua-protobuf.