64-bit alignment check isn't necessary in ARMAssembler::prepareExecutableCopy after r202214

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

After r202214 this check is incorrect and causes crashes,
because the generated IC code isn't necessarily 64-bit sized.

[Csaba Osztrogonác]

Created attachment 283510 [details]
Patch

[Csaba Osztrogonác]

I'm going to attach debug backtrace soon to show what is the problem here.

[Csaba Osztrogonác]

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 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]

(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]

ping?

[Mark Lam]

Comment on attachment 283510 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 283510 [details]
Patch

Clearing flags on attachment: 283510

Committed r203816: <http://trac.webkit.org/changeset/203816>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.