Bug 214966 – REGRESSION(r264688): fails to build on armhf

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Alberto Garcia]

This happens with GCC 9 and GCC 10, here's the error message:

/usr/bin/c++  -DBUILDING_GTK__=1 -DBUILDING_WITH_CMAKE=1 -DBWRAP_EXECUTABLE=\"/usr/bin/bwrap\" -DDBUS_PROXY_EXECUTABLE=\"/usr/bin/xdg-dbus-proxy\" -DGETTEXT_PACKAGE=\"WebKit2GTK-4.0\" -DHAVE_CONFIG_H=1 -DJSC_COMPILATION -DJSC_GLIB_API_ENABLED -DSTATICALLY_LINKED_WITH_WTF -DSVN_REVISION=\"tarball\" -DWEBKITGTK_API_VERSION_STRING=\"4.0\" -IDerivedSources/ForwardingHeaders -I/usr/include/glib-2.0 -I/usr/lib/arm-linux-gnueabihf/glib-2.0/include -I. -I../Source/JavaScriptCore -I../Source/JavaScriptCore/API -I../Source/JavaScriptCore/assembler -I../Source/JavaScriptCore/b3 -I../Source/JavaScriptCore/b3/air -I../Source/JavaScriptCore/bindings -I../Source/JavaScriptCore/builtins -I../Source/JavaScriptCore/bytecode -I../Source/JavaScriptCore/bytecompiler -I../Source/JavaScriptCore/dfg -I../Source/JavaScriptCore/disassembler -I../Source/JavaScriptCore/disassembler/ARM64 -I../Source/JavaScriptCore/disassembler/udis86 -I../Source/JavaScriptCore/domjit -I../Source/JavaScriptCore/ftl -I../Source/JavaScriptCore/heap -I../Source/JavaScriptCore/debugger -I../Source/JavaScriptCore/inspector -I../Source/JavaScriptCore/inspector/agents -I../Source/JavaScriptCore/inspector/augmentable -I../Source/JavaScriptCore/inspector/remote -I../Source/JavaScriptCore/interpreter -I../Source/JavaScriptCore/jit -I../Source/JavaScriptCore/llint -I../Source/JavaScriptCore/parser -I../Source/JavaScriptCore/profiler -I../Source/JavaScriptCore/runtime -I../Source/JavaScriptCore/tools -I../Source/JavaScriptCore/wasm -I../Source/JavaScriptCore/wasm/js -I../Source/JavaScriptCore/yarr -IDerivedSources/JavaScriptCore -IDerivedSources/JavaScriptCore/inspector -IDerivedSources/JavaScriptCore/runtime -IDerivedSources/JavaScriptCore/yarr -IDerivedSources/ForwardingHeaders/JavaScriptCore/glib -IDerivedSources/JavaScriptCore/javascriptcoregtk/jsc -I../Source/JavaScriptCore/API/glib -IDerivedSources/JavaScriptCore/javascriptcoregtk -I../Source/JavaScriptCore/inspector/remote/glib -fdiagnostics-color=always -Wextra -Wall -Wno-expansion-to-defined -Wno-noexcept-type -Wno-psabi -Wno-maybe-uninitialized -Wwrite-strings -Wundef -Wpointer-arith -Wmissing-format-attribute -Wformat-security -Wcast-align -g1 -O2 -fdebug-prefix-map=/home/berto/webkit2gtk-2.29.4=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fno-strict-aliasing -fno-exceptions -fno-rtti -fPIC   -std=c++17 -MD -MT Source/JavaScriptCore/CMakeFiles/LowLevelInterpreterLib.dir/llint/LowLevelInterpreter.cpp.o -MF Source/JavaScriptCore/CMakeFiles/LowLevelInterpreterLib.dir/llint/LowLevelInterpreter.cpp.o.d -o Source/JavaScriptCore/CMakeFiles/LowLevelInterpreterLib.dir/llint/LowLevelInterpreter.cpp.o -c ../Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
/tmp/ccYqDpA7.s: Assembler messages:
/tmp/ccYqDpA7.s:208: Error: r13 not allowed here -- `subs sp,r7,#28'
/tmp/ccYqDpA7.s:360: Error: r13 not allowed here -- `subs sp,r7,#28'

Full logs here: https://buildd.debian.org/status/logs.php?pkg=webkit2gtk&arch=armhf

[Michael Catanzaro]

This is also hitting GNOME OS, but Fedora rawhide is OK.

[Michael Catanzaro]

Anyway the problem seems to be GCC generating invalid assembler. I guess we'll try to build with clang and hope for the best.

[Carlos Alberto Lopez Perez]

Since when happens this? I wonder if could be caused by r259734 ?

[Carlos Alberto Lopez Perez]

(In reply to Carlos Alberto Lopez Perez from comment #3)
> Since when happens this? I wonder if could be caused by r259734 ?

No, forget what i said.. this was already included in 2.29.1

[Carlos Alberto Lopez Perez]

2.29.3 was branched on r264060
2.29.4 was branched on r265032

It has to be some commit in the middle, right? This ones are the commits that touch JSC/llint in that range http://sprunge.us/UuSQCt

[Alberto Garcia]

Created attachment 405844 [details]
Patch

This was broken on r264688.

The problem seems to be that SP cannot be used as a destination register.

This patch fixes the build for me, it's the same approach used in r163179 (see bug 127903).

[Michael Catanzaro]

Comment on attachment 405844 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405844&action=review

Thanks Berto!

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:340
> +    if ARMv7
> +        subp cfr, CalleeRegisterSaveSize, t3
> +        move t3, sp
> +    else
> +        subp cfr, CalleeRegisterSaveSize, sp
> +    end

I guess there's no particularly compelling reason to keep this fallback path, right? We could avoid the need for the condition by just always going through the t3 register.

[Michael Catanzaro]

Ah well, I guess that as long as the fallbacks from r163179 still exist, your patch should match those, so probably better to stick with your version.

[EWS]

Committed r265217: <https://trac.webkit.org/changeset/265217>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405844 [details].