Bug 215357 – ScriptExecutable::newCodeBlockFor() neglected to set the exception pointer result in one case.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

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.

[Mark Lam]

<rdar://problem/57675112>

[Mark Lam]

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.

[Mark Lam]

Created attachment 406365 [details]
proposed patch.

[Yusuke Suzuki]

Comment on attachment 406365 [details]
proposed patch.

r=me

[EWS]

Committed r265493: <https://trac.webkit.org/changeset/265493>

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