1353543 - Crash in LookupAsmJSModuleInCache

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-b19edaa1-4309-41ad-94f3-7cec32170404.
=============================================================
Crashing Thread (5)
Frame 	Module 	Signature 	Source
0 	xul.dll 	LookupAsmJSModuleInCache 	js/src/wasm/AsmJS.cpp:8552
1 	xul.dll 	js::CompileAsmJS(js::ExclusiveContext*, js::frontend::Parser<js::frontend::FullParseHandler>&, js::frontend::ParseNode*, bool*) 	js/src/wasm/AsmJS.cpp:8683
2 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::asmJS(js::frontend::ParseNode*) 	js/src/frontend/Parser.cpp:3705
3 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::maybeParseDirective(js::frontend::ParseNode*, js::frontend::ParseNode*, bool*) 	js/src/frontend/Parser.cpp:3791
4 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::statementList(js::frontend::YieldHandling) 	js/src/frontend/Parser.cpp:3851
5 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::functionBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionSyntaxKind, js::frontend::Parser<js::frontend::FullParseHandler>::FunctionBodyType) 	js/src/frontend/Parser.cpp:2409
6 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::functionFormalParametersAndBody(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::ParseNode*, js::frontend::FunctionSyntaxKind) 	js/src/frontend/Parser.cpp:3487
7 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::trySyntaxParseInnerFunction(js::frontend::ParseNode*, JS::Handle<JSFunction*>, js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::FunctionSyntaxKind, js::GeneratorKind, js::FunctionAsyncKind, bool, js::frontend::Directives, js::frontend::Directives*) 	js/src/frontend/Parser.cpp:3269
8 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::functionDefinition(js::frontend::InHandling, js::frontend::YieldHandling, JS::Handle<JSAtom*>, js::frontend::FunctionSyntaxKind, js::GeneratorKind, js::FunctionAsyncKind, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:3175
9 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::functionExpr(js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction, js::FunctionAsyncKind) 	js/src/frontend/Parser.cpp:3631
10 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::primaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::TokenKind, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:9398
11 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::memberExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::TokenKind, bool, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:8463
12 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::unaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:7990
13 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:7632
14 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::expr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:7217
15 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::primaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::TokenKind, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:9440
16 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::memberExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::TokenKind, bool, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:8463
17 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::unaryExpr(js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:7990
18 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:7632
19 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::assignExpr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:7762
20 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::expr(js::frontend::InHandling, js::frontend::YieldHandling, js::frontend::TripledotHandling, js::frontend::Parser<js::frontend::FullParseHandler>::PossibleError*, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:7217
21 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::expressionStatement(js::frontend::YieldHandling, js::frontend::Parser<js::frontend::FullParseHandler>::InvokedPrediction) 	js/src/frontend/Parser.cpp:5208
22 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::statementListItem(js::frontend::YieldHandling, bool) 	js/src/frontend/Parser.cpp:7110
23 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::statementList(js::frontend::YieldHandling) 	js/src/frontend/Parser.cpp:3829
24 	xul.dll 	js::frontend::Parser<js::frontend::FullParseHandler>::globalBody(js::frontend::GlobalSharedContext*) 	js/src/frontend/Parser.cpp:1951
25 	xul.dll 	BytecodeCompiler::compileScript(JS::Handle<JSObject*>, js::frontend::SharedContext*) 	js/src/frontend/BytecodeCompiler.cpp:335
26 	xul.dll 	BytecodeCompiler::compileGlobalScript(js::ScopeKind) 	js/src/frontend/BytecodeCompiler.cpp:376
27 	xul.dll 	js::ThisThread::GetId() 	js/src/threading/windows/Thread.cpp:118
28 	xul.dll 	js::frontend::CompileGlobalScript(js::ExclusiveContext*, js::LifoAlloc&, js::ScopeKind, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, js::SourceCompressionTask*, js::ScriptSourceObject**) 	js/src/frontend/BytecodeCompiler.cpp:569
29 	xul.dll 	js::ScriptParseTask::parse() 	js/src/vm/HelperThreads.cpp:370
30 	xul.dll 	js::HelperThread::handleParseWorkload(js::AutoLockHelperThreadState&, unsigned int) 	js/src/vm/HelperThreads.cpp:1628
31 	xul.dll 	js::HelperThread::threadLoop() 	js/src/vm/HelperThreads.cpp:1882
32 	xul.dll 	js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start(void*) 	js/src/threading/Thread.h:227
33 	ucrtbase.dll 	_o__CIpow 	
34 	kernel32.dll 	BaseThreadInitThunk 	
35 	ntdll.dll 	__RtlUserThreadStart 	
36 	ntdll.dll 	_RtlUserThreadStart

this crash signature is newly showing up on firefox 52 with a moz_assert introduced in bug 1276029. many of the user comments seem to refer to playing online games as a source of the crashes.

Correlations for Firefox Release
(100.0% in signature vs 27.22% overall) "GL Context+" in app_notes = true [109.01% vs 77.63% if startup_crash = null]
(73.49% in signature vs 27.57% overall) process_type = content [55.86% vs 302.54% if startup_crash = null]
(100.0% in signature vs 00.10% overall) moz_crash_reason = MOZ_RELEASE_ASSERT(cursor == entry.memory + entry.serializedSize)

Comment 1

[Luke Wagner [:luke]]

I'm unable to reproduce trying various combinations of cache hit and cache miss on the URLs in the crashes.  Given top11 is a pretty popular game, there must be some rare condition that causes this to trigger or else we'd see this a lot more.  I had hoped that for a bug of this type, we'd get STR.  We do have a safe and clean failure path here that won't cache (it'll just cause a cache miss which will recompile), so we can just do that.

Comment 2

[Luke Wagner [:luke]]

Attachment: non-fatal-assert

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8854632 [details] [diff] [review]
non-fatal-assert

Review of attachment 8854632 [details] [diff] [review]:
-----------------------------------------------------------------

It's a bit unfortunate, but it's better than having random crashes we can't explain. Thanks!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6a73e6fcf5e
Baldr: soften LookupAsmJSModuleInCache check (r=bbouvier)

Comment 5

[Luke Wagner [:luke]]

Comment on attachment 8854632 [details] [diff] [review]
non-fatal-assert

Approval Request Comment
[Feature/Bug causing the regression]: none
[User impact if declined]: RELEASE_ASSERT() instead of safe cache miss
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not really a fix you can verify; if we could reproduce a crash, we'd just fix it
[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?]: just turns a release assert into a normal failure path; there are many other failure paths just like it
[String changes made/needed]: none

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c6a73e6fcf5e

Comment 7

[Gerry Chang [:gchang]]

Comment on attachment 8854632 [details] [diff] [review]
non-fatal-assert

Take this and see if the crash is decreased. Aurora54+.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b6746564d765

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8854632 [details] [diff] [review]
non-fatal-assert

Let's see how this does with 54 aurora/beta. Too late for 53 uplift.

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8854632 [details] [diff] [review]
non-fatal-assert

Replaces an assert with a proper fix for unexpected state. We've hit this release assert ~600 times in the past week, seems worthwhile to uplift to esr52.2.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/714bb491d200

Comment 12

[Andrei Vaida [:avaida]]

(In reply to Luke Wagner [:luke] from comment #5)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: not really a fix you can verify; if
> we could reproduce a crash, we'd just fix it
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Luke's assessment on manual testing needs and the fact that this fix has automated coverage.