Bug 356083: Incorrect decompilation for ({this setter: function () { } }), patch by Brian Crowder <crowder@fiverocks.com>, r=brendan

js/src/jsobj.c

@@ -832,14 +832,35 @@ js_obj_toSource(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
     val = argv + 2;
 
     for (i = 0, length = ida->length; i < length; i++) {
+        JSBool idIsLexicalIdentifier, needOldStyleGetterSetter;
+
         /* Get strings for id and value and GC-root them via argv. */
         id = ida->vector[i];
 
 #if JS_HAS_GETTER_SETTER
-
         ok = OBJ_LOOKUP_PROPERTY(cx, obj, id, &obj2, &prop);
         if (!ok)
             goto error;
+#endif
+
+        /*
+         * Convert id to a jsval and then to a string.  Decide early whether we
+         * prefer get/set or old getter/setter syntax.
+         */
+        atom = JSID_IS_ATOM(id) ? JSID_TO_ATOM(id) : NULL;
+        id = ID_TO_VALUE(id);
+        idstr = js_ValueToString(cx, id);
+        if (!idstr) {
+            ok = JS_FALSE;
+            OBJ_DROP_PROPERTY(cx, obj2, prop);
+            goto error;
+        }
+        *rval = STRING_TO_JSVAL(idstr);         /* local root */
+        idIsLexicalIdentifier = js_IsIdentifier(idstr);
+        needOldStyleGetterSetter = !idIsLexicalIdentifier;
+
+#if JS_HAS_GETTER_SETTER
+
         valcnt = 0;
         if (prop) {
             ok = OBJ_GET_ATTRIBUTES(cx, obj2, id, prop, &attrs);
@@ -855,8 +876,9 @@ js_obj_toSource(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
                     gsop[valcnt] =
                         ATOM_TO_STRING(cx->runtime->atomState.getterAtom);
 #else
-                    gsop[valcnt] =
+                    gsop[valcnt] = needOldStyleGetterSetter
-                        ATOM_TO_STRING(cx->runtime->atomState.getAtom);
+                        ? ATOM_TO_STRING(cx->runtime->atomState.getterAtom)
+                        : ATOM_TO_STRING(cx->runtime->atomState.getAtom);
 #endif
                     valcnt++;
                 }
@@ -866,8 +888,9 @@ js_obj_toSource(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
                     gsop[valcnt] =
                         ATOM_TO_STRING(cx->runtime->atomState.setterAtom);
 #else
-                    gsop[valcnt] =
+                    gsop[valcnt] = needOldStyleGetterSetter
-                        ATOM_TO_STRING(cx->runtime->atomState.setAtom);
+                        ? ATOM_TO_STRING(cx->runtime->atomState.setterAtom)
+                        : ATOM_TO_STRING(cx->runtime->atomState.setAtom);
 #endif
                     valcnt++;
                 }
@@ -881,6 +904,12 @@ js_obj_toSource(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
 
 #else  /* !JS_HAS_GETTER_SETTER */
 
+        /*
+         * We simplify the source code at the price of minor dead code bloat in
+         * the ECMA version (for testing only, see jsconfig.h).  The null
+         * default values in gsop[j] suffice to disable non-ECMA getter and
+         * setter code.
+         */
         valcnt = 1;
         gsop[0] = NULL;
         ok = OBJ_GET_PROPERTY(cx, obj, id, &val[0]);
@@ -890,22 +919,12 @@ js_obj_toSource(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
         if (!ok)
             goto error;
 
-        /* Convert id to a jsval and then to a string. */
-        atom = JSID_IS_ATOM(id) ? JSID_TO_ATOM(id) : NULL;
-        id = ID_TO_VALUE(id);
-        idstr = js_ValueToString(cx, id);
-        if (!idstr) {
-            ok = JS_FALSE;
-            goto error;
-        }
-        *rval = STRING_TO_JSVAL(idstr);         /* local root */
-
         /*
          * If id is a string that's not an identifier, then it needs to be
          * quoted.  Also, negative integer ids must be quoted.
          */
         if (atom
-            ? !js_IsIdentifier(idstr)
+            ? !idIsLexicalIdentifier
             : (JSID_IS_OBJECT(id) || JSID_TO_INT(id) < 0)) {
             idstr = js_QuoteString(cx, idstr, (jschar)'\'');
             if (!idstr) {
@@ -933,7 +952,8 @@ js_obj_toSource(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
              * Remove '(function ' from the beginning of valstr and ')' from the
              * end so that we can put "get" in front of the function definition.
              */
-            if (gsop[j] && VALUE_IS_FUNCTION(cx, val[j])) {
+            if (gsop[j] && VALUE_IS_FUNCTION(cx, val[j]) &&
+                !needOldStyleGetterSetter) {
                 size_t n = strlen(js_function_str) + 2;
                 JS_ASSERT(vlength > n);
                 vchars += n;
@@ -1015,15 +1035,30 @@ js_obj_toSource(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
             }
             chars[nchars++] = ':';
 #else
-            if (gsop[j]) {
+            if (needOldStyleGetterSetter) {
-                gsoplength = JSSTRING_LENGTH(gsop[j]);
+                js_strncpy(&chars[nchars], idstrchars, idstrlength);
-                js_strncpy(&chars[nchars], JSSTRING_CHARS(gsop[j]), gsoplength);
+                nchars += idstrlength;
-                nchars += gsoplength;
+                if (gsop[j]) {
-                chars[nchars++] = ' ';
+                    chars[nchars++] = ' ';
+                    gsoplength = JSSTRING_LENGTH(gsop[j]);
+                    js_strncpy(&chars[nchars], JSSTRING_CHARS(gsop[j]),
+                               gsoplength);
+                    nchars += gsoplength;
+                }
+                chars[nchars++] = ':';
+            } else {  /* New style "decompilation" */
+                if (gsop[j]) {
+                    gsoplength = JSSTRING_LENGTH(gsop[j]);
+                    js_strncpy(&chars[nchars], JSSTRING_CHARS(gsop[j]),
+                               gsoplength);
+                    nchars += gsoplength;
+                    chars[nchars++] = ' ';
+                }
+                js_strncpy(&chars[nchars], idstrchars, idstrlength);
+                nchars += idstrlength;
+                /* Extraneous space after id here will be extracted later */
+                chars[nchars++] = gsop[j] ? ' ' : ':';
             }
-            js_strncpy(&chars[nchars], idstrchars, idstrlength);
-            nchars += idstrlength;
-            chars[nchars++] = gsop[j] ? ' ' : ':';
 #endif
             if (vsharplength) {
                 js_strncpy(&chars[nchars], vsharp, vsharplength);

js/src/jsopcode.c

@@ -3860,7 +3860,9 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb)
                               rval);
 #else
                 if (lastop == JSOP_GETTER || lastop == JSOP_SETTER) {
-                    if (strncmp(rval, js_function_str, 8) || rval[8] != ' ') {
+                    if (!ATOM_IS_IDENTIFIER(atom) ||
+                        strncmp(rval, js_function_str, 8) ||
+                        rval[8] != ' ') {
                         todo = Sprint(&ss->sprinter, "%s%s%s%s%s:%s", lval,
                                       (lval[1] != '\0') ? ", " : "", xval,
                                       (lastop == JSOP_GETTER ||