Bug 159083 – THUMB2 support not correctly detected on Fedora with GCC 6.1.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Tomas Popela]

After http://trac.webkit.org/changeset/202214 the build fails with:

    In file included from /home/fedora/tpopela/rpmbuild/BUILD/webkitgtk-2.13.2/Source/JavaScriptCore/assembler/MacroAssemblerARM.h:34:0,
                     from /home/fedora/tpopela/rpmbuild/BUILD/webkitgtk-2.13.2/Source/JavaScriptCore/assembler/MacroAssembler.h:40,
                     from /home/fedora/tpopela/rpmbuild/BUILD/webkitgtk-2.13.2/Source/JavaScriptCore/assembler/LinkBuffer.h:39,
                     from /home/fedora/tpopela/rpmbuild/BUILD/webkitgtk-2.13.2/Source/JavaScriptCore/assembler/LinkBuffer.cpp:27:
    /home/fedora/tpopela/rpmbuild/BUILD/webkitgtk-2.13.2/Source/JavaScriptCore/assembler/AbstractMacroAssembler.h: In instantiation of ‘void JSC::AbstractMacroAssembler<AssemblerType, MacroAssemblerType>::emitNops(size_t) [with AssemblerType = JSC::ARMAssembler; MacroAssemblerType = JSC::MacroAssemblerARM; size_t = unsigned int]’:
    /home/fedora/tpopela/rpmbuild/BUILD/webkitgtk-2.13.2/Source/JavaScriptCore/assembler/LinkBuffer.cpp:232:50:   required from here
    /home/fedora/tpopela/rpmbuild/BUILD/webkitgtk-2.13.2/Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:1049:32: error: ‘fillNops’ is not a member of ‘JSC::ARMAssembler’
             AssemblerType::fillNops(static_cast<char*>(buffer.data()) + startCodeSize, memoryToFillWithNopsInBytes, isCopyingToExecutableMemory);


There is an implementation of fillNops missing in the ARMAssembler.h

[Carlos Alberto Lopez Perez]

The GTK+ ARM bot (ARMv7) seems happy. https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release?numbuilds=50

And it seems there is also an implementation of fillNops in ARMv7Assembler.h

Is this an issue that happens only with ARMv6 ?
In that case, does this still happens if the JIT is disabled (-DENABLE_JIT=OFF) ?

[Tomas Popela]

Ok, the problem here is the detection of THUMB2 support. Our code is based on __thumb2__ or __thumb__ defines. This does work on Debian:

$ gcc -dM -E - < /dev/null | grep -i thumb
#define __thumb2__ 1
#define __THUMB_INTERWORK__ 1
#define __thumb__ 1
#define __ARM_ARCH_ISA_THUMB 2
#define __THUMBEL__ 1

but does not on Fedora 24 with GCC 6.1. :

$ gcc -dM -E - < /dev/null | grep -i thumb
#define __THUMB_INTERWORK__ 1
#define __ARM_ARCH_ISA_THUMB 2

as the previously mentions defines are not presented there.

[Tomas Popela]

Created attachment 282244 [details]
Correct the THUMB2 detection on Fedora with gcc 6.1.

[WebKit Commit Bot]

Comment on attachment 282244 [details]
Correct the THUMB2 detection on Fedora with gcc 6.1.

Clearing flags on attachment: 282244

Committed r202560: <http://trac.webkit.org/changeset/202560>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #2)
> Ok, the problem here is the detection of THUMB2 support. Our code is based
> on __thumb2__ or __thumb__ defines. This does work on Debian:
> 
> $ gcc -dM -E - < /dev/null | grep -i thumb
> #define __thumb2__ 1
> #define __THUMB_INTERWORK__ 1
> #define __thumb__ 1
> #define __ARM_ARCH_ISA_THUMB 2
> #define __THUMBEL__ 1
> 
> but does not on Fedora 24 with GCC 6.1. :
> 
> $ gcc -dM -E - < /dev/null | grep -i thumb
> #define __THUMB_INTERWORK__ 1
> #define __ARM_ARCH_ISA_THUMB 2
> 
> as the previously mentions defines are not presented there.


(In reply to comment #4)
> Comment on attachment 282244 [details]
> Correct the THUMB2 detection on Fedora with gcc 6.1.
> 
> Clearing flags on attachment: 282244
> 
> Committed r202560: <http://trac.webkit.org/changeset/202560>

This change is incorrect, because it enables thumb2 instruction set 
on ARMv7 by default and doesn't work with toolchain uses -marm option:
https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Traditional%20Release/builds/1514

You are right, r202214 broke the build with ARM instruction set -
ARM traditional in WebKit terminology - because of missing fillNops
implementation in ARMAssembler.h. ARMv7Assembler.h is responsible
for ARMv7 with Thumb2 instruction set. note: ARMv6 doesn't build
with enabled JIT long long time ago, at least 2.5 years ago,
see bug125581 and bug141288 for details.

The thumb2 detection in Platform.h was correct, but it seem your
toolchain was changed for some reason. The old one was built to use -mthumb,
but the new one was built to use -marm as default instruction set.

You can check your toolchain default settings with this command:
gcc -Q --help=target | grep -i "\(marm\|mthumb\)"

result if you have thumb2 toolchain:
  -marm                                 [disabled]
  -mthumb                               [enabled]
  -mthumb-interwork                     [enabled]

result if you have ARM toolchain:
  -marm                                 [enabled]
  -mthumb                               [disabled]
  -mthumb-interwork                     [enabled]

I'm going to fix the ARM traditional build soon in a separated bug.

[Tomas Popela]

(In reply to comment #6)
> You can check your toolchain default settings with this command:
> gcc -Q --help=target | grep -i "\(marm\|mthumb\)"
>
> result if you have thumb2 toolchain:
>   -marm                                 [disabled]
>   -mthumb                               [enabled]
>   -mthumb-interwork                     [enabled]
> 
> result if you have ARM toolchain:
>   -marm                                 [enabled]
>   -mthumb                               [disabled]
>   -mthumb-interwork                     [enabled]
> 
> I'm going to fix the ARM traditional build soon in a separated bug.

Sorry Ossy about the breakage and thank you for working on the fix! It is as you said the thumb2 toolchain is indeed disabled and the ARM one enabled.