125648 2013-12-12 12:49:07 -0800 jsCStack: Fix handling of uncaught exceptions 2013-12-12 13:23:26 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mark.lam mark.lam fpizlo ggaren mhahnenberg msaboff oliver oldest_to_newest 959304 0 mark.lam 2013-12-12 12:49:07 -0800 The LLINT handleUncaughtException was not working with the correct CallFrame. It should load the callFrame from VM::callFrameForThrow (which should more appropriately be named callFrameForCatch) instead of assuming that the current frame is the frame that will "catch" i.e. handle the exception. In the case of the uncaught exception case, the "catch" frame should be the global frame which VM::callFrameForThrow provides. The baseline JIT still does not handle uncaught exceptions correctly. Will look at that separately. 959305 1 219110 mark.lam 2013-12-12 12:54:09 -0800 Created attachment 219110 the patch. 959307 2 219110 mark.lam 2013-12-12 12:59:41 -0800 Comment on attachment 219110 the patch. I stand corrected. This patch also fixes uncaught exceptions for the baseline JIT. I previously thought it didn't because I had initially implemented the fix by making llint_slow_handle_exception set the returning ExecState. I've since moved the loading of callFrameForThrow to the LLINT handleUncaughtException thunk which fixes the issue for both the LLINT and the baseline JIT. Will fix ChangeLog and upload another patch. 959310 3 219111 mark.lam 2013-12-12 13:01:58 -0800 Created attachment 219111 revised patch. 959316 4 219111 ggaren 2013-12-12 13:18:34 -0800 Comment on attachment 219111 revised patch. r=me Do we need this "bpeq CodeBlock[cfr], 1, .calleeFramePopped / loadp CallerFrame[cfr], cfr" business? I don't think so. You should look into removing it. Since we know we're handling an uncaught exception, we know for certain that callFrameForThrow is the sentinel CallFrame, and we don't need to test it. 959317 5 mark.lam 2013-12-12 13:19:52 -0800 (In reply to comment #4) > (From update of attachment 219111 [details]) > r=me > > Do we need this "bpeq CodeBlock[cfr], 1, .calleeFramePopped / loadp CallerFrame[cfr], cfr" business? I don't think so. You should look into removing it. Since we know we're handling an uncaught exception, we know for certain that callFrameForThrow is the sentinel CallFrame, and we don't need to test it. I'll remove that in my next patch when I fix all exception handling. 959319 6 mark.lam 2013-12-12 13:23:26 -0800 Thanks for the review. Landed in r160505 on the jsCStack branch: <http://trac.webkit.org/r160505>. 219110 2013-12-12 12:54:09 -0800 2013-12-12 12:59:41 -0800 the patch. bug-125648.patch text/plain 1669 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTYwNTA0KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBA CisyMDEzLTEyLTEyICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBG aXggdGhlIExMSU5UIGhhbmRsaW5nIG9mIHVuY2F1Z2h0IGV4Y2VwdGlvbnMuCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMjU2NDguCisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBsbGludC9Mb3dMZXZlbElu dGVycHJldGVyNjQuYXNtOgorICAgICAgICAtIGhhbmRsZVVuY2F1Z2h0RXhjZXB0aW9uIHNob3Vs ZCBsb2FkIHRoZSAiY2F0Y2giIGNhbGxGcmFtZSBpbnN0ZWFkIG9mIGFzc3VtaW5nCisgICAgICAg ICAgdGhhdCB0aGUgY3VycmVudCBvbmUgaXMgYXBwcm9wcmlhdGUuIFRoaXMgaXMgbm93IGZpeGVk LgorCiAyMDEzLTEyLTEyICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgogCiAg ICAgICAgIENTdGFjayBCcmFuY2g6IENoYW5nZSB0aGUgZGlzYWJsaW5nIG9mIERGRyBPU1IgZW50 cnkgdG8gYmUgYmFzZWQgb24gYW4gb3B0aW9uCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUv bGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjY0LmFzbQo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvbGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjY0LmFzbQkocmV2aXNpb24gMTYw NDk4KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2xsaW50L0xvd0xldmVsSW50ZXJwcmV0ZXI2 NC5hc20JKHdvcmtpbmcgY29weSkKQEAgLTE5OCw2ICsxOTgsNyBAQCBtYWNybyBkb0NhbGxUb0ph dmFTY3JpcHQobWFrZUNhbGwpCiAKICAgICBjaGVja1N0YWNrUG9pbnRlckFsaWdubWVudCh0ZW1w MiwgMHhiYWQwZGMwMSkKIAorICAgICMgQWxsb2NhdGUgYW5kIGluaXRpYWxpemUgdGhlIHNlbnRp bmVsIGZyYW1lLgogICAgIG1vdmUgc3AsIGNmcgogICAgIHN1YnAgKENhbGxGcmFtZUhlYWRlclNs b3RzLTEpKjgsIGNmcgogICAgIHN0b3JlcCAwLCBBcmd1bWVudENvdW50W2Nmcl0KQEAgLTI5MSw5 ICsyOTIsMTAgQEAgZW5kCiAKIAogX2hhbmRsZVVuY2F1Z2h0RXhjZXB0aW9uOgotICAgIHN1YnAg MTYsIHNwCi0KLSAgICBjaGVja1N0YWNrUG9pbnRlckFsaWdubWVudCh0MywgMHhiYWQwZWVlZSkK KyAgICBsb2FkcCBTY29wZUNoYWluW2Nmcl0sIHQzCisgICAgYW5kcCBNYXJrZWRCbG9ja01hc2ss IHQzCisgICAgbG9hZHAgTWFya2VkQmxvY2s6Om1fd2Vha1NldCArIFdlYWtTZXQ6Om1fdm1bdDNd LCB0MworICAgIGxvYWRwIFZNOjpjYWxsRnJhbWVGb3JUaHJvd1t0M10sIGNmcgogCiAgICAgYnBl cSBDb2RlQmxvY2tbY2ZyXSwgMSwgLmNhbGxlZUZyYW1lUG9wcGVkCiAgICAgbG9hZHAgQ2FsbGVy RnJhbWVbY2ZyXSwgY2ZyCg== 219111 2013-12-12 13:01:58 -0800 2013-12-12 13:18:34 -0800 revised patch. bug-125648.patch text/plain 1659 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTYwNTA0KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBA CisyMDEzLTEyLTEyICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBG aXggaGFuZGxpbmcgb2YgdW5jYXVnaHQgZXhjZXB0aW9ucy4KKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEyNTY0OC4KKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGxsaW50L0xvd0xldmVsSW50ZXJwcmV0ZXI2 NC5hc206CisgICAgICAgIC0gaGFuZGxlVW5jYXVnaHRFeGNlcHRpb24gc2hvdWxkIGxvYWQgdGhl ICJjYXRjaCIgY2FsbEZyYW1lIGluc3RlYWQgb2YgYXNzdW1pbmcKKyAgICAgICAgICB0aGF0IHRo ZSBjdXJyZW50IG9uZSBpcyBhcHByb3ByaWF0ZS4gVGhpcyBpcyBub3cgZml4ZWQuCisKIDIwMTMt MTItMTIgIE1pY2hhZWwgU2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+CiAKICAgICAgICAgQ1N0 YWNrIEJyYW5jaDogQ2hhbmdlIHRoZSBkaXNhYmxpbmcgb2YgREZHIE9TUiBlbnRyeSB0byBiZSBi YXNlZCBvbiBhbiBvcHRpb24KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9sbGludC9Mb3dM ZXZlbEludGVycHJldGVyNjQuYXNtCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29y ZS9sbGludC9Mb3dMZXZlbEludGVycHJldGVyNjQuYXNtCShyZXZpc2lvbiAxNjA0OTgpCisrKyBT b3VyY2UvSmF2YVNjcmlwdENvcmUvbGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjY0LmFzbQkod29y a2luZyBjb3B5KQpAQCAtMTk4LDYgKzE5OCw3IEBAIG1hY3JvIGRvQ2FsbFRvSmF2YVNjcmlwdCht YWtlQ2FsbCkKIAogICAgIGNoZWNrU3RhY2tQb2ludGVyQWxpZ25tZW50KHRlbXAyLCAweGJhZDBk YzAxKQogCisgICAgIyBBbGxvY2F0ZSBhbmQgaW5pdGlhbGl6ZSB0aGUgc2VudGluZWwgZnJhbWUu CiAgICAgbW92ZSBzcCwgY2ZyCiAgICAgc3VicCAoQ2FsbEZyYW1lSGVhZGVyU2xvdHMtMSkqOCwg Y2ZyCiAgICAgc3RvcmVwIDAsIEFyZ3VtZW50Q291bnRbY2ZyXQpAQCAtMjkxLDkgKzI5MiwxMCBA QCBlbmQKIAogCiBfaGFuZGxlVW5jYXVnaHRFeGNlcHRpb246Ci0gICAgc3VicCAxNiwgc3AKLQot ICAgIGNoZWNrU3RhY2tQb2ludGVyQWxpZ25tZW50KHQzLCAweGJhZDBlZWVlKQorICAgIGxvYWRw IFNjb3BlQ2hhaW5bY2ZyXSwgdDMKKyAgICBhbmRwIE1hcmtlZEJsb2NrTWFzaywgdDMKKyAgICBs b2FkcCBNYXJrZWRCbG9jazo6bV93ZWFrU2V0ICsgV2Vha1NldDo6bV92bVt0M10sIHQzCisgICAg bG9hZHAgVk06OmNhbGxGcmFtZUZvclRocm93W3QzXSwgY2ZyCiAKICAgICBicGVxIENvZGVCbG9j a1tjZnJdLCAxLCAuY2FsbGVlRnJhbWVQb3BwZWQKICAgICBsb2FkcCBDYWxsZXJGcmFtZVtjZnJd LCBjZnIK