205230 2019-12-13 17:30:16 -0800 Fix bad exception assertion in ExceptionHelpers.cpp's createError(). 2019-12-13 17:51:44 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam ews-watchlist keith_miller msaboff saam tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1598567 0 mark.lam 2019-12-13 17:30:16 -0800 <rdar://problem/57875688> 1598568 1 385664 mark.lam 2019-12-13 17:33:59 -0800 Created attachment 385664 proposed patch. 1598572 2 385664 ysuzuki 2019-12-13 17:39:17 -0800 Comment on attachment 385664 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=385664&action=review r=me with nit. > Source/JavaScriptCore/runtime/ExceptionHelpers.cpp:269 > - EXCEPTION_ASSERT(scope.exception() || !!valueDescription); > - if (!valueDescription) { > + if (scope.exception() || !valueDescription) { Can you add a comment why we are not returning when we have `scope.exception()`? 1598577 3 mark.lam 2019-12-13 17:41:39 -0800 Thanks for the review. (In reply to Yusuke Suzuki from comment #2) > > Source/JavaScriptCore/runtime/ExceptionHelpers.cpp:269 > > - EXCEPTION_ASSERT(scope.exception() || !!valueDescription); > > - if (!valueDescription) { > > + if (scope.exception() || !valueDescription) { > > Can you add a comment why we are not returning when we have > `scope.exception()`? Sure. 1598578 4 mark.lam 2019-12-13 17:51:44 -0800 Landed in r253515: <http://trac.webkit.org/r253515>. 385664 2019-12-13 17:33:59 -0800 2019-12-13 17:39:17 -0800 proposed patch. bug-205230.patch text/plain 3399 mark.lam SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNTM1MTApCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDEzIEBACisyMDE5LTEyLTEzICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNv bT4KKworICAgICAgICBGaXggYmFkIGV4Y2VwdGlvbiBhc3NlcnRpb24gaW4gRXhjZXB0aW9uSGVs cGVycy5jcHAncyBjcmVhdGVFcnJvcigpLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MjA1MjMwCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS81Nzg3NTY4 OD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHN0 cmVzcy90ZXN0LWV4Y2VwdGlvbi1hc3NlcnQtaW4tRXhjZXB0aW9uSGVscGVycy1jcmVhdGVFcnJv ci5qczogQWRkZWQuCisKIDIwMTktMTItMTIgIFl1c3VrZSBTdXp1a2kgIDx5c3V6dWtpQGFwcGxl LmNvbT4KIAogICAgICAgICBbSlNDXSBXYXNtIGluaXQtZXhwciBzaG91bGQgcmVqZWN0IG11dGFi bGUgZ2xvYmFscwpJbmRleDogSlNUZXN0cy9zdHJlc3MvdGVzdC1leGNlcHRpb24tYXNzZXJ0LWlu LUV4Y2VwdGlvbkhlbHBlcnMtY3JlYXRlRXJyb3IuanMKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSlNUZXN0cy9z dHJlc3MvdGVzdC1leGNlcHRpb24tYXNzZXJ0LWluLUV4Y2VwdGlvbkhlbHBlcnMtY3JlYXRlRXJy b3IuanMJKG5vbmV4aXN0ZW50KQorKysgSlNUZXN0cy9zdHJlc3MvdGVzdC1leGNlcHRpb24tYXNz ZXJ0LWluLUV4Y2VwdGlvbkhlbHBlcnMtY3JlYXRlRXJyb3IuanMJKHdvcmtpbmcgY29weSkKQEAg LTAsMCArMSwxMSBAQAorLy9AIHNraXAgaWYgJG1lbW9yeUxpbWl0ZWQKKy8vQCBydW5EZWZhdWx0 CisKK3RyeSB7CisgICAgJycucGFkU3RhcnQoMioqMzEtMSkoKTsKK30gY2F0Y2goZSkgeworICAg IGV4Y2VwdGlvbiA9IGU7Cit9CisKK2lmIChleGNlcHRpb24gIT0gIkVycm9yOiBPdXQgb2YgbWVt b3J5IikKKyAgICB0aHJvdyAiRkFJTEVEIjsKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9D aGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwko cmV2aXNpb24gMjUzNDkyKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29y a2luZyBjb3B5KQpAQCAtMSwzICsxLDMyIEBACisyMDE5LTEyLTEzICBNYXJrIExhbSAgPG1hcmsu bGFtQGFwcGxlLmNvbT4KKworICAgICAgICBGaXggYmFkIGV4Y2VwdGlvbiBhc3NlcnRpb24gaW4g RXhjZXB0aW9uSGVscGVycy5jcHAncyBjcmVhdGVFcnJvcigpLgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjA1MjMwCisgICAgICAgIDxyZGFyOi8vcHJv YmxlbS81Nzg3NTY4OD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBUaGUgY29kZSBpbiBjcmVhdGVFcnJvcigpIHdhcyBkb2luZyB0aGUgZm9sbG93aW5n OgorCisgICAgICAgICAgICBTdHJpbmcgdmFsdWVEZXNjcmlwdGlvbiA9IGVycm9yRGVzY3JpcHRp b25Gb3JWYWx1ZShnbG9iYWxPYmplY3QsIHZhbHVlKTsKKyAgICAgICAgICAgIEVYQ0VQVElPTl9B U1NFUlQoc2NvcGUuZXhjZXB0aW9uKCkgfHwgISF2YWx1ZURlc2NyaXB0aW9uKTsKKyAgICAgICAg ICAgIGlmICghdmFsdWVEZXNjcmlwdGlvbikgeworICAgICAgICAgICAgICAgIHNjb3BlLmNsZWFy RXhjZXB0aW9uKCk7CisgICAgICAgICAgICAgICAgcmV0dXJuIGNyZWF0ZU91dE9mTWVtb3J5RXJy b3IoZ2xvYmFsT2JqZWN0KTsKKyAgICAgICAgICAgIH0KKworICAgICAgICBJZiBlcnJvckRlc2Ny aXB0aW9uRm9yVmFsdWUoKSB0aHJvd3MgYW4gZXhjZXB0aW9uLCB0aGVuIHdlIGV4cGVjdCB0aGUK KyAgICAgICAgdmFsdWVEZXNjcmlwdGlvbiBzdHJpbmcgdG8gYmUgbnVsbCBzbyB0aGF0IHdlIGNh biB0aHJvdyBhbiBPdXRPZk1lbW9yeUVycm9yLgorICAgICAgICBIb3dldmVyLCBlcnJvckRlc2Ny aXB0aW9uRm9yVmFsdWUoKSBjYW4gZGV0ZWN0IGFuIGltbWluZW50IG92ZXJmbG93IGluIFN0cmlu ZworICAgICAgICBsZW5ndGggYW5kIGp1c3QgcmV0dXJuIGEgbnVsbCBzdHJpbmcgd2l0aG91dCB0 aHJvd2luZyBhbiBleGNlcHRpb24gd2hpY2ggZmFpbHMKKyAgICAgICAgdGhlIGFib3ZlIGFzc2Vy dGlvbi4KKworICAgICAgICBUaGUgZml4IGlzIHRvIHNpbXBseSBkbyBhbiBleHBsaWNpdCBleGNl cHRpb24gY2hlY2sgaW4gYWRkaXRpb24gdG8gdGhlIG51bGwgc3RyaW5nCisgICAgICAgIGNoZWNr IGFuZCByZW1vdmUgdGhlIGFzc2VydGlvbi4KKworICAgICAgICAqIHJ1bnRpbWUvRXhjZXB0aW9u SGVscGVycy5jcHA6CisgICAgICAgIChKU0M6OmNyZWF0ZUVycm9yKToKKwogMjAxOS0xMi0xMyAg SmltIE1hc29uICA8am1hc29uQGliaW54LmNvbT4KIAogICAgICAgICBbR1RLXSBXZWJLaXRHVEsg YnVpbGQgaGFuZ3Mgb24gZy1pci1zY2FubmVyCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUv cnVudGltZS9FeGNlcHRpb25IZWxwZXJzLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNj cmlwdENvcmUvcnVudGltZS9FeGNlcHRpb25IZWxwZXJzLmNwcAkocmV2aXNpb24gMjUzNDkyKQor KysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvRXhjZXB0aW9uSGVscGVycy5jcHAJKHdv cmtpbmcgY29weSkKQEAgLTI2Niw4ICsyNjYsNyBAQCBKU09iamVjdCogY3JlYXRlRXJyb3IoSlNH bG9iYWxPYmplY3QqIGdsCiAgICAgYXV0byBzY29wZSA9IERFQ0xBUkVfQ0FUQ0hfU0NPUEUodm0p OwogCiAgICAgU3RyaW5nIHZhbHVlRGVzY3JpcHRpb24gPSBlcnJvckRlc2NyaXB0aW9uRm9yVmFs dWUoZ2xvYmFsT2JqZWN0LCB2YWx1ZSk7Ci0gICAgRVhDRVBUSU9OX0FTU0VSVChzY29wZS5leGNl cHRpb24oKSB8fCAhIXZhbHVlRGVzY3JpcHRpb24pOwotICAgIGlmICghdmFsdWVEZXNjcmlwdGlv bikgeworICAgIGlmIChzY29wZS5leGNlcHRpb24oKSB8fCAhdmFsdWVEZXNjcmlwdGlvbikgewog ICAgICAgICBzY29wZS5jbGVhckV4Y2VwdGlvbigpOwogICAgICAgICByZXR1cm4gY3JlYXRlT3V0 T2ZNZW1vcnlFcnJvcihnbG9iYWxPYmplY3QpOwogICAgIH0K