...
Created attachment 296254 [details] WIP Most things are in place now, I just need to teach StackVisitor about wasm.
Created attachment 296323 [details] WIP Just threw a wasm exception: TypeError: Out of bounds memory access ------------------------ <wasm>@[wasm code] <wasm>@[wasm code] testWasmModuleFunctions@[native code] module code@/Volumes/Data/WK/a/OpenSource/JSTests/wasm/function-tests/i32-load.js:27:32 evaluate@[native code] moduleEvaluation@[native code] [native code] promiseReactionJob@[native code] ------------------------
Created attachment 296760 [details] WIP might be done. I need to review the code and write a changelog.
Created attachment 296765 [details] patch
Created attachment 296767 [details] patch rebased
Attachment 296767 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:47: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/wasm/js/JSWebAssemblyModule.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/interpreter/StackVisitor.cpp:33: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:27: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 5 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296768 [details] patch fix merge conflict compile error.
Attachment 296768 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:47: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/wasm/js/JSWebAssemblyModule.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/interpreter/StackVisitor.cpp:33: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:27: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 5 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
View in context: https://bugs.webkit.org/attachment.cgi?id=296767&action=review lgtm, but I'm not super familiar with this code so it would be good to have another look. > Source/JavaScriptCore/API/JSContextRef.cpp:276 > + if (visitor->hasLineAndColumnInfo()) { Should we have a fix to collect wasm function number and index? > Source/JavaScriptCore/interpreter/StackVisitor.cpp:162 > +#if ENABLE(WEBASSEMBLY) Do you need these #if here and elsewhere? You're not using anything which isn't available if not wasm. > Source/JavaScriptCore/interpreter/StackVisitor.cpp:329 > + // FIXME: This seems fine, why FIXME? > Source/JavaScriptCore/runtime/JSCell.cpp:301 > + return inherits(JSWebAssemblyCallee::info()) || inherits(WebAssemblyToJSCallee::info()); I added a function which does this in Repatch.cpp, we should share it.
Looks like WebCore uses StackVisitor.
Comment on attachment 296768 [details] patch I think this seems reasonable assuming you fix the WebCore build issues. Although, I don't fully understand why we would export StackVisitor to WebCore. It also, looks like you need to fix the Wasm-less builds too.
Created attachment 296788 [details] patch for landing
Attachment 296788 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:47: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/wasm/js/JSWebAssemblyModule.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/interpreter/StackVisitor.cpp:33: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:27: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 5 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296791 [details] patch for landing
Created attachment 296792 [details] patch for landing
Attachment 296792 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:47: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:188: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/wasm/js/JSWebAssemblyModule.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/interpreter/StackVisitor.cpp:33: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:27: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/JavaScriptCore/runtime/JSCell.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 6 in 42 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296794 [details] patch for landing
Created attachment 296795 [details] patch for landing
Attachment 296795 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:188: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296796 [details] patch for landing
Attachment 296796 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:188: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296798 [details] patch for landing
Attachment 296798 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:188: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 296798 [details] patch for landing Attachment 296798 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2687180 New failing tests: inspector/codemirror/prettyprinting-css-rules.html
Created attachment 296799 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 296798 [details] patch for landing Attachment 296798 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2687207 New failing tests: imported/w3c/web-platform-tests/streams/readable-streams/templated.https.html imported/w3c/web-platform-tests/streams/readable-streams/general.https.html js/dom/Promise-static-all.html imported/w3c/web-platform-tests/streams/byte-length-queuing-strategy.https.html streams/readable-stream-pipeThrough.html imported/w3c/web-platform-tests/streams/count-queuing-strategy.https.html imported/w3c/web-platform-tests/streams/readable-streams/pipe-through.https.html js/dom/Promise-static-race.html
Created attachment 296800 [details] Archive of layout-test-results from ews113 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 296822 [details] patch for landing
Attachment 296822 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:188: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 296822 [details] patch for landing Attachment 296822 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2692102 New failing tests: inspector/codemirror/prettyprinting-javascript.html
Created attachment 296834 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
(In reply to comment #31) > Created attachment 296834 [details] > Archive of layout-test-results from ews103 for mac-yosemite > > The attached test failures were seen while running run-webkit-tests on the > mac-ews. > Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5 Hmmm, the crashes here don't look related to my patch. I'm going to rebase the patch and re-upload for EWS.
Created attachment 296874 [details] patch for landing
Attachment 296874 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:188: Multi line control clauses should use braces. [whitespace/braces] [4] Total errors found: 1 in 41 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 296874 [details] patch for landing Clearing flags on attachment: 296874 Committed r209696: <http://trac.webkit.org/changeset/209696>
All reviewed patches have been landed. Closing bug.
(In reply to comment #35) > Comment on attachment 296874 [details] > patch for landing > > Clearing flags on attachment: 296874 > > Committed r209696: <http://trac.webkit.org/changeset/209696> It broke the Linux builds, see build.webkit.org for details.
(In reply to comment #37) > (In reply to comment #35) > > Comment on attachment 296874 [details] > > patch for landing > > > > Clearing flags on attachment: 296874 > > > > Committed r209696: <http://trac.webkit.org/changeset/209696> > > It broke the Linux builds, see build.webkit.org for details. About to land a speculative fix
Tried to fix the build in: https://trac.webkit.org/changeset/209702
cloop build is still broken: https://build.webkit.org/builders/Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29/builds/22207
(In reply to comment #40) > cloop build is still broken: > https://build.webkit.org/builders/ > Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29/builds/22207 Fix in: https://trac.webkit.org/changeset/209722