150760 2015-10-31 16:22:13 -0700 Wrong value recovery for DFG try/catch with a getter that throws during an IC miss 2015-11-02 17:20:26 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 149409 1 saam saam benjamin commit-queue fpizlo ggaren gskachkov keith_miller mark.lam msaboff oliver sukolsak ysuzuki oldest_to_newest 1138234 0 saam 2015-10-31 16:22:13 -0700 This program breaks in the DFG. Probably because we're assigning the base and the result to the same register and the IC fails. ``` function assert(b) { if (!b) throw new Error("bad value") } noInline(assert); let oThrow = { x: 20, y: 40, z: 50, get f() { throw new Error("Hello World!"); } }; let o1 = { x: 20, f: 40 }; let o2 = { x: 20, y: 50, get f() { return 20; } }; function foo(f) { let o = f(); try { o = o.f; } catch(e) { print(o); // Make this not undefined. assert(o === oThrow); } } noInline(foo); let i; let flag = false; function f() { if (flag) return oThrow; if (i % 2) return o1; return o2; } noInline(f); for (i = 0; i < 10000; i++) { foo(f); } flag = true; foo(f); ``` I discovered this while solving this issue in FTL try/catch. 1138235 1 saam 2015-10-31 16:26:16 -0700 What I think is happening. The IC misses, but the operationGetByIdOptimize stores the result of the call into the result register. Even though the C call throws, we store the result. And then, we try to recover the base from the register we stored the result into because the result and base are the same register. This is a big bag of fail. 1138744 2 saam 2015-11-02 16:01:23 -0800 (In reply to comment #1) > What I think is happening. > The IC misses, but the operationGetByIdOptimize stores the result > of the call into the result register. Even though the C call throws, > we store the result. And then, we try to recover the base from > the register we stored the result into because the result and base > are the same register. This is a big bag of fail. I've actually realized that the problem is deeper than this. I do think the situation I'm describing is a problem, but I've realized that the result of the call in this example is looking dead to DFG OSR exit when we use PhantomLocal as the liveness preservation mechanism. I'm going to switch the code back to using Flush so that I can land FTL try/catch, and then I will investigate this problem further. 1138746 3 saam 2015-11-02 16:03:07 -0800 (In reply to comment #2) > (In reply to comment #1) > > What I think is happening. > > The IC misses, but the operationGetByIdOptimize stores the result > > of the call into the result register. Even though the C call throws, > > we store the result. And then, we try to recover the base from > > the register we stored the result into because the result and base > > are the same register. This is a big bag of fail. > > I've actually realized that the problem is deeper than this. > I do think the situation I'm describing is a problem, but > I've realized that the result of the call in this example > is looking dead to DFG OSR exit when we use PhantomLocal > as the liveness preservation mechanism. I'm going > to switch the code back to using Flush so that I can > land FTL try/catch, and then I will investigate this problem > further. Let me elaborate: the result of the call looks dead after we do flushRegisters(). 1138767 4 264646 saam 2015-11-02 16:49:22 -0800 Created attachment 264646 patch 1138768 5 264646 ggaren 2015-11-02 16:51:03 -0800 Comment on attachment 264646 patch r=me 1138769 6 commit-queue 2015-11-02 16:52:04 -0800 Attachment 264646 did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp:133: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style. 1138779 7 saam 2015-11-02 17:20:26 -0800 landed in: http://trac.webkit.org/changeset/191930 264646 2015-11-02 16:49:22 -0800 2015-11-02 16:51:03 -0800 patch a-backup.diff text/plain 5075 saam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTkxOTI1KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI4IEBA CisyMDE1LTExLTAyICBTYWFtIGJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAg IFdyb25nIHZhbHVlIHJlY292ZXJ5IGZvciBERkcgdHJ5L2NhdGNoIHdpdGggYSBnZXR0ZXIgdGhh dCB0aHJvd3MgZHVyaW5nIGFuIElDIG1pc3MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTE1MDc2MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIFRoaXMgaXMgcmVsYXRlZCB0byB1c2luZyBQaGFudG9tTG9jYWwg aW5zdGVhZCBvZiBGbHVzaCBhcyAKKyAgICAgICAgdGhlIGxpdmVuZXNzIHByZXNlcnZhdGlvbiBt ZWNoYW5pc20gZm9yIGxpdmUgY2F0Y2ggdmFyaWFibGVzLiAKKyAgICAgICAgSSdtIHRlbXBvcmFy aWx5IHN3aXRjaGluZyB0aGluZ3MgYmFjayB0byBGbHVzaC4gVGhpcyB3aWxsIGJlIGEKKyAgICAg ICAgcGVyZm9ybWFuY2UgaGl0IGZvciB0cnkvY2F0Y2ggaW4gdGhlIERGRy4gTGFuZGluZyB0aGlz IHBhdGNoLAorICAgICAgICB0aG91Z2gsIHdpbGwgYWxsb3cgbWUgdG8gbGFuZCB0cnkvY2F0Y2gg aW4gdGhlIEZUTC4gSXQgYWxzbworICAgICAgICBtYWtlcyB0cnkvY2F0Y2ggaW4gdGhlIERGRyBz b3VuZC4gSSBoYXZlIG9wZW5lZCBhbm90aGVyCisgICAgICAgIGJ1ZyB0byBmdXJ0aGVyIGludmVz dGlnYXRlIHVzaW5nIFBoYW50b21Mb2NhbCBhcyB0aGUKKyAgICAgICAgbGl2ZW5lc3MgcHJlc2Vy dmF0aW9uIG1lY2hhbmlzbTogaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTE1MDgyNAorCisgICAgICAgICogZGZnL0RGR0xpdmVDYXRjaFZhcmlhYmxlUHJlc2VydmF0aW9u UGhhc2UuY3BwOgorICAgICAgICAoSlNDOjpERkc6OkxpdmVDYXRjaFZhcmlhYmxlUHJlc2VydmF0 aW9uUGhhc2U6OmhhbmRsZUJsb2NrKToKKyAgICAgICAgKiB0ZXN0cy9zdHJlc3MvZGZnLXRyeS1j YXRjaC13cm9uZy12YWx1ZS1yZWNvdmVyeS1vbi1pYy1taXNzLmpzOiBBZGRlZC4KKyAgICAgICAg KGFzc2VydCk6CisgICAgICAgIChsZXQub1Rocm93LmdldCBmKToKKyAgICAgICAgKGxldC5vMi5n ZXQgZik6CisgICAgICAgIChmb28pOgorICAgICAgICAoZik6CisKIDIwMTUtMTEtMDIgIEFuZHkg RXN0ZXMgIDxhZXN0ZXNAYXBwbGUuY29tPgogCiAgICAgICAgIFtDb2NvYV0gQWRkIHR2T1MgYW5k IHdhdGNoT1MgdG8gU1VQUE9SVEVEX1BMQVRGT1JNUwpJbmRleDogU291cmNlL0phdmFTY3JpcHRD b3JlL2RmZy9ERkdMaXZlQ2F0Y2hWYXJpYWJsZVByZXNlcnZhdGlvblBoYXNlLmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0xpdmVDYXRjaFZhcmlhYmxlUHJl c2VydmF0aW9uUGhhc2UuY3BwCShyZXZpc2lvbiAxOTE5MDMpCisrKyBTb3VyY2UvSmF2YVNjcmlw dENvcmUvZGZnL0RGR0xpdmVDYXRjaFZhcmlhYmxlUHJlc2VydmF0aW9uUGhhc2UuY3BwCSh3b3Jr aW5nIGNvcHkpCkBAIC0xMTgsNyArMTE4LDcgQEAgcHVibGljOgogICAgICAgICAgICAgICAgICAg ICAgICAgICAgIHZhcmlhYmxlQWNjZXNzRGF0YSA9IG5ld1ZhcmlhYmxlQWNjZXNzRGF0YShvcGVy YW5kKTsKIAogICAgICAgICAgICAgICAgICAgICAgICAgaW5zZXJ0aW9uU2V0Lmluc2VydE5vZGUo aSwgU3BlY05vbmUsIAotICAgICAgICAgICAgICAgICAgICAgICAgICAgIFBoYW50b21Mb2NhbCwg bm9kZS0+b3JpZ2luLCBPcEluZm8odmFyaWFibGVBY2Nlc3NEYXRhKSk7CisgICAgICAgICAgICAg ICAgICAgICAgICAgICAgRmx1c2gsIG5vZGUtPm9yaWdpbiwgT3BJbmZvKHZhcmlhYmxlQWNjZXNz RGF0YSkpOwogICAgICAgICAgICAgICAgICAgICB9CiAgICAgICAgICAgICAgICAgfQogCkBAIC0x MjcsMTAgKzEyNywxMCBAQCBwdWJsaWM6CiAgICAgICAgICAgICB9CiAgICAgICAgIH0KIAotICAg ICAgICAvLyBJbnNlcnQgUGhhbnRvbUxvY2FscyBmb3IgZXZlcnl0aGluZyBhdCB0aGUgZW5kIG9m IHRoZSBibG9jay4KKyAgICAgICAgLy8gSW5zZXJ0IEZsdXNoIGZvciBldmVyeXRoaW5nIGF0IHRo ZSBlbmQgb2YgdGhlIGJsb2NrLgogICAgICAgICB7CiAgICAgICAgICAgICBOb2RlT3JpZ2luIG9y aWdpbiA9IGJsb2NrLT5hdChibG9jay0+c2l6ZSgpIC0gMSktPm9yaWdpbjsKLSAgICAgICAgICAg IGF1dG8gaW5zZXJ0UGhhbnRvbUxvY2FsQXRFbmQgPSBbJl0gKFZpcnR1YWxSZWdpc3RlciBvcGVy YW5kLCBib29sIGFsd2F5c0luc2VydCkgeworICAgICAgICAgICAgYXV0byBwcmVzZXJ2ZUxpdmVu ZXNzQXRFbmRPZkJsb2NrID0gWyZdIChWaXJ0dWFsUmVnaXN0ZXIgb3BlcmFuZCwgYm9vbCBhbHdh eXNJbnNlcnQpIHsKICAgICAgICAgICAgICAgICBpZiAoKG9wZXJhbmQuaXNMb2NhbCgpICYmIG1f Y3VycmVudEJsb2NrTGl2ZW5lc3MuZ2V0KG9wZXJhbmQudG9Mb2NhbCgpKSkgCiAgICAgICAgICAg ICAgICAgICAgIHx8IG9wZXJhbmQuaXNBcmd1bWVudCgpCiAgICAgICAgICAgICAgICAgICAgIHx8 IGFsd2F5c0luc2VydCkgewpAQCAtMTQxLDE0ICsxNDEsMTQgQEAgcHVibGljOgogICAgICAgICAg ICAgICAgICAgICBjdXJyZW50QmxvY2tBY2Nlc3NEYXRhLm9wZXJhbmQob3BlcmFuZCkgPSBhY2Nl c3NEYXRhOwogCiAgICAgICAgICAgICAgICAgICAgIGluc2VydGlvblNldC5pbnNlcnROb2RlKGJs b2NrLT5zaXplKCksIFNwZWNOb25lLCAKLSAgICAgICAgICAgICAgICAgICAgICAgIFBoYW50b21M b2NhbCwgb3JpZ2luLCBPcEluZm8oYWNjZXNzRGF0YSkpOworICAgICAgICAgICAgICAgICAgICAg ICAgRmx1c2gsIG9yaWdpbiwgT3BJbmZvKGFjY2Vzc0RhdGEpKTsKICAgICAgICAgICAgICAgICB9 CiAgICAgICAgICAgICB9OwogICAgICAgICAgICAgZm9yICh1bnNpZ25lZCBsb2NhbCA9IDA7IGxv Y2FsIDwgYmxvY2stPnZhcmlhYmxlc0F0VGFpbC5udW1iZXJPZkxvY2FscygpOyBsb2NhbCsrKQot ICAgICAgICAgICAgICAgIGluc2VydFBoYW50b21Mb2NhbEF0RW5kKHZpcnR1YWxSZWdpc3RlckZv ckxvY2FsKGxvY2FsKSwgZmFsc2UpOworICAgICAgICAgICAgICAgIHByZXNlcnZlTGl2ZW5lc3NB dEVuZE9mQmxvY2sodmlydHVhbFJlZ2lzdGVyRm9yTG9jYWwobG9jYWwpLCBmYWxzZSk7CiAgICAg ICAgICAgICBmb3IgKElubGluZUNhbGxGcmFtZSogaW5saW5lQ2FsbEZyYW1lIDogc2VlbklubGlu ZUNhbGxGcmFtZXMpCi0gICAgICAgICAgICAgICAgaW5zZXJ0UGhhbnRvbUxvY2FsQXRFbmQoVmly dHVhbFJlZ2lzdGVyKGlubGluZUNhbGxGcmFtZS0+c3RhY2tPZmZzZXQgKyBDYWxsRnJhbWU6OnRo aXNBcmd1bWVudE9mZnNldCgpKSwgdHJ1ZSk7Ci0gICAgICAgICAgICBpbnNlcnRQaGFudG9tTG9j YWxBdEVuZChWaXJ0dWFsUmVnaXN0ZXIoQ2FsbEZyYW1lOjp0aGlzQXJndW1lbnRPZmZzZXQoKSks IHRydWUpOworICAgICAgICAgICAgICAgIHByZXNlcnZlTGl2ZW5lc3NBdEVuZE9mQmxvY2soVmly dHVhbFJlZ2lzdGVyKGlubGluZUNhbGxGcmFtZS0+c3RhY2tPZmZzZXQgKyBDYWxsRnJhbWU6OnRo aXNBcmd1bWVudE9mZnNldCgpKSwgdHJ1ZSk7CisgICAgICAgICAgICBwcmVzZXJ2ZUxpdmVuZXNz QXRFbmRPZkJsb2NrKFZpcnR1YWxSZWdpc3RlcihDYWxsRnJhbWU6OnRoaXNBcmd1bWVudE9mZnNl dCgpKSwgdHJ1ZSk7CiAgICAgICAgIH0KICAgICB9CiAKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0 Q29yZS90ZXN0cy9zdHJlc3MvZGZnLXRyeS1jYXRjaC13cm9uZy12YWx1ZS1yZWNvdmVyeS1vbi1p Yy1taXNzLmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS90ZXN0cy9zdHJl c3MvZGZnLXRyeS1jYXRjaC13cm9uZy12YWx1ZS1yZWNvdmVyeS1vbi1pYy1taXNzLmpzCShyZXZp c2lvbiAwKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3Rlc3RzL3N0cmVzcy9kZmctdHJ5LWNh dGNoLXdyb25nLXZhbHVlLXJlY292ZXJ5LW9uLWljLW1pc3MuanMJKHdvcmtpbmcgY29weSkKQEAg LTAsMCArMSw1MCBAQAorZnVuY3Rpb24gYXNzZXJ0KGIpIHsKKyAgICBpZiAoIWIpCisgICAgICAg IHRocm93IG5ldyBFcnJvcigiYmFkIHZhbHVlIikKK30KK25vSW5saW5lKGFzc2VydCk7CisKK2xl dCBvVGhyb3cgPSB7CisgICAgeDogMjAsCisgICAgeTogNDAsCisgICAgejogNTAsCisgICAgZ2V0 IGYoKSB7IHRocm93IG5ldyBFcnJvcigiSGVsbG8gV29ybGQhIik7IH0KK307CisKK2xldCBvMSA9 IHsKKyAgICB4OiAyMCwKKyAgICBmOiA0MAorfTsKKworbGV0IG8yID0geworICAgIHg6IDIwLAor ICAgIHk6IDUwLAorICAgIGY6IDUwMCwKKyAgICBnZXQgZigpIHsgcmV0dXJuIDIwOyB9Cit9Owor CitmdW5jdGlvbiBmb28oZikgeworICAgIGxldCBvID0gZigpOworICAgIHRyeSB7CisgICAgICAg IG8gPSBvLmY7CisgICAgfSBjYXRjaChlKSB7CisgICAgICAgIGFzc2VydChvID09PSBvVGhyb3cp OyAvLyBNYWtlIHN1cmUgdGhpcyBpcyBub3QgdW5kZWZpbmVkLgorICAgIH0KK30KK25vSW5saW5l KGZvbyk7CisKK2xldCBpOworbGV0IGZsYWcgPSBmYWxzZTsKK2Z1bmN0aW9uIGYoKSB7CisgICAg aWYgKGZsYWcpCisgICAgICAgIHJldHVybiBvVGhyb3c7CisgICAgaWYgKGkgJSAyKQorICAgICAg ICByZXR1cm4gbzE7CisgICAgcmV0dXJuIG8yOworfQorbm9JbmxpbmUoZik7Citmb3IgKGkgPSAw OyBpIDwgMTAwMDA7IGkrKykgeworICAgIGZvbyhmKTsKK30KK2ZsYWcgPSB0cnVlOworZm9vKGYp Owo=