975959 - GenerationalGC: Crash [@ js::jit::AssertValidObjectPtr]

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

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

try {
    y = z = [];
    y[1] = x
    for (var e, x = [];; d) {}
} catch (e) {}
try {
    Object.defineProperty(this, "y", {
        get: function() {
            z.filterPar(function() {})
        }
    })(1 instanceof 2)
} catch (e) {}
y
verifyprebarriers()
y
verifyprebarriers()
y

crashes js debug shell (tested with a threadsafe deterministic 32-bit debug build) on m-c changeset 1238ef12b996 with at js::jit::AssertValidObjectPtr and crashes js opt shell with a memory read failure.

My configure options are: (32-bit opt shell)

LD=ld CROSS_COMPILE=1 CXX="clang++ -Qunused-arguments -arch i386" RANLIB=ranlib CC="clang -Qunused-arguments -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments" HOST_CXX="clang++ -Qunused-arguments" sh ./configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-gcgenerational --with-ccache --enable-threadsafe <other NSPR options>

32-bit debug shell:

LD=ld CROSS_COMPILE=1 CXX="clang++ -Qunused-arguments -arch i386" RANLIB=ranlib CC="clang -Qunused-arguments -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments" HOST_CXX="clang++ -Qunused-arguments" sh ./configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-more-deterministic --enable-exact-rooting --enable-gcgenerational --with-ccache --enable-threadsafe <other NSPR options>

Comment 1

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

> build) on m-c changeset 1238ef12b996 with at js::jit::AssertValidObjectPtr

with * --fuzzing-safe --ion-parallel-compile=off --ion-eager *

Comment 2

[Terrence Cole [:terrence]]

This is a nursery allocation that gets stored into a pre-tenured CallObject's arguments vector that then escapes through a heavyweight function. The escaping function refers to the arg directly as an upvar after a GC. This case is a bit detailed, which is probably why it has only come up once before. There was a similarly hard to track down fuzz bug when working on bug 889129 that caught this exact case. Brian removed this code in bug 941311, probably by accident as it wasn't in the uploaded patch. Should be easy to add back.

I think bug 975961 and bug 976021 are both dups of this issue; will fix and verify tomorrow.

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 4

[Terrence Cole [:terrence]]

Ah, in our OOL path we have:

#ifdef JSGC_GENERATIONAL
    // The JIT creates call objects in the nursery, so elides barriers for
    // the initializing writes. The interpreter, however, may have allocated
    // the call object tenured, so barrier as needed before re-entering.
    if (!IsInsideNursery(cx->runtime(), obj))
        cx->runtime()->gcStoreBuffer.putWholeCell(obj);
#endif

Sa failure to allocate in the nursery from the jit is a failure in this one case. Unfortunately, in newGCThing we skip to allocating tenured if the nursery is disabled. Since the pre-barrier verifier disables the nursery, we end up baking in a tenured allocation in a way we can't really test for, since codegen happens off main thread.

Until bug 969012 is ready, we'd best just bake in nursery allocations when that is possible and accept a slower fallback path at runtime in this crazy edge case.

Comment 7

[Terrence Cole [:terrence]]

Attachment: do_not_accidentally_tenure-v0.diff

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8381580 [details] [diff] [review]
do_not_accidentally_tenure-v0.diff

Review of attachment 8381580 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch and always nice to get rid of dead code :)

Comment 9

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca08ca9fd157

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/ca08ca9fd157