129635 2014-03-03 16:27:51 -0800 Crash in JIT code while watching a video @ storyboard.tumblr.com 2014-03-03 16:52:19 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 msaboff msaboff fpizlo oldest_to_newest 986562 0 msaboff 2014-03-03 16:27:51 -0800 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> 986565 1 225709 msaboff 2014-03-03 16:33:43 -0800 Created attachment 225709 Patch 986568 2 225709 fpizlo 2014-03-03 16:39:38 -0800 Comment on attachment 225709 Patch Wow. R=me. 986569 3 fpizlo 2014-03-03 16:40:11 -0800 Can you test if it's not safe to remove this: if (needToRestoreScratch && !slot.isCacheableValue()) return ProtoChainGenerationFailed; 986572 4 msaboff 2014-03-03 16:44:36 -0800 Committed r165021: <http://trac.webkit.org/changeset/165021> 986580 5 msaboff 2014-03-03 16:52:19 -0800 (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"). 225709 2014-03-03 16:33:43 -0800 2014-03-03 16:39:38 -0800 Patch 129635.patch text/plain 2226 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTY1MDE4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBA CisyMDE0LTAzLTAzICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIENyYXNoIGluIEpJVCBjb2RlIHdoaWxlIHdhdGNoaW5nIGEgdmlkZW8gQCBzdG9yeWJvYXJk LnR1bWJsci5jb20KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEyOTYzNQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIENsZWFyIG1fc2V0IGJlZm9yZSB3ZSBzZXQgYml0cyBpbiB0aGUgVGVtcFJlZ2lzdGVyU2V0 KGNvbnN0IFJlZ2lzdGVyU2V0JiBvdGhlcikKKyAgICAgICAgY29uc3RydXRvci4KKworICAgICAg ICAqIGppdC9UZW1wUmVnaXN0ZXJTZXQuY3BwOgorICAgICAgICAoSlNDOjpUZW1wUmVnaXN0ZXJT ZXQ6OlRlbXBSZWdpc3RlclNldCk6IENsZWFyIG1hcCBiZWZvcmUgc2V0dGluZyBpdC4KKyAgICAg ICAgKiBqaXQvVGVtcFJlZ2lzdGVyU2V0Lmg6CisgICAgICAgIChKU0M6OlRlbXBSZWdpc3RlclNl dDo6VGVtcFJlZ2lzdGVyU2V0KTogVXNlIG5ldyBjbGVhckFsbCgpIGhlbHBlci4KKyAgICAgICAg KEpTQzo6VGVtcFJlZ2lzdGVyU2V0OjpjbGVhckFsbCk6IE5ldyBwcml2YXRlIGhlbHBlci4KKwog MjAxNC0wMy0wMyAgQmVuamFtaW4gUG91bGFpbiAgPGJlbmphbWluQHdlYmtpdC5vcmc+CiAKICAg ICAgICAgW3g4Nl0gSW1wcm92ZSBjb2RlIGdlbmVyYXRpb24gb2YgYnl0ZSB0ZXN0CkluZGV4OiBT b3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L1RlbXBSZWdpc3RlclNldC5jcHAKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9UZW1wUmVnaXN0ZXJTZXQuY3BwCShyZXZpc2lv biAxNjUwMTcpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L1RlbXBSZWdpc3RlclNldC5j cHAJKHdvcmtpbmcgY29weSkKQEAgLTM1LDYgKzM1LDggQEAgbmFtZXNwYWNlIEpTQyB7CiAKIFRl bXBSZWdpc3RlclNldDo6VGVtcFJlZ2lzdGVyU2V0KGNvbnN0IFJlZ2lzdGVyU2V0JiBvdGhlcikK IHsKKyAgICBjbGVhckFsbCgpOworCiAgICAgZm9yICh1bnNpZ25lZCBpID0gR1BSSW5mbzo6bnVt YmVyT2ZSZWdpc3RlcnM7IGktLTspIHsKICAgICAgICAgR1BSUmVnIHJlZyA9IEdQUkluZm86OnRv UmVnaXN0ZXIoaSk7CiAgICAgICAgIGlmIChvdGhlci5nZXQocmVnKSkKSW5kZXg6IFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9qaXQvVGVtcFJlZ2lzdGVyU2V0LmgKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L0phdmFTY3JpcHRDb3JlL2ppdC9UZW1wUmVnaXN0ZXJTZXQuaAkocmV2aXNpb24gMTY1MDE3KQor KysgU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9UZW1wUmVnaXN0ZXJTZXQuaAkod29ya2luZyBj b3B5KQpAQCAtMzksOCArMzksNyBAQCBjbGFzcyBUZW1wUmVnaXN0ZXJTZXQgewogcHVibGljOgog ICAgIFRlbXBSZWdpc3RlclNldCgpCiAgICAgewotICAgICAgICBmb3IgKHVuc2lnbmVkIGkgPSBu dW1iZXJPZkJ5dGVzSW5UZW1wUmVnaXN0ZXJTZXQ7IGktLTspCi0gICAgICAgICAgICBtX3NldFtp XSA9IDA7CisgICAgICAgIGNsZWFyQWxsKCk7CiAgICAgfQogICAgIAogICAgIFRlbXBSZWdpc3Rl clNldChjb25zdCBSZWdpc3RlclNldCYpOwpAQCAtMTYyLDYgKzE2MSwxMiBAQCBwdWJsaWM6CiAg ICAgfQogICAgIAogcHJpdmF0ZToKKyAgICB2b2lkIGNsZWFyQWxsKCkKKyAgICB7CisgICAgICAg IGZvciAodW5zaWduZWQgaSA9IG51bWJlck9mQnl0ZXNJblRlbXBSZWdpc3RlclNldDsgaS0tOykK KyAgICAgICAgICAgIG1fc2V0W2ldID0gMDsKKyAgICB9CisKICAgICB2b2lkIHNldEJpdCh1bnNp Z25lZCBpKQogICAgIHsKICAgICAgICAgQVNTRVJUKGkgPCB0b3RhbE51bWJlck9mUmVnaXN0ZXJz KTsK