Bug 160082 – REGRESSION(r203537): It made many tests crash on ARMv7 Linux platforms

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Csaba Osztrogonác]

GTK Linux ARM Release (Thumb2):
- before: https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/11651
- after: https://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/11662 (still running)

JSCOnly Linux ARMv7 Thumb2 Release:
- before: https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/1232
- after: https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Thumb2%20Release/builds/1243 (still running)

/JSCOnly Linux ARMv7 Traditional Release:
- before: https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Traditional%20Release/builds/1613
- after: https://build.webkit.org/builders/JSCOnly%20Linux%20ARMv7%20Traditional%20Release/builds/1623 (still running)

[Csaba Osztrogonác]

Managed to get a crash backtrace:

Running sunspider-1.0/access-fannkuch.js.no-llint
sunspider-1.0/access-fannkuch.js.no-llint: ASSERTION FAILED: !(reinterpret_cast<intptr_t>(target) & 1)
sunspider-1.0/access-fannkuch.js.no-llint: ../../Source/JavaScriptCore/assembler/ARMv7Assembler.h(2757) : static void JSC::ARMv7Assembler::linkJumpAbsolute(uint16_t*, const uint16_t*, void*)
sunspider-1.0/access-fannkuch.js.no-llint: 1   0xb63d0028 WTFCrash
sunspider-1.0/access-fannkuch.js.no-llint: 2   0xb5c54616 JSC::ARMv7Assembler::linkJumpAbsolute(unsigned short*, unsigned short const*, void*)
sunspider-1.0/access-fannkuch.js.no-llint: 3   0xb5c544ce JSC::ARMv7Assembler::linkJump(void*, JSC::AssemblerLabel, void*)
sunspider-1.0/access-fannkuch.js.no-llint: 4   0xb5c554a4 JSC::AbstractMacroAssembler<JSC::ARMv7Assembler, JSC::MacroAssemblerARMv7>::linkJump(void*, JSC::AbstractMacroAssembler<JSC::ARMv7Assembler, JSC::MacroAssemblerARMv7>::Jump, JSC::CodeLocationLabel)
sunspider-1.0/access-fannkuch.js.no-llint: 5   0xb5c55144 JSC::LinkBuffer::link(JSC::AbstractMacroAssembler<JSC::ARMv7Assembler, JSC::MacroAssemblerARMv7>::Jump, JSC::CodeLocationLabel)
sunspider-1.0/access-fannkuch.js.no-llint: 6   0xb609a95e JSC::JITMathIC<JSC::JITAddGenerator>::generateOutOfLine(JSC::VM&, JSC::CodeBlock*, JSC::FunctionPtr)
sunspider-1.0/access-fannkuch.js.no-llint: 7   0xb6094d52
sunspider-1.0/access-fannkuch.js.no-llint: Segmentation fault
sunspider-1.0/access-fannkuch.js.no-llint: ERROR: Unexpected exit code: 139
FAIL: sunspider-1.0/access-fannkuch.js.no-llint

[Csaba Osztrogonác]

(In reply to comment #1)
> Managed to get a crash backtrace:
> 
> Running sunspider-1.0/access-fannkuch.js.no-llint
> sunspider-1.0/access-fannkuch.js.no-llint: ASSERTION FAILED:
> !(reinterpret_cast<intptr_t>(target) & 1)
> sunspider-1.0/access-fannkuch.js.no-llint:
> ../../Source/JavaScriptCore/assembler/ARMv7Assembler.h(2757) : static void
> JSC::ARMv7Assembler::linkJumpAbsolute(uint16_t*, const uint16_t*, void*)
> sunspider-1.0/access-fannkuch.js.no-llint: 1   0xb63d0028 WTFCrash
> sunspider-1.0/access-fannkuch.js.no-llint: 2   0xb5c54616
> JSC::ARMv7Assembler::linkJumpAbsolute(unsigned short*, unsigned short
> const*, void*)
> sunspider-1.0/access-fannkuch.js.no-llint: 3   0xb5c544ce
> JSC::ARMv7Assembler::linkJump(void*, JSC::AssemblerLabel, void*)
> sunspider-1.0/access-fannkuch.js.no-llint: 4   0xb5c554a4
> JSC::AbstractMacroAssembler<JSC::ARMv7Assembler,
> JSC::MacroAssemblerARMv7>::linkJump(void*,
> JSC::AbstractMacroAssembler<JSC::ARMv7Assembler,
> JSC::MacroAssemblerARMv7>::Jump, JSC::CodeLocationLabel)
> sunspider-1.0/access-fannkuch.js.no-llint: 5   0xb5c55144
> JSC::LinkBuffer::link(JSC::AbstractMacroAssembler<JSC::ARMv7Assembler,
> JSC::MacroAssemblerARMv7>::Jump, JSC::CodeLocationLabel)
> sunspider-1.0/access-fannkuch.js.no-llint: 6   0xb609a95e
> JSC::JITMathIC<JSC::JITAddGenerator>::generateOutOfLine(JSC::VM&,
> JSC::CodeBlock*, JSC::FunctionPtr)
> sunspider-1.0/access-fannkuch.js.no-llint: 7   0xb6094d52
> sunspider-1.0/access-fannkuch.js.no-llint: Segmentation fault
> sunspider-1.0/access-fannkuch.js.no-llint: ERROR: Unexpected exit code: 139
> FAIL: sunspider-1.0/access-fannkuch.js.no-llint

It is an assertion on ARMv7 Thumb2 Linux. Didn't you notice the same
crashes on your internal ARMv7 tester bots. ( Maybe it is a silly question,
because they are completely broken 2 days ago after Filip's hacking. :) )

[Csaba Osztrogonác]

GDB backtrace:
Program received signal SIGSEGV, Segmentation fault.
0xb64b0032 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:323
323         *(int *)(uintptr_t)0xbbadbeef = 0;
(gdb) bt
#0  0xb64b0032 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:323
#1  0xb5d34616 in JSC::ARMv7Assembler::linkJumpAbsolute (writeTarget=0xb2c40eb8, instruction=0xb2c40eb8, target=0xb6f3db41)
    at ../../Source/JavaScriptCore/assembler/ARMv7Assembler.h:2757
#2  0xb5d344ce in JSC::ARMv7Assembler::linkJump (code=0xb2c40eae, from=..., to=0xb6f3db41) at ../../Source/JavaScriptCore/assembler/ARMv7Assembler.h:2231
#3  0xb5d354a4 in JSC::AbstractMacroAssembler<JSC::ARMv7Assembler, JSC::MacroAssemblerARMv7>::linkJump (code=0xb2c40eae, jump=..., target=...)
    at ../../Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:970
#4  0xb5d35144 in JSC::LinkBuffer::link (this=0xbeffe734, jump=..., label=...) at ../../Source/JavaScriptCore/assembler/LinkBuffer.h:143
#5  0xb617a95e in JSC::JITMathIC<JSC::JITAddGenerator>::generateOutOfLine (this=0xb2bdf240, vm=..., codeBlock=0xb27abd40, callReplacement=...)
    at ../../Source/JavaScriptCore/jit/JITMathIC.h:131
#6  0xb6174d52 in JSC::operationValueAddProfiledOptimize (exec=0xbeffeaa8, encodedOp1=-18480168992, encodedOp2=-18480170592, arithProfile=0xb1bf9ed4,
    addIC=0xb2bdf240) at ../../Source/JavaScriptCore/jit/JITOperations.cpp:2300
#7  0xb2c43394 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

[Csaba Osztrogonác]

crash log on ARMv7 with ARM instruction set:

Running stress/exit-after-int52-to-double.js.default
stress/exit-after-int52-to-double.js.default: ASSERTION FAILED: linkBuffer.isValid()
stress/exit-after-int52-to-double.js.default: ../../Source/JavaScriptCore/jit/JITMathIC.h(130) : void JSC::JITMathIC<Generator>::generateOutOfLine(JSC::VM&, JSC::CodeBlock*, JSC::FunctionPtr) [with GeneratorType = JSC::JITAddGenerator]
stress/exit-after-int52-to-double.js.default: 1   0xb6394fb0 WTFCrash
stress/exit-after-int52-to-double.js.default: 2   0xb5ea3104 JSC::JITMathIC<JSC::JITAddGenerator>::generateOutOfLine(JSC::VM&, JSC::CodeBlock*, JSC::FunctionPtr)
stress/exit-after-int52-to-double.js.default: 3   0xb5e9a0b8
stress/exit-after-int52-to-double.js.default: Segmentation fault
stress/exit-after-int52-to-double.js.default: ERROR: Unexpected exit code: 139
FAIL: stress/exit-after-int52-to-double.js.default

[Csaba Osztrogonác]

(In reply to comment #4)
> crash log on ARMv7 with ARM instruction set:
> 
> Running stress/exit-after-int52-to-double.js.default
> stress/exit-after-int52-to-double.js.default: ASSERTION FAILED:
> linkBuffer.isValid()
> stress/exit-after-int52-to-double.js.default:
> ../../Source/JavaScriptCore/jit/JITMathIC.h(130) : void
> JSC::JITMathIC<Generator>::generateOutOfLine(JSC::VM&, JSC::CodeBlock*,
> JSC::FunctionPtr) [with GeneratorType = JSC::JITAddGenerator]
> stress/exit-after-int52-to-double.js.default: 1   0xb6394fb0 WTFCrash
> stress/exit-after-int52-to-double.js.default: 2   0xb5ea3104
> JSC::JITMathIC<JSC::JITAddGenerator>::generateOutOfLine(JSC::VM&,
> JSC::CodeBlock*, JSC::FunctionPtr)
> stress/exit-after-int52-to-double.js.default: 3   0xb5e9a0b8
> stress/exit-after-int52-to-double.js.default: Segmentation fault
> stress/exit-after-int52-to-double.js.default: ERROR: Unexpected exit code:
> 139
> FAIL: stress/exit-after-int52-to-double.js.default

It seems it is a similar to bug159720 .

Can't we disable this new feature somehow similar to https://trac.webkit.org/changeset/203272 ? I can't debug new and new regressions at a pace you
add them. :-/ And it seems nobody else help maintaining ARM Linux ports ...

[Saam Barati]

I ran armv7 locally before I landed on iOS and all jsc stress tests passed. I'll check our bots. It's only failing debug tests? Or release tests too?

[Csaba Osztrogonác]

(In reply to comment #6)
> I ran armv7 locally before I landed on iOS and all jsc stress tests passed.
> I'll check our bots. It's only failing debug tests? Or release tests too?

All ARM buildbots on build.webkit.org are release bots, there are 
1016 crashes on the GTK bot, 113 on the JSCOnly bot. Both of them
are ARMv7 Thumb2 tester. And there are 4141 crashes on the JSCOnly
ARMv7 traditional (ARM instruction set) bot.

I generated the debug crash backtraces locally.

[Saam Barati]

It's probably this line that's crashing:
            linkBuffer.link(jump, CodeLocationLabel(m_code.code().executableAddress()));

Maybe we should be linking the dataAddress? That begs the question of why I didn't see the same crash.

[Saam Barati]

I think the line I posted is the bug. I'll look into it when I get into the office.

Ossy can you try this fix locally that just does:

linkBuffer.link(jump, CodeLocationLabel(m_code.code()));

[Csaba Osztrogonác]

(In reply to comment #9)
> I think the line I posted is the bug. I'll look into it when I get into the
> office.
> 
> Ossy can you try this fix locally that just does:
> 
> linkBuffer.link(jump, CodeLocationLabel(m_code.code()));

Sure, will check in 10 minutes.

[Csaba Osztrogonác]

(In reply to comment #10)
> (In reply to comment #9)
> > I think the line I posted is the bug. I'll look into it when I get into the
> > office.
> > 
> > Ossy can you try this fix locally that just does:
> > 
> > linkBuffer.link(jump, CodeLocationLabel(m_code.code()));
> 
> Sure, will check in 10 minutes.

I checked, it seems it fixes the ARMv7 Thumb2 issue.

[Saam Barati]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > I think the line I posted is the bug. I'll look into it when I get into the
> > > office.
> > > 
> > > Ossy can you try this fix locally that just does:
> > > 
> > > linkBuffer.link(jump, CodeLocationLabel(m_code.code()));
> > 
> > Sure, will check in 10 minutes.
> 
> I checked, it seems it fixes the ARMv7 Thumb2 issue.

Ok. I'll test locally too and then put a patch up for review.
FWIW, this is the behavior that PolymorphicAccess uses, and that's well tested code.
We should adopt this behavior regardless. I'm just curious if I hit asserts from this as well on iOS armv7.

[Saam Barati]

Created attachment 284366 [details]
patch

[Keith Miller]

Comment on attachment 284366 [details]
patch

r=me.

[Saam Barati]

fixed in:
https://trac.webkit.org/changeset/203615

[Csaba Osztrogonác]

(In reply to comment #15)
> fixed in:
> https://trac.webkit.org/changeset/203615

ARM traditional crashes are still valid, I filed a new bug report for it: bug160157 .