Bug 220216 – Don't throw if `function.caller` is a non-strict / generator / async function

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

[Alexey Shvayka]

Don't throw if `function.caller` is a non-strict / generator / async function

[Alexey Shvayka]

Created attachment 416876 [details]
Patch

[Alexey Shvayka]

Created attachment 416881 [details]
Patch

Adjust tests.

[Yusuke Suzuki]

Comment on attachment 416881 [details]
Patch

r=me

[EWS]

Committed r271119: <https://trac.webkit.org/changeset/271119>

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

[Radar WebKit Bug Importer]

<rdar://problem/72770747>

[Yusuke Suzuki]

Oops, I think the removed tests are super important ones. I'll attempt to recover them.

[Yusuke Suzuki]

Committed r272031: <https://trac.webkit.org/changeset/272031>

[Alexey Shvayka]

(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());

?

[Yusuke Suzuki]

(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.

[Alexey Shvayka]

(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.