Bug 220216 - Don't throw if `function.caller` is a non-strict / generator / async function
Summary: Don't throw if `function.caller` is a non-strict / generator / async function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Minor
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-31 19:22 PST by Alexey Shvayka
Modified: 2021-05-14 11:41 PDT (History)
8 users (show)

See Also:

Attachments
Patch (25.20 KB, patch)
2020-12-31 19:25 PST, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (30.00 KB, patch)
2021-01-01 05:48 PST, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.