gczeal(9, 1) for (var a = 0; a < 1; a++) { newGlobal({ sameZoneAs: {} }) } crashes js debug shell (tested with a threadsafe deterministic 32-bit debug build) on m-i changeset 4dceda951fba without any CLI arguments at js::gc::GetGCThingRuntime and crashes js opt shell at js::gc::Cell::tenuredZone My configure options are: 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>

This is an exact rooting failure: seems ggc is just as effective as the root-analysis at finding unrooted stack entries. I opened bug 913681 to track.

Try run up at: https://tbpl.mozilla.org/?tree=Try&rev=f3274770ee7c Preemptively flagging for review: feel free to ignore if the try run is burning. I don't really expect it to, however, as this builds locally and is rather simple. This patch stores the zone in CompartmentOptions rather than the same-zone-as object. This lets us continue to ignore rooting CompartmentOptions and removes the heapptr ambiguity where we were storing the object in the heap as well. I also updated the types of things so that the compiler can give us more help: 1) Convert uintptr_t -> void* so that the compiler will not incorrectly interleave reads/writes through the zone pointer here and the original zone pointer. 2) Make ZoneSpecifier the enum type instead of typedef uintptr_t so that setZone can only take valid zone entries and not random ints, bools, etc. 3) Make all fields private and add accessors to allow injection of several correctness assertions and to move setZone/setSameZoneAs out-of-line so that we can get and cast the Zone*. Unfortunately, this continues to overlay the memory of the enum and pointer, but uses a union to make the intended usage clearer. Setting the zone immediately and dropping the union entirely will require setZone to take a JSContext/JSRuntime. This should be doable, but there are enough uses of setZone that I think this will be easier as a follow-up.

Comment on attachment 802451 [details] [diff] [review] fuzz_912813-v0.diff Review of attachment 802451 [details] [diff] [review]: ----------------------------------------------------------------- r=bholley with comments addressed. ::: js/src/jsapi.cpp @@ +2945,5 @@ > JSCompartment *compartment = NewCompartment(cx, zone, principals, options); > if (!compartment) > return NULL; > > + if (options.zoneSpecifier() == JS::SystemZone) { This line threw me for a loop. Please add a comment explaining that this handles the lazy creation of the system zone and alter the conditional to be: if (!rt->systemZone && options.zoneSpecifier() == JS::SystemZone) ::: js/src/jsapi.h @@ +2662,5 @@ > + ZoneSpecifier spec; > + void *pointer; // js::Zone* is not exposed in the API. > + } zone_; > + JSVersion version_; > + bool invisibleToDebugger_; I'm not wild about switching everything to getters, because I'm about to dump a metric ton of jit options into this struct, and I don't want to deal with the overhead. Can we just make zone_ an accessor prop and make the simple stuff public?

https://tbpl.mozilla.org/?tree=Try&rev=f3274770ee7c https://hg.mozilla.org/integration/mozilla-inbound/rev/496b55259bbb