| Summary: | Crash in JIT code while watching a video @ storyboard.tumblr.com | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||
| Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | fpizlo | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
Created attachment 225709 [details]
Patch
Comment on attachment 225709 [details]
Patch
Wow. R=me.
Can you test if it's not safe to remove this:
if (needToRestoreScratch && !slot.isCacheableValue())
return ProtoChainGenerationFailed;
Committed r165021: <http://trac.webkit.org/changeset/165021> (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"). |
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>