The following crashes on ShadowChicken when running with the JIT disabled and debugging opcodes enabled: ``` function foo() { foo() } foo(); ```
<rdar://problem/39682133>
Created attachment 342465 [details] Patch
Comment on attachment 342465 [details] Patch Please add a test for this that runs with LLInt only and shadow chicken enabled
Comment on attachment 342465 [details] Patch Attachment 342465 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/8136845 New failing tests: http/tests/misc/large-js-program.php
Created attachment 342478 [details] Archive of layout-test-results from ews112 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 342480 [details] Patch
Comment on attachment 342480 [details] Patch Attachment 342480 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/8139194 New failing tests: http/tests/misc/large-js-program.php
Created attachment 342496 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 342480 [details] Patch Attachment 342480 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8141064 New failing tests: js/regress-139548.html fast/dom/console-log-stack-overflow.html js/dom/line-column-numbers.html js/regress-141098.html js/dom/stack-trace.html js/stack-overflow-catch.html js/kde/crash-2.html fast/workers/use-machine-stack.html js/dom/deep-recursion-test.html js/stack-overflow-arrity-catch.html fast/dom/error-to-string-stack-overflow.html js/dom/global-recursion-on-full-stack.html js/function-apply-aliased.html
Created attachment 342507 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 342565 [details] Patch Fix CLoop
Comment on attachment 342565 [details] Patch Attachment 342565 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/8151444 New failing tests: js/dom/JSON-stringify.html http/tests/misc/large-js-program.php
Created attachment 342595 [details] Archive of layout-test-results from ews113 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 342565 [details] Patch Attachment 342565 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8156939 New failing tests: http/tests/preload/onload_event.html
Created attachment 342634 [details] Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 342565 [details] Patch LGTM. R=me
Comment on attachment 342565 [details] Patch Rejecting attachment 342565 [details] from commit-queue. tzagallo@apple.com does not have committer permissions according to https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Created attachment 342805 [details] Patch
Created attachment 342806 [details] Patch
Comment on attachment 342806 [details] Patch Attachment 342806 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/8197276 New failing tests: http/tests/misc/large-js-program.php
Created attachment 342813 [details] Archive of layout-test-results from ews114 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 342976 [details] Patch Fix crash when overflowing in the first JS frame
Comment on attachment 342976 [details] Patch r=me still
Comment on attachment 342976 [details] Patch Attachment 342976 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/8237604 New failing tests: js/reentrant-caching.html
Created attachment 342990 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 342976 [details] Patch Attachment 342976 [details] did not pass jsc-ews (mac): Output: http://webkit-queues.webkit.org/results/8237675 New failing tests: jsc-layout-tests.yaml/js/script-tests/reentrant-caching.js.layout-ftl-eager-no-cjit wasm.yaml/wasm/js-api/promise-stack-overflow.js.wasm-no-tls-context jsc-layout-tests.yaml/js/script-tests/regress-139548.js.layout-ftl-no-cjit wasm.yaml/wasm/js-api/promise-stack-overflow.js.wasm-no-cjit-yes-tls-context stress/regress-179355.js.ftl-no-cjit-small-pool wasm.yaml/wasm/js-api/promise-stack-overflow.js.wasm-eager-jettison wasm.yaml/wasm/js-api/promise-stack-overflow.js.wasm-no-call-ic jsc-layout-tests.yaml/js/script-tests/regress-139548.js.layout-no-ftl jsc-layout-tests.yaml/js/script-tests/regress-139548.js.layout jsc-layout-tests.yaml/js/script-tests/regress-139548.js.layout-no-cjit stress/regress-179355.js.ftl-no-cjit-no-inline-validate wasm.yaml/wasm/js-api/promise-stack-overflow.js.wasm-slow-memory wasm.yaml/wasm/js-api/promise-stack-overflow.js.default-wasm wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-no-call-ic jsc-layout-tests.yaml/js/script-tests/reentrant-caching.js.layout-dfg-eager-no-cjit apiTests
Comment on attachment 342976 [details] Patch Attachment 342976 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/8238145 New failing tests: js/regress-139548.html
Created attachment 342992 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 342976 [details] Patch Attachment 342976 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/8237948 New failing tests: js/dom/string-replace-exception-crash.html js/regress-139548.html
Created attachment 342993 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 342976 [details] Patch Attachment 342976 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8238384 New failing tests: js/regress-139548.html js/regress-141098.html
Created attachment 342995 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 343093 [details] Patch try a different approach
Comment on attachment 343093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343093&action=review > Source/JavaScriptCore/interpreter/ShadowChicken.cpp:303 > + JSValue scopeValue = callFrame->bytecodeOffset() && codeBlock && codeBlock->scopeRegister().isValid() This feels very precarious. Are we just assuming that the second byte code will always be getScope? What was wrong with the previous approach?
Comment on attachment 343093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343093&action=review > Source/JavaScriptCore/interpreter/ShadowChicken.cpp:306 > + if (scopeValue.isUndefined() && codeBlock->wasCompiledWithDebuggingOpcodes() && !scopeValue.isUndefined()) { this code is wrong. It can't both be undefined and not undefined.
Comment on attachment 343093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343093&action=review > Source/JavaScriptCore/ChangeLog:13 > + initialize it to undefined). I see. This may be an OK thing to rely on. But I wonder if we still have bugs w.r.t how we handled stack overflow in the LLInt.
Created attachment 343096 [details] Patch
Comment on attachment 343096 [details] Patch r=me What was wrong with the previous approach? I wonder if we still have subtle bugs in the LLInt's stack overflow code
I talked with Phil today, and it seems that it might be better to always handle the stack overflows from the callee instead of from the caller, given that the caller frame may not always be what we need. I will add a follow up bug to update the JIT too.
Comment on attachment 343096 [details] Patch Clearing flags on attachment: 343096 Committed r232983: <https://trac.webkit.org/changeset/232983>
All reviewed patches have been landed. Closing bug.