164969 2016-11-18 16:47:15 -0800 Fix exception scope verification failures in LLIntSlowPaths.cpp 2016-11-28 12:45:12 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 162351 1 mark.lam mark.lam commit-queue fpizlo ggaren jfbastien keith_miller msaboff saam ysuzuki oldest_to_newest 1252952 0 mark.lam 2016-11-18 16:47:15 -0800 Patch coming. 1252954 1 295223 mark.lam 2016-11-18 16:50:38 -0800 Created attachment 295223 proposed patch. 1253912 2 295223 ggaren 2016-11-28 12:12:06 -0800 Comment on attachment 295223 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=295223&action=review > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1452 > + throwScope.release(); > return setUpCall(execCallee, pc, kind, calleeAsValue); I think the throwScope.release() function should assert that there is no pending exception. Otherwise, this release point is an opportunity to make an error when calling into setUpCall. 1253930 3 mark.lam 2016-11-28 12:32:26 -0800 (In reply to comment #2) > Comment on attachment 295223 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=295223&action=review > > > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:1452 > > + throwScope.release(); > > return setUpCall(execCallee, pc, kind, calleeAsValue); > > I think the throwScope.release() function should assert that there is no > pending exception. Otherwise, this release point is an opportunity to make > an error when calling into setUpCall. I agree. but I'll do this in a separate patch because I think this will result in some failures due to some old scope.release() being added this way (for various reasons): result = callSomethingThatThrows(); scope.release(); return result; These need to be fixed like so: scope.release(); return callSomethingThatThrows(); I'll do that fix up along with adding the assertion to scope.release() in one patch. 1253935 4 mark.lam 2016-11-28 12:37:40 -0800 (In reply to comment #3) > I'll do that fix up along with adding the assertion to scope.release() in > one patch. Bug for making scope.release() assert no pending exceptions is https://bugs.webkit.org/show_bug.cgi?id=165105. 1253940 5 mark.lam 2016-11-28 12:45:12 -0800 Thanks for the review. Landed in r209007: <http://trac.webkit.org/r209007>. 295223 2016-11-18 16:50:38 -0800 2016-11-28 12:12:06 -0800 proposed patch. bug-164969.patch text/plain 3144 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjA4OTA4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDE2LTExLTE4ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBG aXggZXhjZXB0aW9uIHNjb3BlIHZlcmlmaWNhdGlvbiBmYWlsdXJlcyBpbiBMTEludFNsb3dQYXRo cy5jcHAuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x NjQ5NjkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IGxsaW50L0xMSW50U2xvd1BhdGhzLmNwcDoKKyAgICAgICAgKEpTQzo6TExJbnQ6OmdldEJ5VmFs KToKKyAgICAgICAgKEpTQzo6TExJbnQ6OnNldFVwQ2FsbCk6CisgICAgICAgIChKU0M6OkxMSW50 Ojp2YXJhcmdzU2V0dXApOgorICAgICAgICAoSlNDOjpMTEludDo6TExJTlRfU0xPV19QQVRIX0RF Q0wpOgorCiAyMDE2LTExLTE4ICBKZXJlbXkgSm9uZXMgIDxqZXJlbXlqQGFwcGxlLmNvbT4KIAog ICAgICAgICBBZGQgcnVudGltZSBmbGFnIHRvIGVuYWJsZSBwb2ludGVyIGxvY2suIEVuYWJsZSBw b2ludGVyIGxvY2sgZmVhdHVyZSBmb3IgbWFjLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3Jl L2xsaW50L0xMSW50U2xvd1BhdGhzLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlw dENvcmUvbGxpbnQvTExJbnRTbG93UGF0aHMuY3BwCShyZXZpc2lvbiAyMDg5MDUpCisrKyBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvbGxpbnQvTExJbnRTbG93UGF0aHMuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC04MTQsNyArODE0LDcgQEAgc3RhdGljIEFMV0FZU19JTkxJTkUgSlNWYWx1ZSBnZXRCeVZh bChWTQogICAgICAgICB1aW50MzJfdCBpID0gc3Vic2NyaXB0LmFzVUludDMyKCk7CiAgICAgICAg IGlmIChpc0pTU3RyaW5nKGJhc2VWYWx1ZSkgJiYgYXNTdHJpbmcoYmFzZVZhbHVlKS0+Y2FuR2V0 SW5kZXgoaSkpCiAgICAgICAgICAgICByZXR1cm4gYXNTdHJpbmcoYmFzZVZhbHVlKS0+Z2V0SW5k ZXgoZXhlYywgaSk7Ci0gICAgICAgIAorICAgICAgICBzY29wZS5yZWxlYXNlKCk7CiAgICAgICAg IHJldHVybiBiYXNlVmFsdWUuZ2V0KGV4ZWMsIGkpOwogICAgIH0KIApAQCAtODIyLDYgKzgyMiw3 IEBAIHN0YXRpYyBBTFdBWVNfSU5MSU5FIEpTVmFsdWUgZ2V0QnlWYWwoVk0KICAgICBSRVRVUk5f SUZfRVhDRVBUSU9OKHNjb3BlLCBKU1ZhbHVlKCkpOwogICAgIGF1dG8gcHJvcGVydHkgPSBzdWJz Y3JpcHQudG9Qcm9wZXJ0eUtleShleGVjKTsKICAgICBSRVRVUk5fSUZfRVhDRVBUSU9OKHNjb3Bl LCBKU1ZhbHVlKCkpOworICAgIHNjb3BlLnJlbGVhc2UoKTsKICAgICByZXR1cm4gYmFzZVZhbHVl LmdldChleGVjLCBwcm9wZXJ0eSk7CiB9CiAKQEAgLTEzMDYsNyArMTMwNyw4IEBAIGlubGluZSBT bG93UGF0aFJldHVyblR5cGUgc2V0VXBDYWxsKEV4ZWMKIAogICAgICAgICBDb2RlQmxvY2sqKiBj b2RlQmxvY2tTbG90ID0gZXhlY0NhbGxlZS0+YWRkcmVzc09mQ29kZUJsb2NrKCk7CiAgICAgICAg IEpTT2JqZWN0KiBlcnJvciA9IGZ1bmN0aW9uRXhlY3V0YWJsZS0+cHJlcGFyZUZvckV4ZWN1dGlv bjxGdW5jdGlvbkV4ZWN1dGFibGU+KHZtLCBjYWxsZWUsIHNjb3BlLCBraW5kLCAqY29kZUJsb2Nr U2xvdCk7Ci0gICAgICAgIGlmIChlcnJvcikKKyAgICAgICAgQVNTRVJUKHRocm93U2NvcGUuZXhj ZXB0aW9uKCkgPT0gZXJyb3IpOworICAgICAgICBpZiAoVU5MSUtFTFkoZXJyb3IpKQogICAgICAg ICAgICAgTExJTlRfQ0FMTF9USFJPVyhleGVjLCBlcnJvcik7CiAgICAgICAgIGNvZGVCbG9jayA9 ICpjb2RlQmxvY2tTbG90OwogICAgICAgICBBU1NFUlQoY29kZUJsb2NrKTsKQEAgLTE0NDYsNiAr MTQ0OCw3IEBAIGlubGluZSBTbG93UGF0aFJldHVyblR5cGUgdmFyYXJnc1NldHVwKEUKICAgICBl eGVjQ2FsbGVlLT51bmNoZWNrZWRSKENhbGxGcmFtZVNsb3Q6OmNhbGxlZSkgPSBjYWxsZWVBc1Zh bHVlOwogICAgIGV4ZWMtPnNldEN1cnJlbnRWUEMocGMpOwogCisgICAgdGhyb3dTY29wZS5yZWxl YXNlKCk7CiAgICAgcmV0dXJuIHNldFVwQ2FsbChleGVjQ2FsbGVlLCBwYywga2luZCwgY2FsbGVl QXNWYWx1ZSk7CiB9CiAKQEAgLTE0NzksOCArMTQ4MiwxMCBAQCBMTElOVF9TTE9XX1BBVEhfREVD TChzbG93X3BhdGhfY2FsbF9ldmFsCiAgICAgZXhlY0NhbGxlZS0+c2V0Q29kZUJsb2NrKDApOwog ICAgIGV4ZWMtPnNldEN1cnJlbnRWUEMocGMpOwogICAgIAotICAgIGlmICghaXNIb3N0RnVuY3Rp b24oY2FsbGVlQXNWYWx1ZSwgZ2xvYmFsRnVuY0V2YWwpKQorICAgIGlmICghaXNIb3N0RnVuY3Rp b24oY2FsbGVlQXNWYWx1ZSwgZ2xvYmFsRnVuY0V2YWwpKSB7CisgICAgICAgIHRocm93U2NvcGUu cmVsZWFzZSgpOwogICAgICAgICByZXR1cm4gc2V0VXBDYWxsKGV4ZWNDYWxsZWUsIHBjLCBDb2Rl Rm9yQ2FsbCwgY2FsbGVlQXNWYWx1ZSk7CisgICAgfQogICAgIAogICAgIHZtLmhvc3RDYWxsUmV0 dXJuVmFsdWUgPSBldmFsKGV4ZWNDYWxsZWUpOwogICAgIExMSU5UX0NBTExfUkVUVVJOKGV4ZWMs IGV4ZWNDYWxsZWUsIExMSW50OjpnZXRDb2RlUHRyKGdldEhvc3RDYWxsUmV0dXJuVmFsdWUpKTsK QEAgLTE1ODUsNiArMTU5MCw3IEBAIExMSU5UX1NMT1dfUEFUSF9ERUNMKHNsb3dfcGF0aF9wdXRf dG9fc2MKICAgICB9CiAKICAgICBib29sIGhhc1Byb3BlcnR5ID0gc2NvcGUtPmhhc1Byb3BlcnR5 KGV4ZWMsIGlkZW50KTsKKyAgICBMTElOVF9DSEVDS19FWENFUFRJT04oKTsKICAgICBpZiAoaGFz UHJvcGVydHkKICAgICAgICAgJiYgc2NvcGUtPmlzR2xvYmFsTGV4aWNhbEVudmlyb25tZW50KCkK ICAgICAgICAgJiYgIWlzSW5pdGlhbGl6YXRpb24oZ2V0UHV0SW5mby5pbml0aWFsaXphdGlvbk1v ZGUoKSkpIHsK