We already have liveness, so using it for a simple stack allocator should be easy
This fixes our Wasm stack overflow issues
*** Bug 201026 has been marked as a duplicate of this bug. ***
*** Bug 200918 has been marked as a duplicate of this bug. ***
Created attachment 388084 [details] patch I just need to adjust my stack limit test since we change stack size in our tests
Created attachment 388090 [details] patch
Created attachment 388091 [details] patch
Comment on attachment 388091 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=388091&action=review > Source/JavaScriptCore/ChangeLog:9 > + code creating huge stack frames and causing simple Wasm code to stack I'll fix "and causing" => "was causing"
*** Bug 206419 has been marked as a duplicate of this bug. ***
Comment on attachment 388091 [details] patch r=me
<rdar://problem/58702105>
Created attachment 388122 [details] patch for landing
The commit-queue encountered the following flaky tests while processing attachment 388122 [details]: editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org) The commit-queue is continuing to process your patch.
Comment on attachment 388122 [details] patch for landing Clearing flags on attachment: 388122 Committed r254788: <https://trac.webkit.org/changeset/254788>
All reviewed patches have been landed. Closing bug.
> Committed r254788: <https://trac.webkit.org/changeset/254788> This seems to have broken 6 JSC tests, as also indicated by EWS. JSC stress test failures: - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit-validate-phases - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit-validate-phases
(In reply to Aakash Jain from comment #15) > > Committed r254788: <https://trac.webkit.org/changeset/254788> > This seems to have broken 6 JSC tests, as also indicated by EWS. > > JSC stress test failures: > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint > - > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit- > validate-phases > - > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit- > validate-phases This seems super unlikely. These aren't wasm tests, right?
(In reply to Saam Barati from comment #16) > (In reply to Aakash Jain from comment #15) > > > Committed r254788: <https://trac.webkit.org/changeset/254788> > > This seems to have broken 6 JSC tests, as also indicated by EWS. > > > > JSC stress test failures: > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint > > - > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit- > > validate-phases > > - > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit- > > validate-phases > > This seems super unlikely. These aren't wasm tests, right? Also, a lot of these tests can't even hit this code path. Are you sure it wasn't a patch before mine?
(In reply to Saam Barati from comment #17) > (In reply to Saam Barati from comment #16) > > (In reply to Aakash Jain from comment #15) > > > > Committed r254788: <https://trac.webkit.org/changeset/254788> > > > This seems to have broken 6 JSC tests, as also indicated by EWS. > > > > > > JSC stress test failures: > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint > > > - > > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit- > > > validate-phases > > > - > > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit- > > > validate-phases > > > > This seems super unlikely. These aren't wasm tests, right? > > Also, a lot of these tests can't even hit this code path. Are you sure it > wasn't a patch before mine? Ugh, maybe it was my use of "requireOptions". I wonder if we forgot to clear that value and then we end up running this test with a super small stack height.
(In reply to Saam Barati from comment #17) > Also, a lot of these tests can't even hit this code path. Are you sure it wasn't a patch before mine? r254787 passed in https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/145 254788 failed in https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/146
From https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/146/steps/jscore-test/logs/stdio mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit-validate-phases: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit-validate-phases: Exception: RangeError: Maximum call stack size exceeded.
The regression is tracked in bug 206477.
Noticed this is still happening, getting RangeError: Maximum call stack size exceeded. upon instantiation of WASM code. If I refresh, it tends to fix it, but only if I refresh things. On older safari (13.0) it doesn't work at all, so this is an improvement, but it doesn't seem to be totally fixed