Bug 225410

Summary: Forbid further execution in jsc shell if execution is terminated.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
propose patch. msaboff: review+, ews-feeder: commit-queue-

Description Mark Lam 2021-05-05 13:16:09 PDT
Also re-implement WorkerOrWorkletScriptController::forbidExecution() and isExecutionForbidden() using the VM's notion of the flag.

rdar://77548608
Comment 1 Mark Lam 2021-05-05 13:23:51 PDT
Created attachment 427797 [details]
propose patch.
Comment 2 Michael Saboff 2021-05-05 14:06:16 PDT
Comment on attachment 427797 [details]
propose patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=427797&action=review

r=me with a suggested change.

> Source/JavaScriptCore/runtime/VM.h:345
> +    void setExecutionForbidden(bool value) { m_executionForbidden = value; }

Seem like we only want to go from "execution allowed", e.g. m_executionForbidden == false, to "execution forbidden", e.g. m_executionForbidden == true.  I suggest we eliminate the argument to setExecutionForbidden() and have it always set m_executionForbidden to true.
Comment 3 Mark Lam 2021-05-05 14:07:38 PDT
(In reply to Michael Saboff from comment #2)
> Comment on attachment 427797 [details]
> propose patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=427797&action=review
> 
> r=me with a suggested change.
> 
> > Source/JavaScriptCore/runtime/VM.h:345
> > +    void setExecutionForbidden(bool value) { m_executionForbidden = value; }
> 
> Seem like we only want to go from "execution allowed", e.g.
> m_executionForbidden == false, to "execution forbidden", e.g.
> m_executionForbidden == true.  I suggest we eliminate the argument to
> setExecutionForbidden() and have it always set m_executionForbidden to true.

I agree.  That's probably the better approach until we find evidence to the contrary.
Comment 4 Mark Lam 2021-05-06 09:24:17 PDT
Thanks for the review.  I've made the suggested change.

Landed in r277094: <http://trac.webkit.org/r277094>.