82013 2012-03-22 21:57:49 -0700 CALLFRAME_OFFSET and EXCEPTION_OFFSET are same in ctiTrampoline on ARM Thumb2 2012-09-09 14:08:18 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 sg5.lee barraclough barraclough gaborb laszlo.gombos loki zherczeg oldest_to_newest 586080 0 sg5.lee 2012-03-22 21:57:49 -0700 CALLFRAME_OFFSET and EXCEPTION_OFFSET have same value in ctiTrampoline on ARM Thumb2 like followings: In JITStubs.cpp #elif (COMPILER(GCC) || COMPILER(RVCT)) && CPU(ARM_THUMB2) #define THUNK_RETURN_ADDRESS_OFFSET 0x38 #define PRESERVED_RETURN_ADDRESS_OFFSET 0x3C ... #define REGISTER_FILE_OFFSET 0x60 #define CALLFRAME_OFFSET 0x64 #define EXCEPTION_OFFSET 0x64 #define ENABLE_PROFILER_REFERENCE_OFFSET 0x68 I wonder it is intentional or not. I think they should have different offset and back up register value in separate stack location. ( like on MIPS or other platforms ) #define CALLFRAME_OFFSET 0x64 #define EXCEPTION_OFFSET 0x68 #define ENABLE_PROFILER_REFERENCE_OFFSET 0x6C Currently, since CALLFRAME and EXCEPTION have same offset, in ARM_THUMB2 ctiTrampoline code, "str r2, [sp, #" STRINGIZE_VALUE_OF(CALLFRAME_OFFSET) "]" "\n" seems to be dead code. The value in stack location is replaced by r3, with the immediately following statement "str r3, [sp, #" STRINGIZE_VALUE_OF(EXCEPTION_OFFSET) "]" "\n" 587446 1 barraclough 2012-03-25 22:18:34 -0700 Uggh, so, the ctiTrampoline is now no longer passed an exception out pointer. Rather than re-jig the arguments in all asm (which we really should do at some point), we left an unused argument in for now. So the 'exception' argument no longer really exists, and is completely unused - other that the fact some entry/exit thunks still preserve it around JIT code. This code should really be deleted (along with all reference to EXCEPTION_OFFSET). The unused exception value is trampling over the call frame, why does this not crash? Well, when we wrote the THUMB2 entry thunk we dumbly spilled all arguments to the stack, since that's where they were on x86 - and the CTI callbacks access these values. But since the call frame may move (it is the JS stack pointer, and updated by JIT code), we cannot rely on the value upon entry to to JIT code, and instead must save a fresh copy each time a call out to C code is made. As such, I think the store of the call frame is also probably completely redundant, and should likely also be removed. I'd need to look closer to determine if this is true. So, my guess is, no actually bug here, just some stupid, wasteful, and potentially confusing & bug prone code. :-/ 715182 2 162923 barraclough 2012-09-07 17:56:20 -0700 Created attachment 162923 Fix 715183 3 barraclough 2012-09-07 17:59:15 -0700 Fixed in r127944 715590 4 162923 ggaren 2012-09-09 14:08:18 -0700 Comment on attachment 162923 Fix r=me 162923 2012-09-07 17:56:20 -0700 2012-09-09 14:08:18 -0700 Fix 82013.patch text/plain 4426 barraclough SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTI3OTQzKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBA CisyMDEyLTA5LTA3ICBHYXZpbiBCYXJyYWNsb3VnaCAgPGJhcnJhY2xvdWdoQGFwcGxlLmNvbT4K KworICAgICAgICBDQUxMRlJBTUVfT0ZGU0VUIGFuZCBFWENFUFRJT05fT0ZGU0VUIGFyZSBzYW1l IGluIGN0aVRyYW1wb2xpbmUgb24gQVJNIFRodW1iMgorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODIwMTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBOZWl0aGVyIG9mIHRoZXNlIHZhbHVlcyBuZWVkIHRvIGJl IHN0b3JlZC4gQXQgYWxsLgorCisgICAgICAgICogaml0L0pJVFN0dWJzLmNwcDoKKyAgICAgICAg KEpTQyk6CisgICAgICAgIChKU0M6OmN0aVRyYW1wb2xpbmUpOgorICAgICAgICAoSlNDOjpKSVRU aHVua3M6OkpJVFRodW5rcyk6CisgICAgICAgICAgICAtIE5vdGhpbmcgdG8gc2VlIGhlcmUuIE1v dmUgYWxvbmcuCisKIDIwMTItMDktMDcgIFNoZXJpZmYgQm90ICA8d2Via2l0LnJldmlldy5ib3RA Z21haWwuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0IHIxMjc5MzguCklu ZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L0pJVFN0dWJzLmNwcAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L0pJVFN0dWJzLmNwcAkocmV2aXNpb24gMTI3OTE3 KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9KSVRTdHVicy5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTIwNCw4ICsyMDQsNiBAQCBTWU1CT0xfU1RSSU5HKGN0aU9wVGhyb3dOb3RDYXVnaHQp ICI6IiAiCiAjZGVmaW5lIFBSRVNFUlZFRF9SMTBfT0ZGU0VUICAgICAgICAgICAgIDB4NTgKICNk ZWZpbmUgUFJFU0VSVkVEX1IxMV9PRkZTRVQgICAgICAgICAgICAgMHg1QwogI2RlZmluZSBSRUdJ U1RFUl9GSUxFX09GRlNFVCAgICAgICAgICAgICAweDYwCi0jZGVmaW5lIENBTExGUkFNRV9PRkZT RVQgICAgICAgICAgICAgICAgIDB4NjQKLSNkZWZpbmUgRVhDRVBUSU9OX09GRlNFVCAgICAgICAg ICAgICAgICAgMHg2NAogI2RlZmluZSBGSVJTVF9TVEFDS19BUkdVTUVOVCAgICAgICAgICAgICAw eDY4CiAKICNlbGlmIChDT01QSUxFUihHQ0MpIHx8IENPTVBJTEVSKE1TVkMpIHx8IENPTVBJTEVS KFJWQ1QpKSAmJiBDUFUoQVJNX1RSQURJVElPTkFMKQpAQCAtMjgzLDggKzI4MSw2IEBAIGV4dGVy biAiQyIgewogI2RlZmluZSBQUkVTRVJWRURfUkVUVVJOX0FERFJFU1NfT0ZGU0VUIDc2CiAjZGVm aW5lIFRIVU5LX1JFVFVSTl9BRERSRVNTX09GRlNFVCA4MAogI2RlZmluZSBSRUdJU1RFUl9GSUxF X09GRlNFVCAgICAgICAgODQKLSNkZWZpbmUgQ0FMTEZSQU1FX09GRlNFVCAgICAgICAgICAgIDg4 Ci0jZGVmaW5lIEVYQ0VQVElPTl9PRkZTRVQgICAgICAgICAgICA5MgogI2RlZmluZSBHTE9CQUxf REFUQV9PRkZTRVQgICAgICAgICAxMDAKICNkZWZpbmUgU1RBQ0tfTEVOR1RIICAgICAgICAgICAg ICAgMTA0CiAjZWxpZiBDUFUoU0g0KQpAQCAtNDYzLDggKzQ1OSw2IEBAIFNZTUJPTF9TVFJJTkco Y3RpVHJhbXBvbGluZSkgIjoiICJcbiIKICAgICAibGkgICAgJDE3LDUxMiAgICAgICMgc2V0IHRp bWVvdXRDaGVja1JlZ2lzdGVyIiAiXG4iCiAgICAgIm1vdmUgICQyNSwkNCAgICAgICAjIG1vdmUg ZXhlY3V0YWJsZUFkZHJlc3MgdG8gdDkiICJcbiIKICAgICAic3cgICAgJDUsIiBTVFJJTkdJWkVf VkFMVUVfT0YoUkVHSVNURVJfRklMRV9PRkZTRVQpICIoJDI5KSAjIHN0b3JlIHJlZ2lzdGVyRmls ZSB0byBjdXJyZW50IHN0YWNrIiAiXG4iCi0gICAgInN3ICAgICQ2LCIgU1RSSU5HSVpFX1ZBTFVF X09GKENBTExGUkFNRV9PRkZTRVQpICIoJDI5KSAgICAgIyBzdG9yZSBjYWxsRnJhbWUgdG8gY3Vy ZW50IHN0YWNrIiAiXG4iCi0gICAgInN3ICAgICQ3LCIgU1RSSU5HSVpFX1ZBTFVFX09GKEVYQ0VQ VElPTl9PRkZTRVQpICIoJDI5KSAgICAgIyBzdG9yZSBleGNlcHRpb24gdG8gY3VycmVudCBzdGFj ayIgIlxuIgogICAgICJsdyAgICAkOSwiIFNUUklOR0laRV9WQUxVRV9PRihTVEFDS19MRU5HVEgg KyAyMCkgIigkMjkpICAgICMgbG9hZCBnbG9iYWxEYXRhIGZyb20gcHJldmlvdXMgc3RhY2siICJc biIKICAgICAiamFsciAgJDI1IiAiXG4iCiAgICAgInN3ICAgICQ5LCIgU1RSSU5HSVpFX1ZBTFVF X09GKEdMT0JBTF9EQVRBX09GRlNFVCkgIigkMjkpICAgIyBzdG9yZSBnbG9iYWxEYXRhIHRvIGN1 cnJlbnQgc3RhY2siICJcbiIKQEAgLTU1Miw4ICs1NDYsNiBAQCBTWU1CT0xfU1RSSU5HKGN0aVRy YW1wb2xpbmUpICI6IiAiXG4iCiAgICAgInN0ciByMTAsIFtzcCwgIyIgU1RSSU5HSVpFX1ZBTFVF X09GKFBSRVNFUlZFRF9SMTBfT0ZGU0VUKSAiXSIgIlxuIgogICAgICJzdHIgcjExLCBbc3AsICMi IFNUUklOR0laRV9WQUxVRV9PRihQUkVTRVJWRURfUjExX09GRlNFVCkgIl0iICJcbiIKICAgICAi c3RyIHIxLCBbc3AsICMiIFNUUklOR0laRV9WQUxVRV9PRihSRUdJU1RFUl9GSUxFX09GRlNFVCkg Il0iICJcbiIKLSAgICAic3RyIHIyLCBbc3AsICMiIFNUUklOR0laRV9WQUxVRV9PRihDQUxMRlJB TUVfT0ZGU0VUKSAiXSIgIlxuIgotICAgICJzdHIgcjMsIFtzcCwgIyIgU1RSSU5HSVpFX1ZBTFVF X09GKEVYQ0VQVElPTl9PRkZTRVQpICJdIiAiXG4iCiAgICAgIm1vdiByNSwgcjIiICJcbiIKICAg ICAibW92IHI2LCAjNTEyIiAiXG4iCiAgICAgImJseCByMCIgIlxuIgpAQCAtNjgxLDggKzY3Myw2 IEBAIF9fYXNtIEVuY29kZWRKU1ZhbHVlIGN0aVRyYW1wb2xpbmUodm9pZCoKICAgICBzdHIgcjEw LCBbc3AsICMgUFJFU0VSVkVEX1IxMF9PRkZTRVQgXQogICAgIHN0ciByMTEsIFtzcCwgIyBQUkVT RVJWRURfUjExX09GRlNFVCBdCiAgICAgc3RyIHIxLCBbc3AsICMgUkVHSVNURVJfRklMRV9PRkZT RVQgXQotICAgIHN0ciByMiwgW3NwLCAjIENBTExGUkFNRV9PRkZTRVQgXQotICAgIHN0ciByMywg W3NwLCAjIEVYQ0VQVElPTl9PRkZTRVQgXQogICAgIG1vdiByNSwgcjIKICAgICBtb3YgcjYsICM1 MTIKICAgICBibHggcjAKQEAgLTgwNyw3ICs3OTcsNiBAQCBKSVRUaHVua3M6OkpJVFRodW5rcyhK U0dsb2JhbERhdGEqIGdsb2JhCiAgICAgQVNTRVJUKE9CSkVDVF9PRkZTRVRPRihzdHJ1Y3QgSklU U3RhY2tGcmFtZSwgcHJlc2VydmVkUjExKSA9PSBQUkVTRVJWRURfUjExX09GRlNFVCk7CiAKICAg ICBBU1NFUlQoT0JKRUNUX09GRlNFVE9GKHN0cnVjdCBKSVRTdGFja0ZyYW1lLCByZWdpc3RlckZp bGUpID09IFJFR0lTVEVSX0ZJTEVfT0ZGU0VUKTsKLSAgICBBU1NFUlQoT0JKRUNUX09GRlNFVE9G KHN0cnVjdCBKSVRTdGFja0ZyYW1lLCBjYWxsRnJhbWUpID09IENBTExGUkFNRV9PRkZTRVQpOwog ICAgIC8vIFRoZSBmaWZ0aCBhcmd1bWVudCBpcyB0aGUgZmlyc3QgaXRlbSBhbHJlYWR5IG9uIHRo ZSBzdGFjay4KICAgICBBU1NFUlQoT0JKRUNUX09GRlNFVE9GKHN0cnVjdCBKSVRTdGFja0ZyYW1l LCB1bnVzZWQxKSA9PSBGSVJTVF9TVEFDS19BUkdVTUVOVCk7CiAKQEAgLTgyNyw4ICs4MTYsNiBA QCBKSVRUaHVua3M6OkpJVFRodW5rcyhKU0dsb2JhbERhdGEqIGdsb2JhCiAgICAgQVNTRVJUKE9C SkVDVF9PRkZTRVRPRihzdHJ1Y3QgSklUU3RhY2tGcmFtZSwgcHJlc2VydmVkUmV0dXJuQWRkcmVz cykgPT0gUFJFU0VSVkVEX1JFVFVSTl9BRERSRVNTX09GRlNFVCk7CiAgICAgQVNTRVJUKE9CSkVD VF9PRkZTRVRPRihzdHJ1Y3QgSklUU3RhY2tGcmFtZSwgdGh1bmtSZXR1cm5BZGRyZXNzKSA9PSBU SFVOS19SRVRVUk5fQUREUkVTU19PRkZTRVQpOwogICAgIEFTU0VSVChPQkpFQ1RfT0ZGU0VUT0Yo c3RydWN0IEpJVFN0YWNrRnJhbWUsIHJlZ2lzdGVyRmlsZSkgPT0gUkVHSVNURVJfRklMRV9PRkZT RVQpOwotICAgIEFTU0VSVChPQkpFQ1RfT0ZGU0VUT0Yoc3RydWN0IEpJVFN0YWNrRnJhbWUsIGNh bGxGcmFtZSkgPT0gQ0FMTEZSQU1FX09GRlNFVCk7Ci0gICAgQVNTRVJUKE9CSkVDVF9PRkZTRVRP RihzdHJ1Y3QgSklUU3RhY2tGcmFtZSwgdW51c2VkMSkgPT0gRVhDRVBUSU9OX09GRlNFVCk7CiAg ICAgQVNTRVJUKE9CSkVDVF9PRkZTRVRPRihzdHJ1Y3QgSklUU3RhY2tGcmFtZSwgZ2xvYmFsRGF0 YSkgPT0gR0xPQkFMX0RBVEFfT0ZGU0VUKTsKIAogI2VuZGlmCg==