Bug 215357 - ScriptExecutable::newCodeBlockFor() neglected to set the exception pointer result in one case.
Summary: ScriptExecutable::newCodeBlockFor() neglected to set the exception pointer re...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-10 22:36 PDT by Mark Lam
Modified: 2020-08-11 01:23 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch. (3.76 KB, patch)
2020-08-10 23:00 PDT, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2020-08-10 22:36:17 PDT
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.
Comment 1 Mark Lam 2020-08-10 22:36:38 PDT
<rdar://problem/57675112>
Comment 2 Mark Lam 2020-08-10 22:46:21 PDT
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.
Comment 3 Mark Lam 2020-08-10 23:00:44 PDT
Created attachment 406365 [details]
proposed patch.
Comment 4 Yusuke Suzuki 2020-08-10 23:04:16 PDT
Comment on attachment 406365 [details]
proposed patch.

r=me
Comment 5 EWS 2020-08-11 01:23:21 PDT
Committed r265493: <https://trac.webkit.org/changeset/265493>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406365 [details].