Closed Bug 1346269 Opened 5 years ago Closed 5 years ago
Crash in js::jit::Auto
Flush ICache::Auto Flush ICache
This bug was filed from the Socorro interface and is report bp-02ae372f-45d3-46ac-8b4b-e32cd2170309. ============================================================= This is the #2 crash in the Android nightly of 20170309110217.
wasm::DeserializeModule on the stack.
Ah, so we're deserializing a module off the main thread and the build-id mismatches so we're recompiling, and deep inside someone is assuming we're on a JSContext's main thread so TlsContext.get() is non-null, but we're on some IDB thread so it is indeed null. The whole point of this AFC is just to suppress icache flushing; let's see if there's a better way to do that that doesn't involve a JSContext*.
The simplest solution I found is just to make the suppression explicit as a parameter.
Assignee: nobody → luke
Status: NEW → ASSIGNED
Attachment #8846059 - Flags: review?(bbouvier)
Comment on attachment 8846059 [details] [diff] [review] fix-afc Review of attachment 8846059 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #8846059 - Flags: review?(bbouvier) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/195ba401a2c6 Baldr: remove dependency on TlsContext in ModuleGenerator::finish (r=bbouvier)
crash-stats shows crashes on 52 for this (all Android, though). Given that this patch touches non-ARM code as well, I'm wondering if it's worth backporting to ESR52 as well. There are some Tier 3 ARM & Android builds off that branch IIRC.
esr52 hasn't shipped wasm (bug 1342440), so no need to backport there. Other platforms are worth it, though. Thanks!
Comment on attachment 8846059 [details] [diff] [review] fix-afc Approval Request Comment [Feature/Bug causing the regression]: 1342060 [User impact if declined]: crash in FF Android when deserializing wasm module after build-id change [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: yes [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?]: pretty simple change, just adding/propagating an additional parameter
Comment on attachment 8846059 [details] [diff] [review] fix-afc fix a crash in wasm, beta53+, aurora54+
Setting qe-verify- based on Luke's assessment on manual testing needs (see Comment 10) and the fact that this fix has automated coverage.
You need to log in before you can comment on or make changes to this bug.