479340 - js_TraceLocalRoots() incorrectly ASSERTS that (v != JSVAL_NULL)

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Mike Moening]

In threaded build with GC_ZEAL level set at 2 and JIT disabled.

Calling JS_InitStandardClasses() on a global object causes this assert to fire.

The problem is that js_InitExceptionClasses() pushes an array of 3 zero values on the cx.localRootStack using:

JSAutoTempValueRooter tvr(cx, JS_ARRAY_LENGTH(roots), roots);

After this call localRootStack has a count of 3 and looks like this:

(*cx).localRootStack	0x0012e228 {scopeMark=0 rootCount=3 topChunk=0x0012e23c ...}	JSLocalRootStack *
	scopeMark	0	unsigned long
	rootCount	3	unsigned long
	topChunk	0x0012e23c {roots=0x0012e23c down=0x33840000 }	JSLocalRootChunk *
	roots	0x0012e23c	int [256]
	[0]	0	int
	[1]	0	int
	[2]	0	int
	[3]	895291584	int

This is all fine and good.
Next js_InitExceptionClasses() calls:

       /* Make the prototype for the current constructor name. */
        proto = js_NewObject(cx, &js_ErrorClass,
                             (i != JSEXN_ERR) ? error_proto : obj_proto,
                             obj, 0);

This does NOT fire GC because it pulls a thing off of the freeLists array and doens't do any real allocation.
It simply pushes the thing to the root stack by calling js_PushLocalRoot()
Which increases the rootCount to 4.
Then it fills the zero position of the rootstack with the proto object it just created:

roots[0] = OBJECT_TO_JSVAL(proto);

Now top 4 cx roots look like:
roots	0x0012e23c	int [256]
   [0]	895291616	int
   [1]	0	int
   [2]	0	int
   [3]	895291616	int

We are still good at this point:

However; now js_InitExceptionClasses() calls:

        /* Make a constructor function for the current name. */
        protoKey = GetExceptionProtoKey(i);
        atom = cx->runtime->atomState.classAtoms[protoKey];
        fun = js_DefineFunction(cx, obj, atom, Exception, 3, 0);

which in turn calls js_NewGCThing()
js_NewCGThing doesn't find any non-null objects on the freeLists->array
so it has to allocate.

doGC is set to true because rt->gcZeal is 2 which fires js_GC(cx, GC_LAST_DITCH);
which calls js_TraceLocalRoots()
which then iterates the cx.localRootStack

and ASSERTS and dies when it sees root[2] is set to 0.
Abort and die!

Here is the call stack
------------------
js_TraceLocalRoots(JSTracer * trc=0x0012e06c, JSLocalRootStack * lrs=0x0012e228)  Line 949	C++
js_TraceContext(JSTracer * trc=0x0012e06c, JSContext * acx=0x33a76598)  Line 3045 + 0x13 bytes	C++
js_TraceRuntime(JSTracer * trc=0x0012e06c, int allAtoms=1)  Line 3113 + 0xd bytes	C++
js_GC(JSContext * cx=0x33a76598, JSGCInvocationKind gckind=GC_LAST_DITCH)  Line 3521 + 0xd bytes	C++
js_NewGCThing(JSContext * cx=0x33a76598, unsigned int flags=0, unsigned int nbytes=56)  Line 1855 + 0xb bytes	C++
js_NewObjectWithGivenProto(JSContext * cx=0x33a76598, JSClass * clasp=0x102c0fa0, JSObject * proto=0x355d0000, JSObject * parent=0x355d1020, unsigned int objectSize=52)  Line 2846 + 0xf bytes	C++
js_NewObject(JSContext * cx=0x33a76598, JSClass * clasp=0x102c0fa0, JSObject * proto=0x355d0000, JSObject * parent=0x355d1020, unsigned int objectSize=0)  Line 2808 + 0x19 bytes	C++
js_NewFunction(JSContext * cx=0x33a76598, JSObject * funobj=0x00000000, int (JSContext *, JSObject *, unsigned int, int *, int *)* native=0x10124c90, unsigned int nargs=3, unsigned int flags=0, JSObject * parent=0x355d1020, JSAtom * atom=0x355d30e4)  Line 2090 + 0x16 bytes	C++
js_DefineFunction(JSContext * cx=0x33a76598, JSObject * obj=0x355d1020, JSAtom * atom=0x355d30e4, int (JSContext *, JSObject *, unsigned int, int *, int *)* native=0x10124c90, unsigned int nargs=3, unsigned int attrs=0)  Line 2156 + 0x1f bytes	C++
js_InitExceptionClasses(JSContext * cx=0x33a76598, JSObject * obj=0x355d1020)  Line 1057 + 0x1a bytes	C++
JS_InitStandardClasses(JSContext * cx=0x33a76598, JSObject * obj=0x355d1020)  Line 1386 + 0x6d bytes	C++

Is this assert really valid?

JS_ASSERT(JSVAL_IS_GCTHING(v) && v != JSVAL_NULL);

I'm trying to test multi-threading with JIT but can't seem to get that far!!

Possible solution is to NOT push 3 zero roots onto the stack but instead push as needed to prevent the assert from firing in the first place.
This could be conditional code when GC_ZEAL is enabled so as not to kill performance in the normal case.


Mike M.

Comment 1

[Mike Moening]

This bug is not valid.
JS corruption was occuring because of me using non-public class

JSAutoTempValueRooter() inside one of my property getters trying to solve bug# 397177 (vp parameter to [gs]etPropertyOps is not rooted)

So I created my own pain.

I still have no idea WHY bug it causes a great deal of corruption using this class.  cx gets all messed up after JSAutoTempValueRooter() gets used on the JS side.

Sure would be nice if bug# 397177 was fixed... 

:-)