Bug 159711

Summary: 64-bit alignment check isn't necessary in ARMAssembler::prepareExecutableCopy after r202214
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: JavaScriptCoreAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, keith_miller, mark.lam, msaboff, ossy, saam
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 159408    
Attachments:
Description Flags
Patch none

Csaba Osztrogonác
Reported 2016-07-13 04:01:49 PDT
After r202214 this check is incorrect and causes crashes, because the generated IC code isn't necessarily 64-bit sized.
Attachments
Patch (1.44 KB, patch)
2016-07-13 04:03 PDT, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2016-07-13 04:03:04 PDT
Created attachment 283510 [details] Patch
Csaba Osztrogonác
Comment 2 2016-07-13 04:03:34 PDT
I'm going to attach debug backtrace soon to show what is the problem here.
Csaba Osztrogonác
Comment 3 2016-07-14 04:43:21 PDT
Program received signal SIGBUS, Bus error. 0xb27c42e8 in ?? () (gdb) bt #0 0xb27c42e8 in ?? () #1 0xb6157554 in JSC::slow_path_enter (Cannot access memory at address 0xfffffffb exec=0x0, pc=0xfffffffc) at ../../Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:596 #2 0xbeffe8e8 in ?? () (gdb) disas 0xb27c42b8,+100 Dump of assembler code from 0xb27c42b8 to 0xb27c431c: 0xb27c42b8: movw r6, #31776 ; 0x7c20 0xb27c42bc: movt r6, #45594 ; 0xb21a 0xb27c42c0: cmp r12, r6 0xb27c42c4: ldrne r12, [pc, #16] ; 0xb27c42dc <====== load from constant pool 0xb27c42c8: bxne r12 0xb27c42cc: ldr r1, [r0, #20] 0xb27c42d0: ldr r0, [r0, #16] 0xb27c42d4: b 0xb27c42e0 <===== jump after the constant pool 0xb27c42d8: bkpt 0xffff 0xb27c42dc: rsbslt r5, r12, #84, 24 ; 0x5400 0xb27c42e0: nop ; (mov r0, r0) 0xb27c42e4: nop ; (mov r0, r0) => 0xb27c42e8: bkpt 0x0000 <======= CRASH! We don't need this barrier. 0xb27c42ec: str r0, [r6] 0xb27c42f0: ldr r6, [pc, #1660] ; 0xb27c4974 0xb27c42f4: str r1, [r6] 0xb27c42f8: str r0, [r11, #-168] ; 0xa8 0xb27c42fc: str r1, [r11, #-164] ; 0xa4 0xb27c4300: mov r2, #0 0xb27c4304: mvn r3, #0 0xb27c4308: ldr r0, [r11, #-168] ; 0xa8 0xb27c430c: ldr r1, [r11, #-164] ; 0xa4 0xb27c4310: cmn r1, #5 0xb27c4314: bne 0xb27c5cd0 0xb27c4318: cmn r3, #1 End of assembler dump.
Saam Barati
Comment 4 2016-07-14 18:52:59 PDT
Comment on attachment 283510 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=283510&action=review > Source/JavaScriptCore/assembler/ARMAssembler.cpp:-399 > - if (!m_buffer.isAligned(8)) > - bkpt(0); Will we need this when we're not emitting code over old code?
Csaba Osztrogonác
Comment 5 2016-07-15 07:07:47 PDT
(In reply to comment #4) > Comment on attachment 283510 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=283510&action=review > > > Source/JavaScriptCore/assembler/ARMAssembler.cpp:-399 > > - if (!m_buffer.isAligned(8)) > > - bkpt(0); > > Will we need this when we're not emitting code over old code? Practically we don't need this at all, it is placed to crash in case of implementation bug instead of undefined or bad behaviour.
Csaba Osztrogonác
Comment 6 2016-07-27 07:00:59 PDT
ping?
Mark Lam
Comment 7 2016-07-27 08:10:45 PDT
Comment on attachment 283510 [details] Patch r=me
WebKit Commit Bot
Comment 8 2016-07-28 02:29:24 PDT
Comment on attachment 283510 [details] Patch Clearing flags on attachment: 283510 Committed r203816: <http://trac.webkit.org/changeset/203816>
WebKit Commit Bot
Comment 9 2016-07-28 02:29:28 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.