https://trac.webkit.org/changeset/130826 made 33 JSC test and 466 layout tests crash on the Qt ARM bot. (last good revision: r130824, ... build fail ..., first bad revision: r130828) results: http://build.webkit.sed.hu/builders/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/builds/7185
ping?
(In reply to comment #1) > ping? You guys are probably doing something strange with branch patching.
Now there are 800 crashing tests on the ARM bot. Zoltán, Gábor, are you going to fix this bug in the near future? Don't you plan maintaining the ARM traditional assembler?
(In reply to comment #4) > ping? I'm working on it. It seems that something goes wrong with the get_by_val operation and the ARMAssembler::getLdrImmAddress function waits a pc-relative ldr instruction but gets something else instead.
Adding it as Qt 5.0.0 release blocker bug. It wouldn't be good to release Qt 5 with this serious bug. Or we should disable DFG JIT on ARM as a workaround. Gábor, so you think if the bug would disappear with disabling DFG JIT?
(In reply to comment #6) > Or we should disable DFG JIT on ARM as a workaround. Gábor, > so you think if the bug would disappear with disabling DFG JIT? Unfortunately disabling the DFG JIT wouldn't solve this problem. I have some debugging information about the crash maybe Filip or someone else with more expertise could figure out something from it: (gdb) info breakpoints Num Type Disp Enb Address What 1 breakpoint keep y 0x002cffd4 in JSC::JIT::privateCompileGetByVal(JSC::ByValInfo*, JSC::ReturnAddressPtr, JSC::JITArrayMode) at /home/bgabor/WebKit/Source/JavaScriptCore/jit/JITPropertyAccess.cpp:1468 2 breakpoint keep n 0x0008f0c0 in JSC::ARMAssembler::getLdrImmAddress(unsigned int*) at /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h:780 (gdb) r Starting program: /home/bgabor/jsc/test-crash/jsc -s -f ecma_3/shell.js -f ecma_3/Object/shell.js -f ecma_3/Object/class-001.js [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/libthread_db.so.1". [New Thread 0x42850450 (LWP 4914)] Breakpoint 1, JSC::JIT::privateCompileGetByVal (this=0xbeffdae0, byValInfo=0x82b6c8, returnAddress=..., arrayMode=JSC::JITArrayStorage) at /home/bgabor/WebKit/Source/JavaScriptCore/jit/JITPropertyAccess.cpp:1468 1468 repatchBuffer.relink(byValInfo->badTypeJump, CodeLocationLabel(byValInfo->stubRoutine->code().code())); (gdb) p byValInfo->badTypeJump $1 = {<JSC::CodeLocationCommon> = {<JSC::MacroAssemblerCodePtr> = {m_value = 0x40022c10}, <No data fields>}, <No data fields>} (gdb) x/i 0x40022c10 0x40022c10: ldr r4, [r0, #3071384] (gdb) enable 2 (gdb) c Continuing. Breakpoint 2, JSC::ARMAssembler::getLdrImmAddress (insn=0x40022c0c) at /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h:783 783 if ((*insn & LdrPcImmediateInstructionMask) != LdrPcImmediateInstruction) { (gdb) x/i 0x40022c0c 0x40022c0c: bne 0x40022e60 (gdb) x/i (0x40022c0c + 0x4) 0x40022c10: ldr r4, [r0, #3071384] (gdb) c Continuing. ASSERTION FAILED: (*insn & BlxInstructionMask) == BlxInstruction /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h(785) : static JSC::ARMWord* JSC::ARMAssembler::getLdrImmAddress(JSC::ARMWord*) 1 0x8f134 /home/bgabor/jsc/test-crash/jsc() [0x8f134] 2 0x93b08 /home/bgabor/jsc/test-crash/jsc() [0x93b08] 3 0x20c6bc /home/bgabor/jsc/test-crash/jsc() [0x20c6bc] 4 0x20ce7c /home/bgabor/jsc/test-crash/jsc() [0x20ce7c] 5 0x20cd84 /home/bgabor/jsc/test-crash/jsc() [0x20cd84] 6 0x2d0028 /home/bgabor/jsc/test-crash/jsc() [0x2d0028] 7 0x2edb88 /home/bgabor/jsc/test-crash/jsc() [0x2edb88] 8 0x2e3da0 /home/bgabor/jsc/test-crash/jsc() [0x2e3da0] 9 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 10 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 11 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 12 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 13 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 14 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 15 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 16 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 17 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 18 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 19 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 20 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 21 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 22 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 23 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 24 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 25 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 26 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 27 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 28 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 29 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 30 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] 31 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] Program received signal SIGSEGV, Segmentation fault. 0x0008f144 in JSC::ARMAssembler::getLdrImmAddress (insn=0x40022c0c) at /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h:785 785 ASSERT((*insn & BlxInstructionMask) == BlxInstruction); (gdb)
Created attachment 170587 [details] Hack for disabling one of the array-related optimizations. And if I disable one of the array-related optimizations as in the attached patch a lot of crashes go away. It is also worth mentioning that the codeblock within the if 0 - endif seems to be completely ignored in Linux-x86.
(In reply to comment #7) > (In reply to comment #6) > > > Or we should disable DFG JIT on ARM as a workaround. Gábor, > > so you think if the bug would disappear with disabling DFG JIT? > > Unfortunately disabling the DFG JIT wouldn't solve this problem. > > I have some debugging information about the crash maybe Filip or someone else with more expertise could figure out something from it: > > > (gdb) info breakpoints > Num Type Disp Enb Address What > 1 breakpoint keep y 0x002cffd4 in JSC::JIT::privateCompileGetByVal(JSC::ByValInfo*, JSC::ReturnAddressPtr, JSC::JITArrayMode) > at /home/bgabor/WebKit/Source/JavaScriptCore/jit/JITPropertyAccess.cpp:1468 > 2 breakpoint keep n 0x0008f0c0 in JSC::ARMAssembler::getLdrImmAddress(unsigned int*) at /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h:780 > (gdb) r > Starting program: /home/bgabor/jsc/test-crash/jsc -s -f ecma_3/shell.js -f ecma_3/Object/shell.js -f ecma_3/Object/class-001.js > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/lib/libthread_db.so.1". > [New Thread 0x42850450 (LWP 4914)] > > Breakpoint 1, JSC::JIT::privateCompileGetByVal (this=0xbeffdae0, byValInfo=0x82b6c8, returnAddress=..., arrayMode=JSC::JITArrayStorage) > at /home/bgabor/WebKit/Source/JavaScriptCore/jit/JITPropertyAccess.cpp:1468 > 1468 repatchBuffer.relink(byValInfo->badTypeJump, CodeLocationLabel(byValInfo->stubRoutine->code().code())); > (gdb) p byValInfo->badTypeJump > $1 = {<JSC::CodeLocationCommon> = {<JSC::MacroAssemblerCodePtr> = {m_value = 0x40022c10}, <No data fields>}, <No data fields>} > (gdb) x/i 0x40022c10 > 0x40022c10: ldr r4, [r0, #3071384] This looks suspicious. Why are we loading from r0 at such a *HUGE* offset? > (gdb) enable 2 > (gdb) c > Continuing. > > Breakpoint 2, JSC::ARMAssembler::getLdrImmAddress (insn=0x40022c0c) at /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h:783 > 783 if ((*insn & LdrPcImmediateInstructionMask) != LdrPcImmediateInstruction) { > (gdb) x/i 0x40022c0c > 0x40022c0c: bne 0x40022e60 So, you're emitting a bne for patchableBranch32, and then you're trying to patch a blx when relink() is called. That's your bug. Either make relink() work with bne, or make patchableBranch32 emit a blx. > (gdb) x/i (0x40022c0c + 0x4) > 0x40022c10: ldr r4, [r0, #3071384] > (gdb) c > Continuing. > ASSERTION FAILED: (*insn & BlxInstructionMask) == BlxInstruction > /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h(785) : static JSC::ARMWord* JSC::ARMAssembler::getLdrImmAddress(JSC::ARMWord*) > 1 0x8f134 /home/bgabor/jsc/test-crash/jsc() [0x8f134] > 2 0x93b08 /home/bgabor/jsc/test-crash/jsc() [0x93b08] > 3 0x20c6bc /home/bgabor/jsc/test-crash/jsc() [0x20c6bc] > 4 0x20ce7c /home/bgabor/jsc/test-crash/jsc() [0x20ce7c] > 5 0x20cd84 /home/bgabor/jsc/test-crash/jsc() [0x20cd84] > 6 0x2d0028 /home/bgabor/jsc/test-crash/jsc() [0x2d0028] > 7 0x2edb88 /home/bgabor/jsc/test-crash/jsc() [0x2edb88] > 8 0x2e3da0 /home/bgabor/jsc/test-crash/jsc() [0x2e3da0] > 9 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 10 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 11 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 12 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 13 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 14 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 15 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 16 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 17 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 18 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 19 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 20 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 21 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 22 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 23 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 24 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 25 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 26 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 27 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 28 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 29 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 30 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > 31 0x2e3b48 /home/bgabor/jsc/test-crash/jsc() [0x2e3b48] > > Program received signal SIGSEGV, Segmentation fault. > 0x0008f144 in JSC::ARMAssembler::getLdrImmAddress (insn=0x40022c0c) at /home/bgabor/WebKit/Source/JavaScriptCore/assembler/ARMAssembler.h:785 > 785 ASSERT((*insn & BlxInstructionMask) == BlxInstruction); > (gdb)
(In reply to comment #9) > So, you're emitting a bne for patchableBranch32, and then you're trying to patch a blx when relink() is called. > > That's your bug. > > Either make relink() work with bne, or make patchableBranch32 emit a blx. > Thank you for your time and your advice. I will follow your suggestion and take a closer look at those areas you have mentioned.
any progress?
Created attachment 173021 [details] Workaround patch This is just a workaround to make the ARM tester buildbot happy. Meanwhile I'm going to work on the proper fix.
Comment on attachment 173021 [details] Workaround patch View in context: https://bugs.webkit.org/attachment.cgi?id=173021&action=review I think we should push this workaround as soon as possible to make ARM_TRADITIONAL tests happy. And I hope Zoltán and Gábor will manage to fix the bug properly on the following weeks. Zoltán? Filip? > Source/JavaScriptCore/jit/JITInlineMethods.h:182 > +/* FIXME: Check if we can do better in ARM. For now we just want to avoid the s/in ARM/on ARM > Source/JavaScriptCore/jit/JITInlineMethods.h:183 > + * following javascriptcore test crashes: s/crashes/crash
I will check this next week.
Created attachment 173263 [details] Rebased fix
(In reply to comment #16) > Created an attachment (id=173263) [details] > Rebased fix Is this workaround still valid? What about the proper fix? The ARM traditional assembler became useless ~1.5 months before. Is there any chance to be fixed in the near future?
Comment on attachment 173263 [details] Rebased fix I don't particularly like this fix. First, it's masking the problem that relink() is assuming that something is a blx when it's actually a bne. Second, if you really want to disable patching of get_by_val/put_by_val then you should do it without preprocessor macros in the JIT's code. That's just ugly and error-prone. I strongly recommend that you do the *really simple* fix of making relink() work with bne, or making patchableBranch32() emit a blx. But I will defer to Zoltan's opinion on this. Zoltan, do you think that what I'm suggesting is sensible? Or do you feel strongly that we should land this workaround as-is?
Created attachment 175969 [details] Proper fix
Comment on attachment 175969 [details] Proper fix Thanks for the fix. View in context: https://bugs.webkit.org/attachment.cgi?id=175969&action=review > Source/JavaScriptCore/assembler/MacroAssemblerARM.h:816 > + ARMWord tmp = (static_cast<unsigned>(imm.m_value) == 0x80000000) ? ARMAssembler::InvalidImmediate : m_assembler.getOp2(-imm.m_value); > + if (tmp != ARMAssembler::InvalidImmediate) > + m_assembler.cmn(reg, tmp); > + else > + m_assembler.cmp(reg, m_assembler.getImm(imm.m_value, ARMRegisters::S0)); Please move this code duplication to a common subfunction.
Created attachment 175985 [details] Refactoring code duplication.
Attachment 175985 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/assembler/MacroAssemblerARM.h:840: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 175987 [details] Style fix
Created attachment 175993 [details] Refactoring compare32 to private.
Created attachment 175994 [details] Style fix
Comment on attachment 175994 [details] Style fix Thanks, r=me
Comment on attachment 175994 [details] Style fix Clearing flags on attachment: 175994 Committed r135717: <http://trac.webkit.org/changeset/135717>
All reviewed patches have been landed. Closing bug.
*** Bug 101832 has been marked as a duplicate of this bug. ***