Bug 129635 – Crash in JIT code while watching a video @ storyboard.tumblr.com

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

We repatch a getById and the code we repatch to is bogus.  This is what the JSC ARMv7 disassembler shows it as:
Generated JIT code for GetById polymorphic list access for zp#AEiZAg:[0x94de000->0xa5b4180->0x6e7c220, DFGFunctionCall, 262], return point 0x55d04ad:
    Code at [0x55d1021, 0x55d10c1):
     0x55d1020:    ldr    r6, [r2]
     0x55d1022:    movw   r12, #45728
     0x55d1026:    movt   r12, #2764
     0x55d102a:    cmp    r6, r12
     0x55d102e:    ittt   ne
     0x55d1030:    nopne
     0x55d1032:    nopne
     0x55d1036:    bne    0x55d0baa
     0x55d103a:    ldr    pc, [r2, #8]  <== here is where we jump into the weeds
     0x55d103e:    .long  fffffc70 (unknown opcode)
     0x55d1042:    mov    r1, r2
     0x55d1044:    .long  fffa4638 (actually vqshlu.s32 d20, d24, #0x1a, also bogus)
     0x55d1048:    mov    r12, #19
     0x55d104c:    str    r12, [r7, #36]
     0x55d1050:    movw   r6, #732
     0x55d1054:    movt   r6, #1704
     0x55d1058:    str    r7, [r6]
     0x55d105a:    movw   r12, #36893
     0x55d105e:    movt   r12, #74
     0x55d1062:    blx    r12
     0x55d1064:    mov    r12, r1
     0x55d1066:    mov    r1, r0
     0x55d1068:    mov    r0, r12
     0x55d106a:    movw   r6, #4596
     0x55d106e:    movt   r6, #1704
     0x55d1072:    ldr    r6, [r6]
     0x55d1074:    cmn    r6, #6
     0x55d1078:    ittt   eq
     0x55d107a:    nopeq
     0x55d107c:    nopeq
     0x55d1080:    beq    0x55d04ac
     0x55d1084:    mov    r1, r7
     0x55d1086:    movw   r0, #45056
     0x55d108a:    movt   r0, #1703
     0x55d108e:    movw   r12, #54953
     0x55d1092:    movt   r12, #74
     0x55d1096:    blx    r12
     0x55d1098:    movw   r6, #4452
     0x55d109c:    movt   r6, #1704
     0x55d10a0:    ldr    r1, [r6]
     0x55d10a2:    bx     r1

It appears that we cannot get a scratchRegister in Repatch.cpp::tryBuildGetByIDList()  and we are using InvalidGPRReg (-1).  The ARMv7 assembler doesn't tolerate this as a register generating instructions.

<rdar://problem/16137985>

[Michael Saboff]

Created attachment 225709 [details]
Patch

[Filip Pizlo]

Comment on attachment 225709 [details]
Patch

Wow.  R=me.

[Filip Pizlo]

Can you test if it's not safe to remove this:

    if (needToRestoreScratch && !slot.isCacheableValue())
        return ProtoChainGenerationFailed;

[Michael Saboff]

Committed r165021: <http://trac.webkit.org/changeset/165021>

[Michael Saboff]

(In reply to comment #3)
> Can you test if it's not safe to remove this:
> 
>     if (needToRestoreScratch && !slot.isCacheableValue())
>         return ProtoChainGenerationFailed;


I plan on looking at that as well in a separate bug (<https://bugs.webkit.org/show_bug.cgi?id=129638> - "Verify that check for InvalidGPR returned from TempRegisterSet::getFreeGPR in Repatch.cpp::generateProtoChainAccessStub() after r165021").