Godot: GDNative build failure on X11 armv7hl

[akien-mga]

Godot version:
Godot 3.0 (4a042b1)

OS/device including version:
Mageia 7, armv7hl

Issue description:
Attempting to build the current master on Mageia 7 (Cauldron)'s buildsystem, it passes fine on i586 and x86_64 but fails on armv7hl with the following error:

build_gdnative_api_struct(["modules/gdnative/include/gdnative_api_struct.gen.h", "modules/gdnative/gdnative_api_struct.gen.cpp"], ["modules/gdnative/gdnative_api.json"])
Compiling ==> modules/gdnative/gdnative.cpp
Compiling ==> modules/gdnative/register_types.cpp
Compiling ==> modules/gdnative/gdnative/pool_arrays.cpp
Compiling ==> modules/gdnative/gdnative/vector2.cpp
Compiling ==> modules/gdnative/gdnative/rid.cpp
Compiling ==> modules/gdnative/gdnative/variant.cpp
Compiling ==> modules/gdnative/gdnative/basis.cpp
Compiling ==> modules/gdnative/gdnative/quat.cpp
modules/gdnative/gdnative/variant.cpp: In function 'godot_variant godot_variant_call(godot_variant*, const godot_string*, const godot_variant**, godot_int, godot_variant_call_error*)':
modules/gdnative/gdnative/variant.cpp:40:81: internal compiler error: in assign_temp, at function.c:968
 #define memnew_placement_custom(m_placement, m_class, m_constr) _post_initialize(new (m_placement, sizeof(m_class), "") m_constr)
                                                                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
modules/gdnative/gdnative/variant.cpp:449:2: note: in expansion of macro 'memnew_placement_custom'
  memnew_placement_custom(dest, Variant, Variant(self->call(*method, args, p_argcount, error)));
  ^~~~~~~~~~~~~~~~~~~~~~~
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://bugs.mageia.org/> for instructions.
scons: *** [modules/gdnative/gdnative/variant.x11.opt.tools.32.o] Error 1
scons: building terminated because of errors.

Build command:

/usr/bin/scons -j4 'CCFLAGS=-O2 -g -pipe -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -march=armv7-a -mfloat-abi=hard -mfpu=vfpv3-d16' 'LINKFLAGS= -Wl,--as-needed -Wl,--no-undefined -Wl,-z,relro -Wl,-O1 -Wl,--build-id -Wl,--enable-new-dtags' builtin_freetype=no builtin_libogg=no builtin_libpng=no builtin_libtheora=no builtin_libvorbis=no builtin_libvpx=no builtin_libwebp=no builtin_openssl=no builtin_opus=no builtin_pcre2=no builtin_zlib=no udev=yes progress=no p=x11 tools=yes target=release_debug

Full build log: https://hastebin.com/edufijugih
GCC version: 7.3.0

CC @karroffel

[hpvb]

This is an ICE, that is always a compiler bug. This should be reported upstream.

[bruvzg]

GCC specific bug.
Tested on Debian Stretch chroot under Android 7.1.1 (ARMv7-A, Qualcomm Krait 450).
Build command: scons -j4 p=x11 tools=yes target=release_debug

gcc 6.3.0-18 - same error.
clang 3.8.1-24 - builds without errors and runs.

[hpvb]

Tested this with GCC 7.3.0 and it still crashes

modules/gdnative/gdnative/variant.cpp: In function 'godot_variant godot_variant_call(godot_variant*, const godot_string*, const godot_variant**, godot_int, godot_variant_call_error*)':
modules/gdnative/gdnative/variant.cpp:40:81: internal compiler error: in assign_temp, at function.c:968
 #define memnew_placement_custom(m_placement, m_class, m_constr) _post_initialize(new (m_placement, sizeof(m_class), "") m_constr)
                                                                 ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
modules/gdnative/gdnative/variant.cpp:449:2: note: in expansion of macro 'memnew_placement_custom'
  memnew_placement_custom(dest, Variant, Variant(self->call(*method, args, p_argcount, error)));
  ^~~~~~~~~~~~~~~~~~~~~~~
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://gcc.gnu.org/bugs/> for instructions.

I'll try to find a workaround and also submit the bug report to GCC.

[hpvb]

Upstream bugreport created: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84881

[hpvb]

It is a duplicate of this bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79085 the original report has some more information which may lead me to a workaround for the time being.

[akien-mga]

Sounds like we should just avoid -Os on armv7hl. Probably wouldn't be a big issue to reduce to -O3 or -O2 (and honestly, I doubt many will run Godot on armv7hl, apart from RaspberryPi maybe).

[hpvb]

We build with -O2 now, not -Os. The error does not appear with -O0 so we could just jury rig Scons to only build that one file with -O0 instead of -O2. That would be the easiest solution. It doesn't look like this particular file will benefit much from optimization :)

[hpvb]

OK, gcc 7.3.0 can successfully build that file with the following options: -O3 -fno-tree-fre -fno-tree-dominator-opts -fno-tree-copy-prop -fno-tree-ccp -fno-code-hoisting -fno-tree-pre -fno-tree-vrp I'll add a little workaround to the scons file to add these options to that directory and for now we should be fine.

[hpvb]

OK, I've uploaded a test build here : https://tmm.cx/nextcloud/s/XAoPE3ptWpt5zcx can someone who has an armv7 machine test this build before I try to make a PR to fix this issue?

[hpvb]

The underlaying bug got fixed in gcc trunk and will be in 8.1, I'll add my workaround to scons with a gcc version filter of >= 6 < 8.1

[bruvzg]

I've uploaded a test build here : https://tmm.cx/nextcloud/s/XAoPE3ptWpt5zcx can someone who has an armv7 machine test this build before I try to make a PR to fix this issue?

Works OK on Linux chroot under Android 7.1.2 (ARMv7-A)

[hpvb]

Thanks @bruvzg. I'll prepare a PR for the changes.

[akien-mga]

I'll prepare a PR for the changes.

Bump @hpvb :)

For the reference, here's the patch I'm using for the Fedora package: https://src.fedoraproject.org/rpms/godot/blob/master/f/godot-3.0.2-workaround-gcc-ice-armv7hl.patch

[BeayemX]

I just failed to build the latest version (older versions also don't work) on a Raspberry Pi 3b running Raspbian.

Some system information:

$ uname -a
Linux raspberry 4.14.81-v7+ #1163 SMP Thu Nov 15 17:16:30 GMT 2018 armv7l GNU/Linux

$ gcc --version
gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516

$ glxinfo | grep "OpenGL"
...
OpenGL ES profile version string: OpenGL ES 3.0 Mesa 13.0.6
...

The build command I tried

scons p=x11 -j 3 tools=no target=release arch=arm64

I tried to use the workaround provided by @akien-mga by adding

env_gdnative.Append(CCFLAGS="-O3 -fno-tree-fre -fno-tree-dominator-opts -fno-tree-copy-prop -fno-tree-ccp -fno-code-hoisting -fno-tree-pre -fno-tree-vrp".split())  

to modules/gdnative/SCsub but I get the following error

[Initial build] Compiling ==> modules/gdnative/gdnative.cpp
g++: error: unrecognized command line option '-fno-code-hoisting'; did you mean '-Wno-type-hiding'?
[Initial build] Compiling ==> modules/gdnative/register_types.cpp
[Initial build] g++: error: unrecognized command line option '-fno-code-hoisting'; did you mean '-Wno-type-hiding'?
[Initial build] scons: *** [modules/gdnative/gdnative.x11.opt.arm64.o] Error 1
scons: *** [modules/gdnative/register_types.x11.opt.arm64.o] Error 1
Compiling ==> modules/gdnative/android/android_gdn.cpp
g++: error: unrecognized command line option '-fno-code-hoisting'; did you mean '-Wno-type-hiding'?
scons: *** [modules/gdnative/android/android_gdn.x11.opt.arm64.o] Error 1
scons: building terminated because of errors.

When I remove the flag -fno-code-hoisting I get lots of errors like this:

In file included from thirdparty/libvpx/vp8/common/arm/neon/copymem_neon.c:11:0:
thirdparty/libvpx/vp8/common/arm/neon/copymem_neon.c: In function 'vp8_copy_mem8x4_neon':
/usr/lib/gcc/arm-linux-gnueabihf/6/include/arm_neon.h:9440:1: error: inlining failed in call to always_inline 'vst1_u8': target specific option mismatch
 vst1_u8 (uint8_t * __a, uint8x8_t __b)
thirdparty/libvpx/vp8/common/arm/neon/copymem_neon.c:23:9: note: called from here
         vst1_u8(dst, vtmp);

I am not really familiar with build systems and I already googled all these errors but nothing so far has worked.
When searching for the flag -fno-code-hoisting it does not return any useful results at all. Are these godot-specific flags?

Any help would be appreciated.

[akien-mga]

When searching for the flag -fno-code-hoisting it does not return any useful results at all. Are these godot-specific flags?

You should search for "gcc fcode-hoisting" without the -, otherwise most search engines would consider that you don't want to have results with this string. Anyway, it's a new option in GCC 7+ so that's why it doesn't work for you, but it's fine to remove it from the workaround.

The libvpx build issue is different from the issue in this report, so it would be better to report it in its own ticket. As a workaround you can build with module_webm_enabled=no.

[marxin]

Note that month ago we released GCC 7.4 that has the problem fixed:
https://www.gnu.org/software/gcc/gcc-7/

As GCC 7.x is the oldes supported branch right now, I would recommend to remove any work-around for RPI and request to use at lease GCC 7.4.0+.

Rule of thumb: Use latest possible GCC release. If not possible, update to the latest minor update of the GCC branch. Always use a GCC branch that's supported: Supported Releases (https://gcc.gnu.org/).

[akien-mga]

Bumping minimum GCC requirement to 7.4.0+ will simply mean that people will use Clang instead, as it's easier to install manually if they don't have the right GCC version shipped in their distro. There are many supported distros which are stuck on GCC 5 or 6, and we still have to support those. Our own CI runs on GCC 4.8 to ensure that we don't break the build for older versions.

See e.g. Debian Stretch (6.3): https://packages.debian.org/stretch/gcc
Fedora 28 (8.0.1, still affected by this bug): https://fedora.pkgs.org/28/fedora-i386/gcc-8.0.1-0.20.fc28.i686.rpm.html

So it's easier to maintain a small workaround for old GCC versions than to drop support for them.

We do build official releases with supported GCC versions, but we can't force all users to run on rolling distros :)

[marxin]

Bumping minimum GCC requirement to 7.4.0+ will simply mean that people will use Clang instead, as it's easier to install manually if they don't have the right GCC version shipped in their distro.

Note the GCC has almost no dependencies and one can build it easily with ../configure && make && make install. Why do you think it's easier to build Clang than GCC?

There are many supported distros which are stuck on GCC 5 or 6, and we still have to support those. > Our own CI runs on GCC 4.8 to ensure that we don't break the build for older versions.

See e.g. Debian Stretch (6.3): https://packages.debian.org/stretch/gcc
Fedora 28 (8.0.1, still affected by this bug): https://fedora.pkgs.org/28/fedora-i386/gcc-8.0.1-0.20.fc28.i686.rpm.html

I understand that, but disadvantage of the that is that we don't backport critical bugs (as miscompilations) into old branches. And one can easily end up with a binary that is miscompiled due to usage of an old toolchain.

So it's easier to maintain a small workaround for old GCC versions than to drop support for them.

Sure, go ahead with the work-around. One idea I have is that you can print a gentle warning when you detect a legacy toolchain (e.g. 4.8.0). You can print caveat that you can't guarantee built binary will work fine.

We do build official releases with supported GCC versions, but we can't force all users to run on rolling distros :)

Thumbs up for it ;)

[marxin]

Fedora 28 (8.0.1, still affected by this bug): https://fedora.pkgs.org/28/fedora-i386/gcc-8.0.1-0.20.fc28.i686.rpm.html

This package is a GCC development branch snapshot before GCC 8.1 was released. If I see correctly, there are available official releases in Alternatives:
https://fedora.pkgs.org/28/fedora-i386/gcc-8.0.1-0.20.fc28.i686.rpm.html

[akien-mga]

Why do you think it's easier to build Clang than GCC?

I did not refer to building from source, but using pre-compiled binaries, such as those provided for LLVM. IINM GCC does not provide pre-compiled binaries for the most common arch/distros, as it's expected to be available on all distros out of the box (but with the caveat that it can be an unsupported version, as seen above).

One idea I have is that you can print a gentle warning when you detect a legacy toolchain (e.g. 4.8.0). You can print caveat that you can't guarantee built binary will work fine.

Yes that's a good idea, I'll add such a warning for GCC < 7.3 and LLVM < 5.0.2 (not sure what's their support policy, but I see that released 5.0.2 mid-2018, so I guess it was/might still be supported).

[marxin]

Why do you think it's easier to build Clang than GCC?

I did not refer to building from source, but using pre-compiled binaries, such as those provided for LLVM. IINM GCC does not provide pre-compiled binaries for the most common arch/distros, as it's expected to be available on all distros out of the box (but with the caveat that it can be an unsupported version, as seen above).

Makes sense to me!

One idea I have is that you can print a gentle warning when you detect a legacy toolchain (e.g. 4.8.0). You can print caveat that you can't guarantee built binary will work fine.

Yes that's a good idea, I'll add such a warning for GCC < 7.3 and LLVM < 5.0.2 (not sure what's their support policy, but I see that released 5.0.2 mid-2018, so I guess it was/might still be supported).

Good. Can you please then add the workaround for ARM conditionally, similarly to what was removed in https://github.com/godotengine/godot/commit/d9eb6a5b206b0a2f67022b7bae32fb8e46badb95#diff-86df594b7be96561efbf45b55bac081cL2683