Bug 160082

Summary: REGRESSION(r203537): It made many tests crash on ARMv7 Linux platforms
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: benjamin, cgarcia, clopez, commit-queue, keith_miller, mark.lam, msaboff, ossy, saam
Priority: P1    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108645, 159649    
Attachments:
Description Flags
patch keith_miller: review+

Attachments
patch (1.76 KB, patch)
2016-07-22 13:14 PDT, Saam Barati
keith_miller: review+
Csaba Osztrogonác
Comment 1 2016-07-22 04:47:54 PDT
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
Comment 2 2016-07-22 04:58:38 PDT
(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
Comment 3 2016-07-22 06:01:47 PDT
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
Comment 4 2016-07-22 06:09:43 PDT
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
Comment 5 2016-07-22 06:13:19 PDT
(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
Comment 6 2016-07-22 09:08:29 PDT
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
Comment 7 2016-07-22 09:12:09 PDT
(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
Comment 8 2016-07-22 09:17:29 PDT
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
Comment 9 2016-07-22 09:30:56 PDT
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
Comment 10 2016-07-22 09:35:15 PDT
(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
Comment 11 2016-07-22 09:59:55 PDT
(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
Comment 12 2016-07-22 11:30:40 PDT
(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
Comment 13 2016-07-22 13:14:34 PDT
Keith Miller
Comment 14 2016-07-22 13:24:45 PDT
Comment on attachment 284366 [details] patch r=me.
Saam Barati
Comment 15 2016-07-22 13:39:42 PDT
Csaba Osztrogonác
Comment 16 2016-07-25 02:15:08 PDT
(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 .
Note You need to log in before you can comment on or make changes to this bug.