Bug 462103 - TM: We don't trace some variants of string + other type (gal+brendan red-headed stepchild).

js/src/builtins.tbl

@@ -91,6 +91,7 @@ BUILTIN3(extern, BOOL,      js_HasNamedProperty, CONTEXT, OBJECT, STRING,
 BUILTIN3(extern, JSVAL,     js_CallGetter, CONTEXT, OBJECT, SCOPEPROP,          0, 0)
 BUILTIN2(extern, STRING,    js_TypeOfObject, CONTEXT, OBJECT,                   1, 1)
 BUILTIN2(extern, STRING,    js_TypeOfBoolean, CONTEXT, INT32,                   1, 1)
-BUILTIN2(extern, DOUBLE,    js_BooleanToNumber, CONTEXT, INT32,                 1, 1)
+BUILTIN2(extern, DOUBLE,    js_BooleanOrUndefinedToNumber, CONTEXT, INT32,      1, 1)
+BUILTIN2(extern, STRING,    js_BooleanOrUndefinedToString, CONTEXT, INT32,      1, 1)
 BUILTIN2(extern, STRING,    js_ObjectToString, CONTEXT, OBJECT,                 0, 0)
 BUILTIN1(extern, OBJECT,    js_Arguments, CONTEXT,                              0, 0)

js/src/jsatom.cpp

@@ -68,17 +68,23 @@ js_AtomToPrintableString(JSContext *cx, JSAtom *atom)
 #undef JS_PROTO
 
 /*
- * Names for common atoms defined in JSAtomState starting from
+ * String constants for common atoms defined in JSAtomState starting from
  * JSAtomState.emptyAtom until JSAtomState.lazy.
  *
  * The elements of the array after the first empty string define strings
- * corresponding to JSType enumerators from jspubtd.h and to two boolean
+ * corresponding to the two boolean literals, false and true, followed by the
- * literals, false and true. The following assert insists that JSType defines
+ * JSType enumerators from jspubtd.h starting with "undefined" for JSTYPE_VOID
- * exactly 8 types.
+ * (which is pseudo-boolean 2) and continuing as initialized below. The static
+ * asserts check these relations.
  */
 JS_STATIC_ASSERT(JSTYPE_LIMIT == 8);
+JS_STATIC_ASSERT(JSVAL_TO_BOOLEAN(JSVAL_VOID) == 2);
+JS_STATIC_ASSERT(JSTYPE_VOID == 0);
+
 const char *const js_common_atom_names[] = {
     "",                         /* emptyAtom                    */
+    js_false_str,               /* booleanAtoms[0]              */
+    js_true_str,                /* booleanAtoms[1]              */
     js_undefined_str,           /* typeAtoms[JSTYPE_VOID]       */
     js_object_str,              /* typeAtoms[JSTYPE_OBJECT]     */
     js_function_str,            /* typeAtoms[JSTYPE_FUNCTION]   */
@@ -87,8 +93,6 @@ const char *const js_common_atom_names[] = {
     "boolean",                  /* typeAtoms[JSTYPE_BOOLEAN]    */
     js_null_str,                /* typeAtoms[JSTYPE_NULL]       */
     "xml",                      /* typeAtoms[JSTYPE_XML]        */
-    js_false_str,               /* booleanAtoms[0]              */
-    js_true_str,                /* booleanAtoms[1]              */
     js_null_str,                /* nullAtom                     */
 
 #define JS_PROTO(name,code,init) js_##name##_str,

js/src/jsatom.h

@@ -162,9 +162,12 @@ struct JSAtomState {
     /* The rt->emptyString atom, see jsstr.c's js_InitRuntimeStringState. */
     JSAtom              *emptyAtom;
 
-    /* Type names and value literals. */
+    /*
-    JSAtom              *typeAtoms[JSTYPE_LIMIT];
+     * Literal value and type names.
+     * NB: booleanAtoms must come right before typeAtoms!
+     */
     JSAtom              *booleanAtoms[2];
+    JSAtom              *typeAtoms[JSTYPE_LIMIT];
     JSAtom              *nullAtom;
 
     /* Standard class constructor or prototype names. */
@@ -278,16 +281,15 @@ extern const char *const js_common_atom_names[];
 
 /*
  * Macros to access C strings for JSType and boolean literals together with
- * checks that type names and booleans starts from index 1 and 1+JSTYPE_LIMIT
+ * checks that boolean names start from index 1 and type names from 1+2.
- * correspondingly.
  */
-#define JS_TYPE_STR(type)    (js_common_atom_names[1 + (type)])
+#define JS_BOOLEAN_STR(type) (js_common_atom_names[1 + (type)])
-#define JS_BOOLEAN_STR(type) (js_common_atom_names[1 + JSTYPE_LIMIT + (type)])
+#define JS_TYPE_STR(type)    (js_common_atom_names[1 + 2 + (type)])
 
 JS_STATIC_ASSERT(1 * sizeof(JSAtom *) ==
-                 offsetof(JSAtomState, typeAtoms) - ATOM_OFFSET_START);
-JS_STATIC_ASSERT((1 + JSTYPE_LIMIT) * sizeof(JSAtom *) ==
                  offsetof(JSAtomState, booleanAtoms) - ATOM_OFFSET_START);
+JS_STATIC_ASSERT((1 + 2) * sizeof(JSAtom *) ==
+                 offsetof(JSAtomState, typeAtoms) - ATOM_OFFSET_START);
 
 /* Well-known predefined C strings. */
 #define JS_PROTO(name,code,init) extern const char js_##name##_str[];

js/src/jsbuiltins.cpp

@@ -432,13 +432,20 @@ js_TypeOfBoolean(JSContext* cx, int32 unboxed)
 }
 
 jsdouble FASTCALL
-js_BooleanToNumber(JSContext* cx, int32 unboxed)
+js_BooleanOrUndefinedToNumber(JSContext* cx, int32 unboxed)
 {
     if (unboxed == JSVAL_TO_BOOLEAN(JSVAL_VOID))
         return js_NaN;
     return unboxed;
 }
 
+JSString* FASTCALL
+js_BooleanOrUndefinedToString(JSContext *cx, int32 unboxed)
+{
+    JS_ASSERT(uint32(unboxed) <= 2);
+    return ATOM_TO_STRING(cx->runtime->atomState.booleanAtoms[unboxed]);
+}
+
 JSString* FASTCALL
 js_ObjectToString(JSContext* cx, JSObject* obj)
 {

js/src/jsbuiltins.h

@@ -330,6 +330,9 @@ JS_DECLARE_CALLINFO(js_Array_1str)
 JS_DECLARE_CALLINFO(js_Array_2obj)
 JS_DECLARE_CALLINFO(js_Array_3num)
 
+/* Defined in jsbool.cpp */
+JS_DECLARE_CALLINFO(js_BooleanToString)
+
 /* Defined in jsdate.cpp */
 JS_DECLARE_CALLINFO(js_FastNewDate)
 

js/src/jstracer.cpp

@@ -1939,7 +1939,7 @@ TraceRecorder::checkType(jsval& v, uint8 t, jsval*& stage_val, LIns*& stage_ins,
         if (!isNumber(v))
             return false; /* not a number? type mismatch */
         LIns* i = get(&v);
-        /* We sink i2f conversions into the side exit, but at the loop edge we have to make
+        /* We sink i2f conversions into the side exit, but at the loop edge we have to
            sure we promote back to double if at loop entry we want a double. */
         if (isPromoteInt(i)) {
             stage_val = &v;
@@ -3864,6 +3864,30 @@ LIns* TraceRecorder::makeNumberInt32(LIns* f)
     return x;
 }
 
+LIns*
+TraceRecorder::stringify(jsval& v, LIns* v_ins)
+{
+    if (JSVAL_IS_STRING(v))
+        return v_ins;
+
+    LIns* args[] = { v_ins, cx_ins };
+    const CallInfo* ci;
+    if (JSVAL_IS_NUMBER(v)) {
+        ci = &js_NumberToString_ci;
+    } else if (JSVAL_TAG(v) == JSVAL_BOOLEAN) {
+        ci = &js_BooleanOrUndefinedToString_ci;
+    } else {
+        JS_ASSERT(JSVAL_IS_OBJECT(v));
+        // This is unsafe until we are able to abort if we re-enter the interpreter.
+        // FIXME: 456511
+        // ci = &js_ObjectToString_ci;
+        return NULL;
+    }
+    v_ins = lir->insCall(ci, args);
+    guard(false, lir->ins_eq0(v_ins), OOM_EXIT);
+    return v_ins;
+}
+
 bool
 TraceRecorder::ifop()
 {
@@ -4129,7 +4153,7 @@ TraceRecorder::cmp(LOpcode op, int flags)
              * branched.  Failing that, I want to be able to ins_choose on quads
              * without cmov.  Failing that, eat flaming builtin!
              */
-            l_ins = lir->insCall(&js_BooleanToNumber_ci, args);
+            l_ins = lir->insCall(&js_BooleanOrUndefinedToNumber_ci, args);
         } else if (!isNumber(l)) {
             ABORT_TRACE("unsupported LHS type for cmp vs number");
         }
@@ -4145,7 +4169,7 @@ TraceRecorder::cmp(LOpcode op, int flags)
             r_ins = lir->insCall(&js_StringToNumber_ci, args);
         } else if (JSVAL_TAG(r) == JSVAL_BOOLEAN) {
             // See above for the sob story.
-            r_ins = lir->insCall(&js_BooleanToNumber_ci, args);
+            r_ins = lir->insCall(&js_BooleanOrUndefinedToNumber_ci, args);
         } else if (!isNumber(r)) {
             ABORT_TRACE("unsupported RHS type for cmp vs number");
         }
@@ -4268,12 +4292,12 @@ TraceRecorder::binary(LOpcode op)
     }
     if (JSVAL_TAG(l) == JSVAL_BOOLEAN) {
         LIns* args[] = { a, cx_ins };
-        a = lir->insCall(&js_BooleanToNumber_ci, args);
+        a = lir->insCall(&js_BooleanOrUndefinedToNumber_ci, args);
         leftNumber = true;
     }
     if (JSVAL_TAG(r) == JSVAL_BOOLEAN) {
         LIns* args[] = { b, cx_ins };
-        b = lir->insCall(&js_BooleanToNumber_ci, args);
+        b = lir->insCall(&js_BooleanOrUndefinedToNumber_ci, args);
         rightNumber = true;
     }
     if (leftNumber && rightNumber) {
@@ -5041,21 +5065,10 @@ TraceRecorder::record_JSOP_ADD()
 {
     jsval& r = stackval(-1);
     jsval& l = stackval(-2);
-    if (JSVAL_IS_STRING(l)) {
+    if (JSVAL_IS_STRING(l) || JSVAL_IS_STRING(r)) {
-        LIns* args[] = { NULL, get(&l), cx_ins };
+        LIns* args[] = { stringify(r, get(&r)), stringify(l, get(&l)), cx_ins };
-        if (JSVAL_IS_STRING(r)) {
+        if (!args[0] || !args[1])
-            args[0] = get(&r);
+            ABORT_TRACE("can't stringify objects");
-        } else {
-            LIns* args2[] = { get(&r), cx_ins };
-            if (JSVAL_IS_NUMBER(r)) {
-                args[0] = lir->insCall(&js_NumberToString_ci, args2);
-            } else if (JSVAL_IS_OBJECT(r)) {
-                args[0] = lir->insCall(&js_ObjectToString_ci, args2);
-            } else {
-                ABORT_TRACE("untraceable right operand to string-JSOP_ADD");
-            }
-            guard(false, lir->ins_eq0(args[0]), OOM_EXIT);
-        }
         LIns* concat = lir->insCall(&js_ConcatStrings_ci, args);
         guard(false, lir->ins_eq0(concat), OOM_EXIT);
         set(&l, concat);

js/src/jstracer.h

@@ -287,6 +287,7 @@ class TraceRecorder : public GCObject {
 
     nanojit::LIns* f2i(nanojit::LIns* f);
     nanojit::LIns* makeNumberInt32(nanojit::LIns* f);
+    nanojit::LIns* stringify(jsval& v, nanojit::LIns* v_ins);
     
     bool ifop();
     bool switchop();

js/src/trace-test.js

@@ -2214,6 +2214,28 @@ function testAddUndefined() {
 }
 test(testAddUndefined);
 
+function testStringify() {
+    var t = true, f = false, u = undefined, n = 5, d = 5.5, s = "x";
+    var a = [];
+    for (var i = 0; i < 10; ++i) {
+	a[0] = "" + t;
+	a[1] = t + "";
+	a[2] = "" + f;
+	a[3] = f + "";
+	a[4] = "" + u;
+	a[5] = u + "";
+	a[6] = "" + n;
+	a[7] = n + "";
+	a[8] = "" + d;
+	a[9] = d + "";
+	a[10] = "" + s;
+	a[11] = s + "";
+    }
+    return a.join(",");
+}
+testStringify.expected = "true,true,false,false,undefined,undefined,5,5,5.5,5.5,x,x";
+test(testStringify);
+
 /* NOTE: Keep this test last, since it screws up all for...in loops after it. */
 function testGlobalProtoAccess() {
     return "ok";