Bugs, bugs bugs:

- js_DecompileValueGenerator had rusted due to bytecode/source-note changes,
  or maybe parts of it never worked right.  Anyway, it now does not induce a
  crashing underflow in the decompiler.  As part of this fix, it now takes a
  checkStack flag telling whether to look for the jsval v argument on the JS
  stack.  The calls from ImportProperty, js_SetProperty, and js_DeleteProperty
  pass in v a jsval for the property id, which should not be sought after on
  the stack (it might happen to be there due to o['p'] = 2, but we want to
  decompile o["p"], not "p").
- js_DecompileValueGenerator would load a generating pc even if the value v
  did not match the pc's corresponding stack item!  Oops.  This lead to less
  than idea diagnostics.
- js_DecompileValueGenerator was also not mapping JSOP_TRAP to the real op at
  a sufficiently early and univeral point in its control flow.
- Fix PopOff to assert and check for stack underflow in the decompiler, and
  beef up PushOff too (it asserted, but did not check).
- js_ReportIsNotFunction now avoids JS_InternString by indexing directly into
  cx->runtime->atomState.typeAtoms with the result of JS_TypeOfValue.
- Removed unnecessary local GC root reserved by non-zero trailing member of
  obj_eval's JSFunctionSpec initializer.

js/src/jsfun.c

@@ -1117,7 +1117,7 @@ fun_hasInstance(JSContext *cx, JSObject *obj, jsval v, JSBool *bp)
      * Throw a runtime error if instanceof is called on a function
      * that has a non-Object as its .prototype value.
      */
-    str = js_DecompileValueGenerator(cx, OBJECT_TO_JSVAL(obj), NULL);
+    str = js_DecompileValueGenerator(cx, JS_TRUE, OBJECT_TO_JSVAL(obj), NULL);
     if (str) {
 	JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_PROTOTYPE,
 			     JS_GetStringBytes(str));
@@ -1748,7 +1748,7 @@ js_ValueToFunction(JSContext *cx, jsval *vp, JSBool constructing)
 void
 js_ReportIsNotFunction(JSContext *cx, jsval *vp, JSBool constructing)
 {
-    const char *typename;
+    JSType type;
     JSString *fallback;
     JSStackFrame *fp;
     JSString *str;
@@ -1759,16 +1759,16 @@ js_ReportIsNotFunction(JSContext *cx, jsval *vp, JSBool constructing)
      * called as the default case inside js_DecompileValueGenerator (and
      * so recursing back to here).
      */
-    typename = JS_GetTypeName(cx, JS_TypeOfValue(cx, *vp));
-    fallback = JS_InternString(cx, typename);
+    type = JS_TypeOfValue(cx, *vp);
+    fallback = ATOM_TO_STRING(cx->runtime->atomState.typeAtoms[type]);
     fp = cx->fp;
     if (fp) {
         jsval *sp = fp->sp;
         fp->sp = vp;
-        str = js_DecompileValueGenerator(cx, *vp, fallback);
+        str = js_DecompileValueGenerator(cx, JS_TRUE, *vp, fallback);
         fp->sp = sp;
     } else {
-        str = js_DecompileValueGenerator(cx, *vp, fallback);
+        str = js_DecompileValueGenerator(cx, JS_TRUE, *vp, fallback);
     }
     if (str) {
 	JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,

js/src/jsinterp.c

@@ -868,7 +868,8 @@ ImportProperty(JSContext *cx, JSObject *obj, jsid id)
 	if (!OBJ_LOOKUP_PROPERTY(cx, obj, id, &obj2, &prop))
 	    return JS_FALSE;
 	if (!prop) {
-	    str = js_DecompileValueGenerator(cx, js_IdToValue(id), NULL);
+	    str = js_DecompileValueGenerator(cx, JS_FALSE, js_IdToValue(id),
+					     NULL);
 	    if (str)
 		js_ReportIsNotDefined(cx, JS_GetStringBytes(str));
 	    return JS_FALSE;
@@ -878,7 +879,8 @@ ImportProperty(JSContext *cx, JSObject *obj, jsid id)
 	if (!ok)
 	    return JS_FALSE;
 	if (!(attrs & JSPROP_EXPORTED)) {
-	    str = js_DecompileValueGenerator(cx, js_IdToValue(id), NULL);
+	    str = js_DecompileValueGenerator(cx, JS_FALSE, js_IdToValue(id),
+					     NULL);
 	    if (str) {
 		JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
 				     JSMSG_NOT_EXPORTED,
@@ -1082,7 +1084,8 @@ js_Interpret(JSContext *cx, jsval *result)
 	    if (nuses) {
 		fp->sp = sp - nuses;
 		for (n = 0; n < nuses; n++) {
-		    str = js_DecompileValueGenerator(cx, *fp->sp, NULL);
+		    str = js_DecompileValueGenerator(cx, JS_TRUE, *fp->sp,
+						     NULL);
 		    if (str != NULL) {
 			fprintf(tracefp, "%s %s",
 				(n == 0) ? "  inputs:" : ",",
@@ -2747,7 +2750,7 @@ js_Interpret(JSContext *cx, jsval *result)
 	  case JSOP_INSTANCEOF:
 	    rval = POP();
 	    if (JSVAL_IS_PRIMITIVE(rval)) {
-		str = js_DecompileValueGenerator(cx, rval, NULL);
+		str = js_DecompileValueGenerator(cx, JS_TRUE, rval, NULL);
 		if (str) {
 		    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
 					 JSMSG_BAD_INSTANCEOF_RHS,
@@ -2820,7 +2823,8 @@ js_Interpret(JSContext *cx, jsval *result)
 	    if (ndefs) {
 		fp->sp = sp - ndefs;
 		for (n = 0; n < ndefs; n++) {
-		    str = js_DecompileValueGenerator(cx, *fp->sp, NULL);
+		    str = js_DecompileValueGenerator(cx, JS_TRUE, *fp->sp,
+						     NULL);
 		    if (str) {
 			fprintf(tracefp, "%s %s",
 				(n == 0) ? "  output:" : ",",

js/src/jsobj.c

@@ -804,19 +804,19 @@ obj_propertyIsEnumerable(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
 
 static JSFunctionSpec object_methods[] = {
 #if JS_HAS_TOSOURCE
-    {js_toSource_str,   js_obj_toSource,        0, 0, OBJ_TOSTRING_EXTRA},
+    {js_toSource_str,         js_obj_toSource,        0, 0, OBJ_TOSTRING_EXTRA},
 #endif
-    {js_toString_str,	js_obj_toString,	0, 0, OBJ_TOSTRING_EXTRA},
-    {js_valueOf_str,	obj_valueOf,		0},
-    {js_eval_str,	obj_eval,		1, 0, 1},
+    {js_toString_str,         js_obj_toString,        0, 0, OBJ_TOSTRING_EXTRA},
+    {js_valueOf_str,          obj_valueOf,            0},
+    {js_eval_str,             obj_eval,               1},
 #if JS_HAS_OBJ_WATCHPOINT
-    {"watch",           obj_watch,              2},
-    {"unwatch",         obj_unwatch,            1},
+    {"watch",                 obj_watch,              2},
+    {"unwatch",               obj_unwatch,            1},
 #endif
 #if JS_HAS_NEW_OBJ_METHODS
-    {"hasOwnProperty",  obj_hasOwnProperty,     1},
-    {"isPrototypeOf",   obj_isPrototypeOf,      1},
-    {"propertyIsEnumerable", obj_propertyIsEnumerable, 1},
+    {"hasOwnProperty",        obj_hasOwnProperty,     1},
+    {"isPrototypeOf",         obj_isPrototypeOf,      1},
+    {"propertyIsEnumerable",  obj_propertyIsEnumerable, 1},
 #endif
     {0}
 };
@@ -1873,7 +1873,7 @@ read_only:
 	JS_UNLOCK_OBJ(cx, obj);
 	if (JSVERSION_IS_ECMA(cx->version))
 	    return JS_TRUE;
-	str = js_DecompileValueGenerator(cx, js_IdToValue(id), NULL);
+	str = js_DecompileValueGenerator(cx, JS_FALSE, js_IdToValue(id), NULL);
 	if (str)
 	    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
 				 JSMSG_READ_ONLY, JS_GetStringBytes(str));
@@ -2017,7 +2017,7 @@ js_DeleteProperty(JSContext *cx, JSObject *obj, jsid id, jsval *rval)
 	    *rval = JSVAL_FALSE;
 	    return JS_TRUE;
 	}
-	str = js_DecompileValueGenerator(cx, js_IdToValue(id), NULL);
+	str = js_DecompileValueGenerator(cx, JS_FALSE, js_IdToValue(id), NULL);
 	if (str) {
 	    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
 				 JSMSG_PERMANENT, JS_GetStringBytes(str));
@@ -2144,7 +2144,7 @@ js_DefaultValue(JSContext *cx, JSObject *obj, JSType hint, jsval *vp)
 	    str = NULL;
 	}
 	*vp = OBJECT_TO_JSVAL(obj);
-	str = js_DecompileValueGenerator(cx, v, str);
+	str = js_DecompileValueGenerator(cx, JS_TRUE, v, str);
 	if (str) {
 	    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
 				 JSMSG_CANT_CONVERT_TO,
@@ -2477,7 +2477,7 @@ js_ValueToNonNullObject(JSContext *cx, jsval v)
     if (!js_ValueToObject(cx, v, &obj))
 	return NULL;
     if (!obj) {
-	str = js_DecompileValueGenerator(cx, v, NULL);
+	str = js_DecompileValueGenerator(cx, JS_TRUE, v, NULL);
 	if (str) {
 	    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
 				 JSMSG_NO_PROPERTIES, JS_GetStringBytes(str));

js/src/jsopcode.c

@@ -529,12 +529,26 @@ typedef struct SprintStack {
 static JSBool
 PushOff(SprintStack *ss, ptrdiff_t off, JSOp op)
 {
-    JS_ASSERT(JSOP_LIMIT <= 254);
+    uintN top;
+
+    JS_ASSERT(JSOP_LIMIT <= JSOP_GETPROP2);
     if (!SprintAlloc(&ss->sprinter, PAREN_SLOP))
 	return JS_FALSE;
-    JS_ASSERT(ss->top < ss->printer->script->depth);
-    ss->offsets[ss->top] = off;
-    ss->opcodes[ss->top++] = (op >= 128) ? op - 128 : op;
+
+    /* ss->top points to the next free slot; be paranoid about overflow. */
+    top = ss->top;
+    JS_ASSERT(top < ss->printer->script->depth);
+    if (top >= ss->printer->script->depth) {
+	JS_ReportOutOfMemory(ss->sprinter.context);
+	return JS_FALSE;
+    }
+
+    /* The opcodes stack must contain real bytecodes that index js_CodeSpec. */
+    ss->offsets[top] = off;
+    ss->opcodes[top] = (op == JSOP_GETPROP2) ? JSOP_GETPROP
+		     : (op == JSOP_GETELEM2) ? JSOP_GETELEM
+		     : (jsbytecode) op;
+    ss->top = ++top;
     ss->sprinter.offset += PAREN_SLOP;
     return JS_TRUE;
 }
@@ -546,9 +560,15 @@ PopOff(SprintStack *ss, JSOp op)
     JSCodeSpec *cs, *topcs;
     ptrdiff_t off;
 
-    top   = --ss->top;
-    cs    = &js_CodeSpec[op];
+    /* ss->top points to the next free slot; be paranoid about underflow. */
+    top = ss->top;
+    JS_ASSERT(top != 0);
+    if (top == 0)
+	return 0;
+
+    ss->top = --top;
     topcs = &js_CodeSpec[ss->opcodes[top]];
+    cs = &js_CodeSpec[op];
     if (topcs->prec != 0 && topcs->prec < cs->prec) {
 	ss->offsets[top] -= 2;
 	ss->sprinter.offset = ss->offsets[top];
@@ -2130,17 +2150,19 @@ js_DecompileFunction(JSPrinter *jp, JSFunction *fun, JSBool newlines)
 }
 
 JSString *
-js_DecompileValueGenerator(JSContext *cx, jsval v, JSString *fallback)
+js_DecompileValueGenerator(JSContext *cx, JSBool checkStack, jsval v,
+			   JSString *fallback)
 {
     JSStackFrame *fp;
     jsbytecode *pc, *begin, *end, *tmp;
     jsval *sp, *base, *limit;
     JSScript *script;
+    JSOp op;
     JSCodeSpec *cs;
     uint32 format, mode;
     intN depth;
     jssrcnote *sn;
-    uintN len, maxoplen;
+    uintN len, off;
     JSPrinter *jp;
     JSString *name;
 
@@ -2151,10 +2173,18 @@ js_DecompileValueGenerator(JSContext *cx, jsval v, JSString *fallback)
     /* Try to find sp's generating pc depth slots under it on the stack. */
     pc = fp->pc;
     limit = (jsval *) cx->stackPool.current->avail;
-    if (!pc &&
-	fp->argv &&
-	fp->down &&
-	(script = fp->down->script) != NULL) {
+    if (!pc && fp->argv && fp->down) {
+	/*
+	 * Current frame is native, look under it for a scripted call in which
+	 * a decompilable bytecode string that generated the value might exist.
+	 * But if we're told not to check the stack for v, give up.
+	 */
+	if (!checkStack)
+	    goto do_fallback;
+	script = fp->down->script;
+	if (!script)
+	    goto do_fallback;
+
 	/*
 	 * Native frame called by script: try to match v with actual argument.
 	 * If (fp->sp < fp->argv), normally an impossibility, then we are in
@@ -2169,32 +2199,50 @@ js_DecompileValueGenerator(JSContext *cx, jsval v, JSString *fallback)
 	    }
 	}
     } else {
-	/* Could be native frame called by native code, so check script. */
+	/*
+	 * At this point, pc may or may not be null.  I.e., we could be in a
+	 * script activation, or we could be in a native frame that was called
+	 * by another native function.  Check script.
+	 */
 	script = fp->script;
 	if (!script)
 	    goto do_fallback;
 
-	/* OK, interpreted frame.  Try the operand at sp, or one above it. */
-	sp = fp->sp;
-	if (sp[0] != v && sp + 1 < limit && sp[1] == v)
-	    sp++;
+	/*
+	 * OK, we're in an interpreted frame.  The interpreter calls us just
+	 * after popping one or two operands for a given bytecode at fp->pc.
+	 * So try the operand at sp, or one above it.
+	 */
+	if (checkStack) {
+	    sp = fp->sp;
+	    if (sp[0] != v && sp + 1 < limit && sp[1] == v)
+		sp++;
 
-	/* Try to find an operand-generating pc just above fp's variables. */
-	depth = (intN)script->depth;
-	base = fp->vars
-	       ? fp->vars + fp->nvars
-	       : (jsval *) cx->stackPool.current->base;
-	if (JS_UPTRDIFF(sp - depth, base) < JS_UPTRDIFF(limit, base))
-	    pc = (jsbytecode *) sp[-depth];
+	    if (sp[0] == v) {
+		/* Try to find an operand-generating pc just above fp's vars. */
+		depth = (intN)script->depth;
+		base = fp->vars
+		       ? fp->vars + fp->nvars
+		       : (jsval *) cx->stackPool.current->base;
+		if (JS_UPTRDIFF(sp - depth, base) < JS_UPTRDIFF(limit, base))
+		    pc = (jsbytecode *) sp[-depth];
+	    }
+	}
+
+	/*
+	 * If fp->pc was null, and either we had no luck checking the stack,
+	 * or our caller synthesized v himself and does not want us to check
+	 * the stack, then we fall back.
+	 */
+	if (!pc)
+	    goto do_fallback;
     }
 
-    /* Be paranoid about loading an invalid pc from sp[-depth]. */
-    if (!pc)
-	goto do_fallback;
-
     /*
+     * Be paranoid about loading an invalid pc from sp[-depth].
+     *
      * Using an object for which js_DefaultValue fails as part of an expression
-     * blows this assert.  Disabled for now.
+     * blows this assert.  Disabled for now.  XXXbe true w/ JSINVOKE_INTERNAL?
      * JS_ASSERT(JS_UPTRDIFF(pc, script->code) < (jsuword)script->length);
      */
     if (JS_UPTRDIFF(pc, script->code) >= (jsuword)script->length) {
@@ -2202,7 +2250,10 @@ js_DecompileValueGenerator(JSContext *cx, jsval v, JSString *fallback)
 	if (!pc)
 	    goto do_fallback;
     }
-    cs = &js_CodeSpec[*pc];
+    op = *pc;
+    if (op == JSOP_TRAP)
+	op = JS_GetTrapOpcode(cx, script, pc);
+    cs = &js_CodeSpec[op];
     format = cs->format;
     mode = (format & JOF_MODEMASK);
 
@@ -2216,31 +2267,35 @@ js_DecompileValueGenerator(JSContext *cx, jsval v, JSString *fallback)
 	begin = pc - js_GetSrcNoteOffset(sn, 0);
     }
     end = pc + cs->length;
-    len =PTRDIFF(end, begin, jsbytecode);
+    len = PTRDIFF(end, begin, jsbytecode);
 
     if (format & (JOF_SET | JOF_DEL | JOF_INCDEC | JOF_IMPORT)) {
-	/* These formats require bytecode source extension. */
-	maxoplen = js_CodeSpec[JSOP_GETPROP].length;
-	tmp = JS_malloc(cx, (len + maxoplen) * sizeof(jsbytecode));
+	tmp = JS_malloc(cx, len * sizeof(jsbytecode));
 	if (!tmp)
 	    return NULL;
 	memcpy(tmp, begin, len * sizeof(jsbytecode));
 	if (mode == JOF_NAME) {
 	    tmp[0] = JSOP_NAME;
 	} else {
-	    if (begin[0] == JSOP_TRAP)
-		tmp[0] = JS_GetTrapOpcode(cx, script, begin);
+	    /*
+	     * We must replace the faulting pc's bytecode with a corresponding
+	     * JSOP_GET* code.  For JSOP_SET{PROP,ELEM}, we must use the "2nd"
+	     * form of JSOP_GET{PROP,ELEM}, to throw away the assignment op's
+	     * right-hand operand and decompile it as if it were a GET of its
+	     * left-hand operand.
+	     */
+	    off = len - cs->length;
+	    JS_ASSERT(off == PTRDIFF(pc, begin, jsbytecode));
 	    if (mode == JOF_PROP) {
-		tmp[len++] = (format & JOF_SET) ? JSOP_GETPROP2 : JSOP_GETPROP;
-		tmp[len++] = pc[1];
-		tmp[len++] = pc[2];
+		tmp[off] = (format & JOF_SET) ? JSOP_GETPROP2 : JSOP_GETPROP;
 	    } else {
-		tmp[len++] = (format & JOF_SET) ? JSOP_GETELEM2 : JSOP_GETELEM;
+		JS_ASSERT(mode == JOF_ELEM);
+		tmp[off] = (format & JOF_SET) ? JSOP_GETELEM2 : JSOP_GETELEM;
 	    }
 	}
 	begin = tmp;
     } else {
-	/* No need to extend script bytecode. */
+	/* No need to revise script bytecode. */
 	tmp = NULL;
     }
 

js/src/jsopcode.h

@@ -181,10 +181,12 @@ js_DecompileFunction(JSPrinter *jp, JSFunction *fun, JSBool newlines);
 /*
  * Find the source expression that resulted in v, and return a new string
  * containing it.  Fall back on v's string conversion if we can't find the
- * bytecode that generated and pushed v on the operand stack.
+ * bytecode that generated and pushed v on the operand stack.  Don't look
+ * for v on the stack if checkStack is false.
  */
 extern JSString *
-js_DecompileValueGenerator(JSContext *cx, jsval v, JSString *fallback);
+js_DecompileValueGenerator(JSContext *cx, JSBool checkStack, jsval v,
+			   JSString *fallback);
 
 JS_END_EXTERN_C