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

Alexey Shvayka
Reported 2020-12-31 19:22:09 PST
Don't throw if `function.caller` is a non-strict / generator / async function
Attachments
Patch (25.20 KB, patch)
2020-12-31 19:25 PST, Alexey Shvayka 		no flags
DetailsFormatted DiffDiff
Patch (30.00 KB, patch)
2021-01-01 05:48 PST, Alexey Shvayka 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alexey Shvayka
Comment 1 2020-12-31 19:25:04 PST
Created attachment 416876 [details] Patch
Alexey Shvayka
Comment 2 2021-01-01 05:48:56 PST
Created attachment 416881 [details] Patch Adjust tests.
Yusuke Suzuki
Comment 3 2021-01-02 10:39:09 PST
Comment on attachment 416881 [details] Patch r=me
EWS
Comment 4 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].
Radar WebKit Bug Importer
Comment 5 2021-01-02 10:43:24 PST
<rdar://problem/72770747>
Yusuke Suzuki
Comment 6 2021-01-28 12:30:20 PST
Oops, I think the removed tests are super important ones. I'll attempt to recover them.
Yusuke Suzuki
Comment 7 2021-01-28 12:42:08 PST
Committed r272031: <https://trac.webkit.org/changeset/272031>
Alexey Shvayka
Comment 8 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()); ?
Yusuke Suzuki
Comment 9 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.
Alexey Shvayka
Comment 10 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.
Note You need to log in before you can comment on or make changes to this bug.