635968 - JM: Crash on Google Spreadsheet

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Brendan is getting a crash on the given Google Spreadsheet URL. The two instances I've looked at so far: The first had an IC jump off into invalid memory. The second tried to enter a JIT script that had a code pointer, but the address went to invalid memory.

It seems like there's a pool refcounting bug somewhere. I'm going to attach a patch that will make all executable pools have exactly one reference (by making them over-allocate) to try and root out a failure quicker.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: one pool per code region

Brendan, can you try this patch and see if the crash morphs?

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: log inc/decrefs

No crash with the unique ref patch. This one will log when pools are created, incref'd, and decref'd.

Comment 3

[Brendan Eich [:brendan]]

See bug 633176 comment 28 et seq.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: version of logging patch that writes to /tmp/dvander.log

I've also turned off h/w acceleration. I'll try this patch again shortly without h/w acceleration, hope that avoids gfx woes, and see if it does not interfere enough that i don't get a crash. IOW, I hope to crash with a good log to attach here!

/be

Comment 5

[Brendan Eich [:brendan]]

Ok, not reproducing now. Rebooted this a.m. and am running a fresh tm tip build (8101f728882a). I will let this sit open for a good day or three.

/be

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

The hint here was that one of the crashes was in sweepCallICs, which during 1/8th-life purging, is where a ton of decrefs occur on the pool vector in JITScripts. I audited the MonoIC code and spotted this. We add the pool to execPools, then check if the code is too far away to link. If so, we decref. When we sweep again, we'll decref an extra time. WHOOPS

This is likely to explain most of Brendan's crashes, especially since his laptop was able to reproduce out-of-range linking problems very frequently, back when that didn't work at all.

The ExecuteTrace crashes are less clear. JM pages are separate from TM pages. So that might be unrelated but we'll see.

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

NB: This affects x64 only, since on x86/ARM jump patching always succeeds.

Comment 8

[Brendan Eich [:brendan]]

Cool. BTW my laptop can't be credited, since I ditched the old one due to gfx freakouts blamed wrongly on hardware (I hope Apple has tested it and let it out of rehab). It's my glorious session restore state that keeps making big gaps in mmapped memory. I have no idea why, but we need some stress-testing automaton to take over -- I'm tired!

/be

Comment 9

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/a4dedc663b68

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/mozilla-central/rev/c6c296b5629c

Brendan, waiting with bated breath as to how this improves your random crashes :)

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/a4dedc663b68

Comment 12

[Brendan Eich [:brendan]]

So far so good...

Keep an eye out for missing addRefs too, but this was a clean fix. Thanks for sticking with me and my golden session store (and my two MBPs :-/).

/be

Comment 13

[Nicholas Nethercote [inactive]]

I filed bug 637216 as a follow-up.