868334 - crash in js::ion::AssemblerX86Shared::bind

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Scoobidiver (away)]

It has spiked across Nightly builds since May 1 and is #1 top crasher in today's build.

Every comments talk about Epic Citadel (http://www.unrealengine.com/html5/).

It doesn't crash for me using the Benchmark menu.

There are two kinds of stack traces:
Frame 	Module 	Signature 	Source
0 	mozjs.dll 	js::ion::AssemblerX86Shared::bind 	js/src/ion/shared/Assembler-x86-shared.h:574
1 	mozjs.dll 	js::ion::MacroAssemblerX86Shared::emitSet 	js/src/ion/shared/MacroAssembler-x86-shared.h:455
2 	mozjs.dll 	js::RegExpObject::createNoStatics 	js/src/vm/RegExpObject.cpp:237
3 	nss3.dll 	md_UnlockAndPostNotifies 	nsprpub/pr/src/md/windows/w95cv.c:152
4 	mozjs.dll 	GenerateCodeForFinishedJob 	js/src/ion/AsmJS.cpp:4541
5 	mozjs.dll 	CheckFunctionBodiesParallelImpl 	js/src/ion/AsmJS.cpp:4579
6 	mozjs.dll 	CheckFunctionBodiesParallel 	js/src/ion/AsmJS.cpp:4672
7 	mozjs.dll 	CheckModule 	js/src/ion/AsmJS.cpp:5474
8 	mozjs.dll 	js::CompileAsmJS 	js/src/ion/AsmJS.cpp:5535
...

Frame 	Module 	Signature 	Source
0 	mozjs.dll 	js::ion::AssemblerX86Shared::bind 	js/src/ion/shared/Assembler-x86-shared.h:574
1 	mozjs.dll 	js::ion::CodeGeneratorX86::visitAsmJSStoreHeap 	js/src/ion/x86/CodeGenerator-x86.cpp:582
2 	mozjs.dll 	js::ion::LAsmJSStoreHeap::accept 	js/src/ion/LIR-Common.h:4404
3 	mozjs.dll 	js::ion::CodeGenerator::generateBody 	js/src/ion/CodeGenerator.cpp:2379
4 	mozjs.dll 	js::ion::CodeGenerator::generateAsmJS 	js/src/ion/CodeGenerator.cpp:4861
5 	mozjs.dll 	js::ion::GenerateCode 	js/src/ion/Ion.cpp:1211
6 	mozjs.dll 	GenerateAsmJSCode 	js/src/ion/AsmJS.cpp:4435
7 	mozjs.dll 	GenerateCodeForFinishedJob 	js/src/ion/AsmJS.cpp:4541
8 	mozjs.dll 	CheckFunctionBodiesParallelImpl 	js/src/ion/AsmJS.cpp:4579
9 	mozjs.dll 	CheckFunctionBodiesParallel 	js/src/ion/AsmJS.cpp:4672
10 	mozjs.dll 	CheckModule 	js/src/ion/AsmJS.cpp:5474
11 	mozjs.dll 	js::CompileAsmJS 	js/src/ion/AsmJS.cpp:5535
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3Aion%3A%3AAssemblerX86Shared%3A%3Abind%28js%3A%3Aion%3A%3ALabel*%29

Comment 1

[Luke Wagner [:luke]]

A few observations:
 - no mac crashes (despite many webdevs using macs), seems related to Win or x86
 - there was a trickle of crashes (~3 a day) leading up to the Epic demo release spike
 - for the Epic demo spike, there are many with the same address (e.g. 0x797ffffc) and almost all of them end with ffffc; addresses don't have this pattern before the Epic demo spike

The Epic demo uses up a lot of memory, so one guess is that this is a bug related to OOM that is triggered more often with this demo.

Comment 2

[Luke Wagner [:luke]]

Attachment: test oom in masm.nextJump

Looking at the crash site and looking for ways OOM could crash us, I do see one big possibility: if OOM occurs when growing masm's internal buffer.  masm.nextJump doesn't check oom(), even though its siblings setNextJump and linkJump do (with a scary comment explaining that the embedded links can become garbage).  This patch adds an oom() check to nextJump as well.

Comment 3

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ea4106350e9

We'll leave this open and see what happens to the crashes.  Unfortunately, they are decreasing on their own, presumably b/c less people are visiting the demo.  Hopefully, this patch would be an abrupt decline.

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5ea4106350e9

Comment 5

[Joonas]

Apparently this fixed the crash for me: no crash with the newest 2013-05-05 nightly.

Yesterday's 2013-05-04 used to crash every time. Here's an example crash report:
https://crash-stats.mozilla.com/report/index/bp-71b0467e-084c-42a7-b512-465b92130504

PS. Although it doesn't crash anymore, the demo still doesn't work. It's stuck on "Preparing JavaScript..."

Comment 6

[Sean Stangl [:sstangl]]

(In reply to Joonas Marttila from comment #5)
> PS. Although it doesn't crash anymore, the demo still doesn't work. It's
> stuck on "Preparing JavaScript..."

The browser is running out of memory. If you open the Nightly -> Developer -> Web Console, there should be a message about failed asm.js compilation.

Comment 7

[Joonas]

I ran the demo multiple times (failed every time).

In the Web Console, I saw either
this: "uncaught exception: out of memory"
... or this: "out of memory"
... or these lines: "TypeError: asm.js type error: Internal codegen failure (probably out of memory) UDKGame-....js:9654" -- followed with "out of memory"

I'm using 32-bit Windows 7 with 4GiB of RAM installed, which AFAIK is more than 32-bit Windows can address. I had just rebooted and didn't have any other programs open. So 2+ GiB RAM available.

Comment 8

[Robert Kaiser]

From looking at crash-stats, the crashes with this signature are gone with the checkin of this patch.

Comment 9

[Luke Wagner [:luke]]

Comment on attachment 745357 [details] [diff] [review]
test oom in masm.nextJump

[Approval Request Comment]
Bug caused by (feature/regressing bug #): IonMonkey
User impact if declined: crash on OOM
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low

Comment 10

[Luke Wagner [:luke]]

(In reply to Joonas Marttila from comment #7)
How many cores do you have?

Comment 11

[Alex Keybl [:akeybl]]

(In reply to Luke Wagner [:luke] from comment #9)
> Comment on attachment 745357 [details] [diff] [review]
> test oom in masm.nextJump
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): IonMonkey

What caused the crash volume regression in FF23 then?

Doesn't look like this is needed for FF22, given very low crash volume there (unless you disagree).

Comment 12

[Luke Wagner [:luke]]

(In reply to Alex Keybl [:akeybl] from comment #11)
> What caused the crash volume regression in FF23 then?

The Epic Citadel demo being released (http://www.unrealengine.com/html5).  The page instructed people to get the latest Firefox nightly, which is presumably why FF23 didn't have the same spike.

> Doesn't look like this is needed for FF22, given very low crash volume there
> (unless you disagree).

Once FF22 gets to release and we people start trying these asm.js demos on release, there will be crashes.

Comment 13

[Joonas]

(In reply to Luke Wagner [:luke] from comment #10)
> How many cores do you have?

2 cores, 4 logical processors (threads).

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Ioana (away)]

I tried to reproduce this issue with http://www.unrealengine.com/html5/. On a 05/01 Nightly, the browser freezes for a few seconds, but I get no crash. With the 07/11 Firefox 23b5 I don't get the freeze anymore.

There are a lot of very recent crashes with this signature though:
https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&reason_type=contains&date=2013-07-12&range_value=28&range_unit=days&hang_type=any&process_type=any&signature=js%3A%3Aion%3A%3AAssemblerX86Shared%3A%3Abind%28js%3A%3Aion%3A%3ALabel%2A%29

Comment 16

[Mihai Morar, (:MihaiMorar)]

I think this is not fixed yet. As Ioana mentioned in Comment 15 there are lots of crashes on FF 23 beta, but most of them are on Windows XP instead of Windows 7.

https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&reason_type=contains&date=2013-07-16&range_value=7&range_unit=days&hang_type=any&process_type=any&signature=js%3A%3Aion%3A%3AAssemblerX86Shared%3A%3Abind%28js%3A%3Aion%3A%3ALabel%2A%29

I can't reproduce the crash with signature [@ js::ion::AssemblerX86Shared::bind(js::ion::Label*)] only a couple of hangs on http://www.unrealengine.com/html5/ using Nightly 2013-05-03

OS's used: WIndows 7 x86 and Windows XP x86.

Comment 17

[Scoobidiver (away)]

(In reply to Mihai Morar, QA [:MarioMi] from comment #16)
> I think this is not fixed yet
See:
(In reply to Scoobidiver from comment #0)
> It has spiked across Nightly builds since May 1 and is #1 top crasher in
> today's build.

The spike is fixed.

Comment 18

[u279076]

Marking this verified since it appears the spike has been fixed.