Bug 176981 – Arity fixup during inlining should do a 2 phase commit so it properly recovers the frame in case of exit

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

[Saam Barati]

Yusuke convinced me we really don't want to exit during arity fixup. What he wrote:

[arg3][arg2][arg1][arg0]
[fix ][fix ][arg3][arg2][arg1][arg0]

In this case, when moving arg2, it writes arg2 to arg0's place. At that time, area is like,
[arg3][arg2][arg1][arg2][arg1][arg0]
So, in the middle of arity fixup, the region may be clobbered like,
[arg3][arg2][arg1][arg2]
If re-execute arity-fixup again, the stack becomes,
[fix ][fix ][arg3][arg2][arg1][arg2]

[Saam Barati]

*** Bug 176989 has been marked as a duplicate of this bug. ***

[Saam Barati]

Created attachment 320880 [details]
patch

[Build Bot]

Attachment 320880 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:24:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Saam Barati]

Created attachment 320881 [details]
patch

remove tab character

[Yusuke Suzuki]

Comment on attachment 320881 [details]
patch

r=me, this implementation is ideal one!

[Build Bot]

Comment on attachment 320881 [details]
patch

Attachment 320881 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/4556129

New failing tests:
stress/arrowfunction-lexical-bind-supercall-2.js.dfg-eager-no-cjit-validate
stress/promise-finally.js.ftl-eager-no-cjit
stress/arrowfunction-lexical-bind-supercall-2.js.ftl-eager-no-cjit-b3o1
stress/arrowfunction-lexical-bind-supercall-2.js.no-cjit-validate-phases
stress/arrowfunction-lexical-bind-supercall-2.js.ftl-no-cjit-no-put-stack-validate
stress/arrowfunction-lexical-bind-supercall-2.js.ftl-no-cjit-validate-sampling-profiler
stress/arrowfunction-lexical-bind-supercall-2.js.ftl-eager-no-cjit

[Build Bot]

Comment on attachment 320881 [details]
patch

Attachment 320881 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4556374

New failing tests:
imported/w3c/web-platform-tests/streams/readable-byte-streams/general.dedicatedworker.html

[Build Bot]

Created attachment 320885 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Yusuke Suzuki]

Comment on attachment 320881 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320881&action=review

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1578
>          calleeVariable->mergeShouldNeverUnbox(true);

Is this ok?
I think we need to consider LoadVarargs case too. But this does not become problem, because LoadVarargs case does not have arityFixupCount (isVarargs() case, we respect to function->parameterCount() for var args stack allocation.)

[Build Bot]

Comment on attachment 320881 [details]
patch

Attachment 320881 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4556505

New failing tests:
imported/w3c/web-platform-tests/streams/readable-byte-streams/general.dedicatedworker.html

[Build Bot]

Created attachment 320886 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Saam Barati]

Created attachment 320943 [details]
patch for landing

[Saam Barati]

The fix here is to have semantic origin be the caller's frame, and forExit to be op_enter of callee.

[WebKit Commit Bot]

Comment on attachment 320943 [details]
patch for landing

Clearing flags on attachment: 320943

Committed r222115: <http://trac.webkit.org/changeset/222115>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693313>