Closed Bug 1326452 Opened 7 years ago Closed 7 years ago

Crash in AstDecodeExpr


(Core :: JavaScript Engine: JIT, defect, P1)

52 Branch



Tracking Status
firefox51 --- unaffected
firefox52 + fixed
firefox53 --- fixed


(Reporter: sharmalay, Assigned: bbouvier)



Crash Data


(3 files, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-c0bb0466-b22a-4854-8cf3-d2c8d2161230.

Reloading a web page containing wasm with Developer Tools open causes Firefox to crash. Also switching to the Debugger tab of Developer Tools causes a crash.

Steps to reproduce:
1) Ensure config option `javascript.options.wasm` is set to true.
2) Compile the attached C source to a HTML file using the emcc command `emcc -s WASM=1 -s "BINARYEN_METHOD='native-wasm,asmjs,interpret-binary'" -o test.html test.c`.
3) Open the resulting HTML file in Firefox over HTTP.
4) Open Developer Tools.
5) Reload page, or switch to Debugger tab.

The past few updates of Firefox Developer Edition (and possibly before, cannot confirm) have been affected by this. Using emcc version 1.37.0. The wasm demo on does not experience this problem. As per the crash report, the offending module is
Blocks: wasm
Attached file 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:

as the patch where the assertion failure appears to be fixed.
Component: Developer Tools → JavaScript Engine: JIT
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.
Flags: needinfo?(bbouvier)
Flags: needinfo?(bbouvier)
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.
Attached patch no-source.patch (obsolete) — Splinter Review
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:
Assignee: nobody → bbouvier
Ever confirmed: true
Attachment #8823633 - Flags: review?(luke)
Attachment #8823633 - Flags: approval-mozilla-aurora?
Comment on attachment 8823633 [details] [diff] [review]

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]

Review of attachment 8823633 [details] [diff] [review]:

Attachment #8823633 - Flags: review?(luke) → review+
Attached patch no-source.patchSplinter Review
Approval Request Comment: see previous comments.

Updated test comment; carrying forward r+.
Attachment #8823633 - Attachment is obsolete: true
Attachment #8823633 - Flags: approval-mozilla-aurora?
Attachment #8823745 - Flags: review+
Attachment #8823745 - Flags: approval-mozilla-aurora?
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.
Priority: -- → P1
Comment on attachment 8823745 [details] [diff] [review]

avoid crash in devtools with wasm in aurora52
Attachment #8823745 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Blocks: 1317319
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.