215357 2020-08-10 22:36:17 -0700 ScriptExecutable::newCodeBlockFor() neglected to set the exception pointer result in one case. 2020-08-11 01:23:22 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=215358 InRadar P2 Normal --- 1 mark.lam mark.lam ews-watchlist keith_miller msaboff saam tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1679312 0 mark.lam 2020-08-10 22:36:17 -0700 At the bottom of ScriptExecutable::newCodeBlockFor(), it calls: RELEASE_AND_RETURN(throwScope, FunctionCodeBlock::create(vm, executable, unlinkedCodeBlock, scope)); However, ScriptExecutable::newCodeBlockFor() has 2 return values: a CodeBlock*, and a passed in Exception*& that needs to be set if there's an exception. FunctionCodeBlock::create() is capable of returning a null CodeBlock* because CodeBlock::finishCreation() can throw exceptions. As a result, we have a scenario here where ScriptExecutable::newCodeBlockFor() can return a null CodeBlock* without setting the Exception*& result. Consequently, Interpreter::executeCall() is relying on this and can end up crashing while dereferencing a null CodeBlock* because the exception result was not set. We can fix this in 1 of 2 ways: 1. Fix ScriptExecutable::newCodeBlockFor() to set the exception result. 2. Get rid of having to set the exception result, and use throwScope.exception() as the canonical method of checking for exceptions. I'm going to try to apply solution 2 if it doesn't introduce an unreasonable amount of code change. 1679313 1 mark.lam 2020-08-10 22:36:38 -0700 <rdar://problem/57675112> 1679316 2 mark.lam 2020-08-10 22:46:21 -0700 On 2nd thought, I'll just apply solution 1 which is a small and simple patch. I'll investigate applying solution 2 later with a refactoring in https://bugs.webkit.org/show_bug.cgi?id=215358. 1679321 3 406365 mark.lam 2020-08-10 23:00:44 -0700 Created attachment 406365 proposed patch. 1679322 4 406365 ysuzuki 2020-08-10 23:04:16 -0700 Comment on attachment 406365 proposed patch. r=me 1679335 5 ews-feeder 2020-08-11 01:23:21 -0700 Committed r265493: <https://trac.webkit.org/changeset/265493> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406365. 406365 2020-08-10 23:00:44 -0700 2020-08-11 01:23:22 -0700 proposed patch. bug-215357.patch text/plain 3855 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjY1NDg4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDMwIEBA CisyMDIwLTA4LTEwICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBT Y3JpcHRFeGVjdXRhYmxlOjpuZXdDb2RlQmxvY2tGb3IoKSBuZWdsZWN0ZWQgdG8gc2V0IHRoZSBl eGNlcHRpb24gcG9pbnRlciByZXN1bHQgaW4gb25lIGNhc2UuCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTUzNTcKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzU3Njc1MTEyPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIEF0IHRoZSBib3R0b20gb2YgU2NyaXB0RXhlY3V0YWJsZTo6bmV3Q29kZUJsb2NrRm9y KCksIGl0IGNhbGxzOgorICAgICAgICAgICAgUkVMRUFTRV9BTkRfUkVUVVJOKHRocm93U2NvcGUs IEZ1bmN0aW9uQ29kZUJsb2NrOjpjcmVhdGUodm0sIGV4ZWN1dGFibGUsIHVubGlua2VkQ29kZUJs b2NrLCBzY29wZSkpOworCisgICAgICAgIEhvd2V2ZXIsIFNjcmlwdEV4ZWN1dGFibGU6Om5ld0Nv ZGVCbG9ja0ZvcigpIGhhcyAyIHJldHVybiB2YWx1ZXM6IGEgQ29kZUJsb2NrKiwKKyAgICAgICAg YW5kIGEgcGFzc2VkIGluIEV4Y2VwdGlvbiomIHRoYXQgbmVlZHMgdG8gYmUgc2V0IGlmIHRoZXJl J3MgYW4gZXhjZXB0aW9uLgorICAgICAgICBGdW5jdGlvbkNvZGVCbG9jazo6Y3JlYXRlKCkgaXMg Y2FwYWJsZSBvZiByZXR1cm5pbmcgYSBudWxsIENvZGVCbG9jayogYmVjYXVzZQorICAgICAgICBD b2RlQmxvY2s6OmZpbmlzaENyZWF0aW9uKCkgY2FuIHRocm93IGV4Y2VwdGlvbnMuICBBcyBhIHJl c3VsdCwgd2UgaGF2ZSBhIHNjZW5hcmlvCisgICAgICAgIGhlcmUgd2hlcmUgU2NyaXB0RXhlY3V0 YWJsZTo6bmV3Q29kZUJsb2NrRm9yKCkgY2FuIHJldHVybiBhIG51bGwgQ29kZUJsb2NrKiB3aXRo b3V0CisgICAgICAgIHNldHRpbmcgdGhlIEV4Y2VwdGlvbiomIHJlc3VsdC4KKworICAgICAgICBD b25zZXF1ZW50bHksIEludGVycHJldGVyOjpleGVjdXRlQ2FsbCgpIGlzIHJlbHlpbmcgb24gdGhp cyBhbmQgY2FuIGVuZCB1cAorICAgICAgICBjcmFzaGluZyB3aGlsZSBkZXJlZmVyZW5jaW5nIGEg bnVsbCBDb2RlQmxvY2sqIGJlY2F1c2UgdGhlIGV4Y2VwdGlvbiByZXN1bHQgd2FzCisgICAgICAg IG5vdCBzZXQuCisKKyAgICAgICAgVGhpcyBwYXRjaCBmaXhlcyBTY3JpcHRFeGVjdXRhYmxlOjpu ZXdDb2RlQmxvY2tGb3IoKSB0byBzZXQgdGhlIGV4Y2VwdGlvbiByZXN1bHQuCisKKyAgICAgICAg KiBydW50aW1lL1NjcmlwdEV4ZWN1dGFibGUuY3BwOgorICAgICAgICAoSlNDOjpTY3JpcHRFeGVj dXRhYmxlOjpuZXdDb2RlQmxvY2tGb3IpOgorCiAyMDIwLTA4LTEwICBMYXVybyBNb3VyYSAgPGxt b3VyYUBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtDTWFrZV1bSlNDXSBGaXggdGVzdGFwaVNjcmlw dHMgY29weSBsb2NhdGlvbgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvU2Ny aXB0RXhlY3V0YWJsZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3J1 bnRpbWUvU2NyaXB0RXhlY3V0YWJsZS5jcHAJKHJldmlzaW9uIDI2NTQ4NykKKysrIFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9ydW50aW1lL1NjcmlwdEV4ZWN1dGFibGUuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0yNjEsNyArMjYxLDcgQEAgQ29kZUJsb2NrKiBTY3JpcHRFeGVjdXRhYmxlOjpuZXdDb2Rl QmxvYwogICAgICAgICBSRUxFQVNFX0FTU0VSVChraW5kID09IENvZGVGb3JDYWxsKTsKICAgICAg ICAgUkVMRUFTRV9BU1NFUlQoIWV4ZWN1dGFibGUtPm1fZXZhbENvZGVCbG9jayk7CiAgICAgICAg IFJFTEVBU0VfQVNTRVJUKCFmdW5jdGlvbik7Ci0gICAgICAgIGF1dG8gY29kZUJsb2NrID0gRXZh bENvZGVCbG9jazo6Y3JlYXRlKHZtLAorICAgICAgICBhdXRvKiBjb2RlQmxvY2sgPSBFdmFsQ29k ZUJsb2NrOjpjcmVhdGUodm0sCiAgICAgICAgICAgICBleGVjdXRhYmxlLCBleGVjdXRhYmxlLT5t X3VubGlua2VkRXZhbENvZGVCbG9jay5nZXQoKSwgc2NvcGUpOwogICAgICAgICBFWENFUFRJT05f QVNTRVJUKHRocm93U2NvcGUuZXhjZXB0aW9uKCkgfHwgY29kZUJsb2NrKTsKICAgICAgICAgaWYg KCFjb2RlQmxvY2spIHsKQEAgLTI3OCw3ICsyNzgsNyBAQCBDb2RlQmxvY2sqIFNjcmlwdEV4ZWN1 dGFibGU6Om5ld0NvZGVCbG9jCiAgICAgICAgIFJFTEVBU0VfQVNTRVJUKGtpbmQgPT0gQ29kZUZv ckNhbGwpOwogICAgICAgICBSRUxFQVNFX0FTU0VSVCghZXhlY3V0YWJsZS0+bV9wcm9ncmFtQ29k ZUJsb2NrKTsKICAgICAgICAgUkVMRUFTRV9BU1NFUlQoIWZ1bmN0aW9uKTsKLSAgICAgICAgYXV0 byBjb2RlQmxvY2sgPSBQcm9ncmFtQ29kZUJsb2NrOjpjcmVhdGUodm0sCisgICAgICAgIGF1dG8q IGNvZGVCbG9jayA9IFByb2dyYW1Db2RlQmxvY2s6OmNyZWF0ZSh2bSwKICAgICAgICAgICAgIGV4 ZWN1dGFibGUsIGV4ZWN1dGFibGUtPm1fdW5saW5rZWRQcm9ncmFtQ29kZUJsb2NrLmdldCgpLCBz Y29wZSk7CiAgICAgICAgIEVYQ0VQVElPTl9BU1NFUlQodGhyb3dTY29wZS5leGNlcHRpb24oKSB8 fCBjb2RlQmxvY2spOwogICAgICAgICBpZiAoIWNvZGVCbG9jaykgewpAQCAtMjk1LDcgKzI5NSw3 IEBAIENvZGVCbG9jayogU2NyaXB0RXhlY3V0YWJsZTo6bmV3Q29kZUJsb2MKICAgICAgICAgUkVM RUFTRV9BU1NFUlQoa2luZCA9PSBDb2RlRm9yQ2FsbCk7CiAgICAgICAgIFJFTEVBU0VfQVNTRVJU KCFleGVjdXRhYmxlLT5tX21vZHVsZVByb2dyYW1Db2RlQmxvY2spOwogICAgICAgICBSRUxFQVNF X0FTU0VSVCghZnVuY3Rpb24pOwotICAgICAgICBhdXRvIGNvZGVCbG9jayA9IE1vZHVsZVByb2dy YW1Db2RlQmxvY2s6OmNyZWF0ZSh2bSwKKyAgICAgICAgYXV0byogY29kZUJsb2NrID0gTW9kdWxl UHJvZ3JhbUNvZGVCbG9jazo6Y3JlYXRlKHZtLAogICAgICAgICAgICAgZXhlY3V0YWJsZSwgZXhl Y3V0YWJsZS0+bV91bmxpbmtlZE1vZHVsZVByb2dyYW1Db2RlQmxvY2suZ2V0KCksIHNjb3BlKTsK ICAgICAgICAgRVhDRVBUSU9OX0FTU0VSVCh0aHJvd1Njb3BlLmV4Y2VwdGlvbigpIHx8IGNvZGVC bG9jayk7CiAgICAgICAgIGlmICghY29kZUJsb2NrKSB7CkBAIC0zMzcsNyArMzM3LDEwIEBAIENv ZGVCbG9jayogU2NyaXB0RXhlY3V0YWJsZTo6bmV3Q29kZUJsb2MKICAgICAgICAgcmV0dXJuIG51 bGxwdHI7CiAgICAgfQogCi0gICAgUkVMRUFTRV9BTkRfUkVUVVJOKHRocm93U2NvcGUsIEZ1bmN0 aW9uQ29kZUJsb2NrOjpjcmVhdGUodm0sIGV4ZWN1dGFibGUsIHVubGlua2VkQ29kZUJsb2NrLCBz Y29wZSkpOworICAgIGF1dG8qIGNvZGVCbG9jayA9IEZ1bmN0aW9uQ29kZUJsb2NrOjpjcmVhdGUo dm0sIGV4ZWN1dGFibGUsIHVubGlua2VkQ29kZUJsb2NrLCBzY29wZSk7CisgICAgaWYgKHRocm93 U2NvcGUuZXhjZXB0aW9uKCkpCisgICAgICAgIGV4Y2VwdGlvbiA9IHRocm93U2NvcGUuZXhjZXB0 aW9uKCk7CisgICAgcmV0dXJuIGNvZGVCbG9jazsKIH0KIAogQ29kZUJsb2NrKiBTY3JpcHRFeGVj dXRhYmxlOjpuZXdSZXBsYWNlbWVudENvZGVCbG9ja0ZvcigK