Bug 220216

Summary: Don't throw if `function.caller` is a non-strict / generator / async function
Product: WebKit Reporter: Alexey Shvayka <ashvayka>
Component: JavaScriptCoreAssignee: Alexey Shvayka <ashvayka>
Status: RESOLVED FIXED    
Severity: Minor CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=184630
https://bugs.webkit.org/show_bug.cgi?id=220610
https://bugs.webkit.org/show_bug.cgi?id=225277
Attachments:
Description Flags
Patch
none
Patch none

Description Alexey Shvayka 2020-12-31 19:22:09 PST
Don't throw if `function.caller` is a non-strict / generator / async function
Comment 1 Alexey Shvayka 2020-12-31 19:25:04 PST
Created attachment 416876 [details]
Patch
Comment 2 Alexey Shvayka 2021-01-01 05:48:56 PST
Created attachment 416881 [details]
Patch

Adjust tests.
Comment 3 Yusuke Suzuki 2021-01-02 10:39:09 PST
Comment on attachment 416881 [details]
Patch

r=me
Comment 4 EWS 2021-01-02 10:42:13 PST
Committed r271119: <https://trac.webkit.org/changeset/271119>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416881 [details].
Comment 5 Radar WebKit Bug Importer 2021-01-02 10:43:24 PST
<rdar://problem/72770747>
Comment 6 Yusuke Suzuki 2021-01-28 12:30:20 PST
Oops, I think the removed tests are super important ones. I'll attempt to recover them.
Comment 7 Yusuke Suzuki 2021-01-28 12:42:08 PST
Committed r272031: <https://trac.webkit.org/changeset/272031>
Comment 8 Alexey Shvayka 2021-01-28 13:08:27 PST
(In reply to Yusuke Suzuki from comment #6)
> Oops, I think the removed tests are super important ones. I'll attempt to
> recover them.

(In reply to Yusuke Suzuki from comment #7)
> Committed r272031: <https://trac.webkit.org/changeset/272031>

The tests were not removed, but rather merged into JSTests/stress/function-hidden-as-caller.js and expanded. What did I miss?

Aren't internal-only functions rejected by

    if (function->isHostOrBuiltinFunction())
        return JSValue::encode(jsNull());

?
Comment 9 Yusuke Suzuki 2021-01-29 13:20:32 PST
(In reply to Alexey Shvayka from comment #8)
> (In reply to Yusuke Suzuki from comment #6)
> > Oops, I think the removed tests are super important ones. I'll attempt to
> > recover them.
> 
> (In reply to Yusuke Suzuki from comment #7)
> > Committed r272031: <https://trac.webkit.org/changeset/272031>
> 
> The tests were not removed, but rather merged into
> JSTests/stress/function-hidden-as-caller.js and expanded. What did I miss?
> 
> Aren't internal-only functions rejected by
> 
>     if (function->isHostOrBuiltinFunction())
>         return JSValue::encode(jsNull());
> 
> ?

Is the generator body function etc. builtin function? It must not be exposed since these functions are assuming particular arguments will be passed.
Comment 10 Alexey Shvayka 2021-05-14 11:41:45 PDT
(In reply to Alexey Shvayka from comment #8)
> (In reply to Yusuke Suzuki from comment #6)
> > Oops, I think the removed tests are super important ones. I'll attempt to
> > recover them.
> 
> The tests were not removed, but rather merged into
> JSTests/stress/function-hidden-as-caller.js and expanded. What did I miss?

I've missed calling next() on generators: function-hidden-as-caller.js wasn't testing them at all.
That is fixed in https://bugs.webkit.org/show_bug.cgi?id=225277.

(In reply to Yusuke Suzuki from comment #9)
> Is the generator body function etc. builtin function? It must not be exposed
> since these functions are assuming particular arguments will be passed.

Yeah, I've figured there are two functions per generator / async: public wrapper and private body. The latter is not caught by isHostOrBuiltinFunction(), unlike @generatorResume(), and needs to be handled separately. We can't easily get public wrapper from private body (even via scope lookup) to align with V8 / SM, and we shouldn't: there is stage 1 proposal to standardize our behaviour.