679094 - Crash [@ JSScript::isEmpty] // GC related corruption

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test crashes, tested on TI revision 8e7da0684155 (options -j -m -n -a -Z 2):

var TIME_0000  = (function () {  })();
function getTimeZoneDiff() {}


Seems to be a GC related problem that causes a waste amount of different crashes. Must be recently introduced during merge with m-c.

Comment 1

[Brian Hackett [Laid off!]]

This is a GC hazard, but it looks like it came in on the merge from jsdbg2 and not as a result of a merge botch.  When defining the globals in a top level script, we would root the script's binding info (primarily its empty shape), but nothing else.  This used to be OK as the script has not had its u.object created yet and is invulnerable to GCs, but now the u.object is created earlier (for the debugger newScript hook) so the script itself can be collected while defining globals.

jorendorff, is there something that should be keeping the script rooted here that I missed?  If not, this should go on m-c now I think.

http://hg.mozilla.org/projects/jaegermonkey/rev/74bfd74ca289

Comment 2

[Christian Holler (:decoder)]

Ccing clegnitto and chofmann on this, as I can confirm this affects mozilla-central (confirmed with testcase from comment 0 on revision f597467fac5e (debug build) with options "-j -m -Z 2").

Comment 3

[chris hofmann]

yeah, we should get this fixed on mozilla-central and aurora if the change ends up there after the war room work tomorrow.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Christian Holler (:decoder) from comment #0)
> Seems to be a GC related problem that causes a waste amount of different
> crashes. Must be recently introduced during merge with m-c.

fwiw, I see this too, autoBisect pointing to the m-c merge as well.

Comment 5

[Jason Orendorff [:jorendorff]]

I was never able to reproduce this, but I pushed bhackett's fix, which seems obviously correct per comment 1.

http://hg.mozilla.org/mozilla-central/rev/7027d3788076

Comment 6

[Daniel Veditz [:dveditz]]

This landed before the merge on 8-16 and is in the Firefox 8 beta repo already.

Comment 7

[u279076]

qa- as nothing to do for QA fix verification -- please set to qa+ if this is not the case.