1041307 - assertion in FinalizeArenas on non-ion platforms

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Steve Singer (:stevensn)]

On non-ion builds firefox asserts in FinalizeArenas on  MOZ_CRASH("Invalid alloc kind")


#0  0x0e0da0ac in FinalizeArenas (fop=fop@entry=0xfffecec8, src=src@entry=0xe4711e28, dest=..., thingKind=thingKind@entry=
    js::gc::FINALIZE_JITCODE, budget=...) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsgc.cpp:624
#1  0x0e0dbde0 in js::gc::ArenaLists::foregroundFinalize (this=this@entry=0xe4711c1c, fop=fop@entry=0xfffecec8, thingKind=
    js::gc::FINALIZE_JITCODE, sliceBudget=..., sweepList=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsgc.cpp:4360
#2  0x0e0dccb4 in js::gc::GCRuntime::sweepPhase (this=this@entry=0xf6f481a0, sliceBudget=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsgc.cpp:4410
#3  0x0e0de304 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0xf6f481a0, budget=<optimized out>, reason=reason@entry=
    JS::gcreason::REFRESH_FRAME, gckind=gckind@entry=js::GC_NORMAL)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsgc.cpp:4937
#4  0x0e0def88 in js::gc::GCRuntime::gcCycle (this=this@entry=0xf6f481a0, incremental=incremental@entry=true, budget=10000, 
    gckind=gckind@entry=js::GC_NORMAL, reason=reason@entry=JS::gcreason::REFRESH_FRAME)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsgc.cpp:5105

This appears to be introduced by bug 1017165

c9e3514ab40f does not experience the issue
42d193544465 does have the issue

Comment 1

[Emanuel Hoogeveen [:ehoogeveen]]

Attachment: Don't try to finalize JITcode in non-ion builds.

I bet I know why this changed. In bug 1017165 I removed the early return in foregroundFinalize so we wouldn't return before inserting the sorted arenas, but that means we can now call into FinalizeArenas even if the list of arenas to sweep is empty. As a result, when we get to the FinalizePhaseJitCode we hit this assert.

There's several ways we could fix this:
(1) We could #ifdef JS_ION the FinalizePhaseJitCode (and things that reference it),
(2) We could reintroduce the early return in foregroundFinalize (since we now keep track of the finalized arenas, we can check if that list is empty),
(3) We could just return true for FINALIZE_JITCODE in FinalizeArenas instead of falling through to the default case.

I did (1) in this patch - although it's the biggest change, it seemed the most direct to me. (2) would return us to the previous behavior, but also seems kind of non-obvious, and (3) would still do the SortedArenaList stuff, which seems kind of nasty (though it's harmless).

Steve, could you confirm that this patch fixes the problem for you?

Comment 2

[Steve Singer (:stevensn)]

Attachement 8459310 fixes the problem for me.
Thanks

Comment 3

[Emanuel Hoogeveen [:ehoogeveen]]

Doesn't look like I broke any of the builds: https://tbpl.mozilla.org/?tree=Try&rev=17565781e0bb

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/597e8c7e10e4

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/597e8c7e10e4