Summary: | [JSC] Polymorphic call stub's slow path should restore callee saves before performing tail call | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||
Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2019-06-11 16:03:13 PDT
And polymorphic call stub's slow path seems not restoring it too. Created attachment 371914 [details]
Patch
The change of virtual call is not necessary. Only polymorphic call is problematic. Created attachment 371934 [details]
Patch
Created attachment 371935 [details]
Patch
Comment on attachment 371934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371934&action=review > Source/JavaScriptCore/ChangeLog:30 > + This patch does that skips after restoring callee saves. “does that skips” => “makes branches to the slow path happen” > JSTests/stress/poly-call-stub-slow-path-should-restore-callee-saves-when-doing-tail-call.js:5 > + var a = WeakSet.bind(); Maybe also two other tests: - One where the poly call is in the test itself instead of a built in (in case we change the built in in the future) - A test with a non cell callee Comment on attachment 371935 [details]
Patch
r=me with comments from previous patch
Comment on attachment 371934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371934&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:30 >> + This patch does that skips after restoring callee saves. > > “does that skips” => “makes branches to the slow path happen” Fixed. >> JSTests/stress/poly-call-stub-slow-path-should-restore-callee-saves-when-doing-tail-call.js:5 >> + var a = WeakSet.bind(); > > Maybe also two other tests: > - One where the poly call is in the test itself instead of a built in (in case we change the built in in the future) > - A test with a non cell callee Nice, added. Committed r246372: <https://trac.webkit.org/changeset/246372> |