650673 - TI: Assertion failure: addr % sizeof(FreeCell) == 0, at ../jsgc.h:391

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: shell testcase, unpack, chdir and run "js -m -n -a -f shell.js -f main.js"

Here's another GC related problem, maybe it's even related to the previous (yet unfixed) problem. The test case is again quite complicated, unpack, chdir and run "js -m -n -a -f shell.js -f main.js" to reproduce (tested on TI ac0989a03bf1, 32 bit).

Comment 1

[Brian Hackett [Laid off!]]

The problem here is that after calling NewScriptFromCG during compileScript, we don't root it and call defineGlobals, which can make new shapes and trigger a GC.  That GC can't (I think) collect the new shape since no script object has been made for it yet, but the script isn't traced and the GC can collect the bindings which have been removed from the cg and added to the new script.

This problem also seems to be on TM.  The only reason it doesn't show up there seems to be that the JM branch allocates shapes slightly differently (different empty shapes for each script) and there is a small window where the GC can happen for the bug to occur.  Even with gczeal(2), GCs only occur when arenas get filled up and not on every allocation.

Would it be possible to have a gczeal(3) or something that GCs on every allocation?  (is that how gczeal used to work?)

Comment 2

[Christian Holler (:decoder)]

If gczeal(2) does not execute a GC at every possible point of time where it could also happen in normal execution mode, then I second the need for gczeal(3). It would probably make it much easier to find certain GC bugs.

Comment 4

[Brian Hackett [Laid off!]]

Attachment: patch

Patch against JM (similar in TM except the ensureShape call is not there).  I think this is causing orange on the JM tinderbox now, and should go in before the fixes to gczeal (bug 650978).

How should this land?  I think this hazard has been in since November.

Comment 5

[David Mandelin [:dmandelin]]

Comment on attachment 533881 [details] [diff] [review]
patch

It would be nice to replace "goto bad" with RAII but that's optional. I think billm is the one you want to coordinate landing with.

Comment 6

[Brian Hackett [Laid off!]]

With bug 650978 fixed this should have started causing problems in TM (were it a TM hazard) but didn't.  Bill looked into this more closely (thanks!) and it turns out the bindings at risk are always empty (always global scripts), so the shape is the compartment's empty call shape which the conservative GC seems to always be able to find a reference to (luckily, as we don't take care to keep it rooted).  This affects TI because it uses a different empty shape for each script, which does not get accidentally rooted.  Pushed the fix to TM, but I don't think this needs to go into aurora or beta.

http://hg.mozilla.org/tracemonkey/rev/14f91384ce26

Comment 7

[Bob Clary [:bc] (inactive)]

This assertion was seen once in crash automation on Windows XP Beta on http://tintuconline.com.vn/vn/index.html I haven't tried to reproduce locally.

Comment 8

[Brian Hackett [Laid off!]]

Looks fixed on m-c.

http://hg.mozilla.org/mozilla-central/rev/14f91384ce26

Re: comment 7, this is a generic assertion indicating heap corruption detected by the GC, so could be caused by any number of factors.

Comment 9

[u279076]

Would QA be able to verify this using one of the debug builds from FTP? If so, some instruction would be appreciated. Thanks.

Comment 10

[Christian Holler (:decoder)]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #9)
> Would QA be able to verify this using one of the debug builds from FTP? If
> so, some instruction would be appreciated. Thanks.

If the location you are downloading from offers a jsshell-*.zip file (like the Tinderbox debug builds do), then you can use the js shell in there (the js binary) to run the test as described in comment 0. If you don't have the js shell but want to build it, then you can do so using the branch you want to test on. If you want me to help you with that and/or give you build instructions/an easy build script, let me know! :)

Comment 11

[u279076]

(In reply to Christian Holler (:decoder) from comment #10)
> If the location you are downloading from offers a jsshell-*.zip file (like
> the Tinderbox debug builds do), then you can use the js shell in there.

This would be simplest I think. Could you point me to some test builds to use for verification?

Comment 12

[Christian Holler (:decoder)]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #11)
> This would be simplest I think. Could you point me to some test builds to
> use for verification?

Sure. For example for x86-64 aurora, you can use the jsshell-*.zip in https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-linux64-debug/1323342110/

Let me know if you need anything else :)

Comment 13

[u279076]

Thanks Christian. Marking [qa+] for verification with the build (and similar) as per comment 12.