Bug 159709 - LinkBuffer::linkCode() should put barrier before the constant pool after r202214
Summary: LinkBuffer::linkCode() should put barrier before the constant pool after r202214
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Blocks: 159408
  Show dependency treegraph
Reported: 2016-07-13 03:47 PDT by Csaba Osztrogonác
Modified: 2017-06-20 02:15 PDT (History)
7 users (show)

See Also:

Patch (1.55 KB, patch)
2016-07-13 03:49 PDT, Csaba Osztrogonác
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2016-07-13 03:47:30 PDT

infos about the crash I notice:

sunspider-1.0/3d-cube.js.dfg-maximal-flush-validate-no-cjit:     [1470] get_by_id         loc9, loc8, NumPx(@id22) llint(struct = 0xb29a7ae0 (offset = 103))    predicting Nonboolint32
sunspider-1.0/3d-cube.js.dfg-maximal-flush-validate-no-cjit:         disassembly not available for range 0xb2f13da4...0xb2f13e0c

(gdb) disas 0xb2f13da4,0xb2f13e0c
Dump of assembler code from 0xb2f13da4 to 0xb2f13e0c:
   0xb2f13da4:  ldr     r0, [r11, #-72] ; 0x48
   0xb2f13da8:  ldr     r1, [r11, #-68] ; 0x44
   0xb2f13dac:  cmn     r1, #5
   0xb2f13db0:  bne     0xb2f18c48
   0xb2f13db4:  ldr     r12, [r0]
   0xb2f13db8:  movw    r6, #31456      ; 0x7ae0
   0xb2f13dbc:  movt    r6, #45722      ; 0xb29a
   0xb2f13dc0:  cmp     r12, r6
   0xb2f13dc4:  ldrne   r12, [pc, #16]  ; 0xb2f13ddc
   0xb2f13dc8:  bxne    r12
   0xb2f13dcc:  ldr     r0, [r0, #8]
   0xb2f13dd0:  ldr     r1, [r0, #-36]  ; 0x24
   0xb2f13dd4:  ldr     r0, [r0, #-40]  ; 0x28
----------------------------------------------------------- constant pool, we need a jump here
=> 0xb2f13dd8:  bkpt    0xffff
   0xb2f13ddc:  rscslt  r8, r1, #72, 24 ; 0x4800
   0xb2f13de0:  nop                     ; (mov r0, r0)
   0xb2f13de4:  nop                     ; (mov r0, r0)
   0xb2f13de8:  nop                     ; (mov r0, r0)
   0xb2f13dec:  nop                     ; (mov r0, r0)
   0xb2f13df0:  nop                     ; (mov r0, r0)
   0xb2f13df4:  ldr     r6, [pc, #1848] ; 0xb2f14534
   0xb2f13df8:  str     r0, [r6]
   0xb2f13dfc:  ldr     r6, [pc, #1844] ; 0xb2f14538
   0xb2f13e00:  str     r1, [r6]
   0xb2f13e04:  str     r0, [r11, #-80] ; 0x50
   0xb2f13e08:  str     r1, [r11, #-76] ; 0x4c
End of assembler dump.
(gdb) info registers
r0             0x168    360
r1             0xffffffff       4294967295
r2             0x0      0
r3             0xbed44828       3201583144
r4             0xffffffff       4294967295
r5             0x0      0
r6             0xb29a7ae0       2996468448
r7             0x0      0
r8             0xb29c71e0       2996597216
r9             0xfffffffb       4294967291
r10            0x1      1
r11            0xbed44968       3201583464
r12            0xb29a7ae0       2996468448
sp             0xbed448a0       0xbed448a0
lr             0xb2f13d1c       -1292813028
pc             0xb2f13dd8       0xb2f13dd8
cpsr           0x600f0010       1611595792
Comment 1 Csaba Osztrogonác 2016-07-13 03:49:39 PDT
Created attachment 283507 [details]
Comment 2 Michael Catanzaro 2016-09-08 20:52:25 PDT
Comment on attachment 283507 [details]

Looks like this has been broken for several months, so a few more days makes no difference: please wait a bit for any JSC folks to object before landing. I have no clue what this patch does, except that it looks like something we shouldn't leave sitting in Bugzilla.
Comment 3 Saam Barati 2016-09-11 23:38:26 PDT
Csaba, can you explain what this patch does and why it's necessary?
Comment 4 Csaba Osztrogonác 2016-09-12 02:17:26 PDT
(In reply to comment #3)
> Csaba, can you explain what this patch does and why it's necessary?

Before the IC refactoring/optimization work we didn't need to add jump before
this constant pool, because the control flow didn't run to the constant pool.

But after r202214, we got the crashes can be found in the description of this bug,
because instructions should be executed after ldrs. There are only nops on
platforms which don't have constant pool. But platforms which have constant
pool, should jump over it instead of trying to execute non valid instructions.