Created attachment 8822732 [details] test.wasm The corresponding wasm file generated with the above test case.
In a debug build, this appears to manifest as: Assertion failure: kind_ == DefinitionKind::Function, at .../src/js/src/wasm/WasmAST.h:668 I see this in Aurora but not in Nightly. Binary search points to this: https://hg.mozilla.org/integration/mozilla-inbound/rev/bcaa68d34a94 as the patch where the assertion failure appears to be fixed.
Product: Firefox → Core
Thanks for opening the report, and thanks for letting me know about this. I think this has been fixed by/into a massive refactoring of the sections decoding in general. We've decided *not* to uplift the patches because they were big and risky, but now I do realize that this means that opening the devtools may indeed crash the entire browser. Only 52 is affected, on which we probably need to mitigate the issue by just not displaying anything; or, we could just uplift it all. I'll investigate mitigating on Aurora.
Since debugging doesn't work at all in 52, I think it makes sense to just remove the BinaryToText() call from Code::createText(), keeping just the experimental message, updated appropriately.
Created attachment 8823633 [details] [diff] [review] no-source.patch Approval Request Comment [Feature/Bug causing the regression]: risky patches not uplifted to aurora [User impact if declined]: crashes when running WebAssembly programs with devtools open [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: not a fix for nightly; just a feature to disable in aurora; manual shell testing passed; manual browser testing will be done once the try build finishes. [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: it is temporarily disabling a feature (namely, wasm text format generation in the debugger devtools), so it is very conservative. [String changes made/needed]: n/a Try build to test the results in a browser: https://treeherder.mozilla.org/#/jobs?repo=try&revision=57d6de37204868438bc2f5d76b3558f39c5c9f8a
Comment on attachment 8823633 [details] [diff] [review] no-source.patch Review of attachment 8823633 [details] [diff] [review]: ----------------------------------------------------------------- typo in test comment ::: js/src/jit-test/tests/debug/wasm-05.js @@ +2,5 @@ > // when source text is generated. > > load(libdir + "asserts.js"); > > +// Disabled in aurora (see also bug 1326462). bug number is 1326452
Thanks, fixed locally.
Comment on attachment 8823633 [details] [diff] [review] no-source.patch Review of attachment 8823633 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8823633 - Flags: review?(luke) → review+
Created attachment 8823745 [details] [diff] [review] no-source.patch Approval Request Comment: see previous comments. Updated test comment; carrying forward r+.
Fwiw, I've manually checked that the expected message showed up in the devtools console in aurora, and it does. This will be confusing for users of aurora52 because the message tells to download aurora, but it will be correct for when 52 is beta.
status-firefox52: --- → affected
tracking-firefox52: --- → ?
Comment on attachment 8823745 [details] [diff] [review] no-source.patch avoid crash in devtools with wasm in aurora52
Attachment #8823745 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → unaffected
status-firefox52: affected → fixed
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.