Closed Bug 635968 Opened 9 years ago Closed 9 years ago

JM: Crash on Google Spreadsheet


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
blocking2.0 --- final+


(Reporter: dvander, Assigned: dvander)




(Whiteboard: [hardblocker][has patch][fixed-in-tracemonkey])


(1 file, 3 obsolete files)

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.
blocking2.0: --- → ?
Attached patch one pool per code region (obsolete) — Splinter Review
Brendan, can you try this patch and see if the crash morphs?
Attached patch log inc/decrefs (obsolete) — Splinter Review
No crash with the unique ref patch. This one will log when pools are created, incref'd, and decref'd.
Attachment #514319 - Attachment is obsolete: true
blocking2.0: ? → final+
OS: Linux → Mac OS X
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!

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.

Attached patch fixSplinter Review
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.
Attachment #514372 - Attachment is obsolete: true
Attachment #514621 - Attachment is obsolete: true
Attachment #514732 - Flags: review?(dmandelin)
NB: This affects x64 only, since on x86/ARM jump patching always succeeds.
Whiteboard: [hardblocker][has patch]
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!

Attachment #514732 - Flags: review?(dmandelin) → review+
Whiteboard: [hardblocker][has patch] → [hardblocker][has patch][fixed-in-tracemonkey]

Brendan, waiting with bated breath as to how this improves your random crashes :)
Closed: 9 years ago
Resolution: --- → FIXED
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 :-/).

I filed bug 637216 as a follow-up.
You need to log in before you can comment on or make changes to this bug.