Bug 609617 - delete(eval(...)) calls indirect eval (r=jimb).

js/src/js.msg

@@ -130,7 +130,7 @@ MSG_DEF(JSMSG_MIN_TOO_BIG,             47, 1, JSEXN_SYNTAXERR, "overlarge minimu
 MSG_DEF(JSMSG_MAX_TOO_BIG,             48, 1, JSEXN_SYNTAXERR, "overlarge maximum {0}")
 MSG_DEF(JSMSG_OUT_OF_ORDER,            49, 1, JSEXN_SYNTAXERR, "maximum {0} less than minimum")
 MSG_DEF(JSMSG_BAD_DESTRUCT_DECL,       50, 0, JSEXN_SYNTAXERR, "missing = in destructuring declaration")
-MSG_DEF(JSMSG_BAD_DESTRUCT_ASS,        51, 0, JSEXN_SYNTAXERR, "invalid destructuring assignment operator")
+MSG_DEF(JSMSG_BAD_DESTRUCT_ASS,        51, 0, JSEXN_REFERENCEERR, "invalid destructuring assignment operator")
 MSG_DEF(JSMSG_PAREN_AFTER_LET,         52, 0, JSEXN_SYNTAXERR, "missing ) after let head")
 MSG_DEF(JSMSG_CURLY_AFTER_LET,         53, 0, JSEXN_SYNTAXERR, "missing } after let block")
 MSG_DEF(JSMSG_MISSING_PAREN,           54, 0, JSEXN_SYNTAXERR, "unterminated parenthetical")
@@ -208,7 +208,7 @@ MSG_DEF(JSMSG_BAD_LABEL,              125, 0, JSEXN_SYNTAXERR, "invalid label")
 MSG_DEF(JSMSG_DUPLICATE_LABEL,        126, 0, JSEXN_SYNTAXERR, "duplicate label")
 MSG_DEF(JSMSG_VAR_HIDES_ARG,          127, 1, JSEXN_TYPEERR, "variable {0} redeclares argument")
 MSG_DEF(JSMSG_BAD_VAR_INIT,           128, 0, JSEXN_SYNTAXERR, "invalid variable initialization")
-MSG_DEF(JSMSG_BAD_LEFTSIDE_OF_ASS,    129, 0, JSEXN_SYNTAXERR, "invalid assignment left-hand side")
+MSG_DEF(JSMSG_BAD_LEFTSIDE_OF_ASS,    129, 0, JSEXN_REFERENCEERR, "invalid assignment left-hand side")
 MSG_DEF(JSMSG_BAD_OPERAND,            130, 1, JSEXN_SYNTAXERR, "invalid {0} operand")
 MSG_DEF(JSMSG_BAD_PROP_ID,            131, 0, JSEXN_SYNTAXERR, "invalid property id")
 MSG_DEF(JSMSG_RESERVED_ID,            132, 1, JSEXN_SYNTAXERR, "{0} is a reserved identifier")
@@ -299,8 +299,8 @@ MSG_DEF(JSMSG_ARRAY_COMP_LEFTSIDE,    216, 0, JSEXN_SYNTAXERR, "invalid array co
 MSG_DEF(JSMSG_NON_XML_FILTER,         217, 1, JSEXN_TYPEERR, "XML filter is applied to non-XML value {0}")
 MSG_DEF(JSMSG_EMPTY_ARRAY_REDUCE,     218, 0, JSEXN_TYPEERR, "reduce of empty array with no initial value")
 MSG_DEF(JSMSG_NON_LIST_XML_METHOD,    219, 2, JSEXN_TYPEERR, "can't call {0} method on an XML list with {1} elements")
-MSG_DEF(JSMSG_BAD_DELETE_OPERAND,     220, 0, JSEXN_SYNTAXERR, "invalid delete operand")
+MSG_DEF(JSMSG_BAD_DELETE_OPERAND,     220, 0, JSEXN_REFERENCEERR, "invalid delete operand")
-MSG_DEF(JSMSG_BAD_INCOP_OPERAND,      221, 0, JSEXN_SYNTAXERR, "invalid increment/decrement operand")
+MSG_DEF(JSMSG_BAD_INCOP_OPERAND,      221, 0, JSEXN_REFERENCEERR, "invalid increment/decrement operand")
 MSG_DEF(JSMSG_UNEXPECTED_TYPE,        222, 2, JSEXN_TYPEERR, "{0} is {1}")
 MSG_DEF(JSMSG_LET_DECL_NOT_IN_BLOCK,  223, 0, JSEXN_SYNTAXERR, "let declaration not directly within block")
 MSG_DEF(JSMSG_BAD_OBJECT_INIT,        224, 0, JSEXN_SYNTAXERR, "invalid object initializer")
@@ -314,7 +314,7 @@ MSG_DEF(JSMSG_NOT_NONNULL_OBJECT,     231, 0, JSEXN_TYPEERR, "value is not a non
 MSG_DEF(JSMSG_DEPRECATED_OCTAL,       232, 0, JSEXN_SYNTAXERR, "octal literals and octal escape sequences are deprecated")
 MSG_DEF(JSMSG_STRICT_CODE_WITH,       233, 0, JSEXN_SYNTAXERR, "strict mode code may not contain 'with' statements")
 MSG_DEF(JSMSG_DUPLICATE_PROPERTY,     234, 1, JSEXN_SYNTAXERR, "property name {0} appears more than once in object literal")
-MSG_DEF(JSMSG_DEPRECATED_DELETE_OPERAND, 235, 0, JSEXN_SYNTAXERR, "Applying the 'delete' operator to an unqualified name is deprecated")
+MSG_DEF(JSMSG_DEPRECATED_DELETE_OPERAND, 235, 0, JSEXN_SYNTAXERR, "applying the 'delete' operator to an unqualified name is deprecated")
 MSG_DEF(JSMSG_DEPRECATED_ASSIGN,      236, 1, JSEXN_SYNTAXERR, "assignment to {0} is deprecated")
 MSG_DEF(JSMSG_BAD_BINDING,            237, 1, JSEXN_SYNTAXERR, "redefining {0} is deprecated")
 MSG_DEF(JSMSG_INVALID_DESCRIPTOR,     238, 0, JSEXN_TYPEERR, "property descriptors must not specify a value or be writable when a getter or setter has been specified")

js/src/jsemit.cpp

@@ -5097,7 +5097,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
                 } else
 #endif
                 if (pn3->pn_type == TOK_LP) {
-                    JS_ASSERT(pn3->pn_op == JSOP_SETCALL);
+                    JS_ASSERT(pn3->pn_xflags & PNX_SETCALL);
                     if (!js_EmitTree(cx, cg, pn3))
                         return JS_FALSE;
                     if (js_Emit1(cx, cg, JSOP_ENUMELEM) < 0)
@@ -6497,8 +6497,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
             if (!useful) {
                 off = noteIndex = -1;
             } else {
-                if (pn2->pn_op == JSOP_SETCALL)
+                JS_ASSERT_IF(pn2->pn_type == TOK_LP, !(pn2->pn_xflags & PNX_SETCALL));
-                    pn2->pn_op = JSOP_CALL;
                 if (!js_EmitTree(cx, cg, pn2))
                     return JS_FALSE;
                 off = CG_OFFSET(cg);
@@ -6640,6 +6639,10 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
             if (EmitBlockChain(cx, cg) < 0)
                 return JS_FALSE;
         }
+        if (pn->pn_xflags & PNX_SETCALL) {
+            if (js_Emit1(cx, cg, JSOP_SETCALL) < 0)
+                return JS_FALSE;
+        }
         break;
       }
 

js/src/jsinterp.cpp

@@ -4788,11 +4788,7 @@ END_CASE(JSOP_CALL)
 
 BEGIN_CASE(JSOP_SETCALL)
 {
-    uintN argc = GET_ARGC(regs.pc);
+    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_LEFTSIDE_OF_ASS);
-    Value *vp = regs.sp - argc - 2;
-    JSBool ok = Invoke(cx, InvokeArgsAlreadyOnTheStack(vp, argc), 0);
-    if (ok)
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_BAD_LEFTSIDE_OF_ASS);
     goto error;
 }
 END_CASE(JSOP_SETCALL)

js/src/jsopcode.cpp

@@ -225,8 +225,7 @@ js_GetVariableStackUses(JSOp op, jsbytecode *pc)
         return GET_UINT16(pc);
       default:
         /* stack: fun, this, [argc arguments] */
-        JS_ASSERT(op == JSOP_NEW || op == JSOP_CALL ||
+        JS_ASSERT(op == JSOP_NEW || op == JSOP_CALL || op == JSOP_EVAL ||
-                  op == JSOP_EVAL || op == JSOP_SETCALL ||
                   op == JSOP_FUNCALL || op == JSOP_FUNAPPLY);
         return 2 + GET_ARGC(pc);
     }
@@ -2076,9 +2075,6 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
                           case JSOP_ENUMCONSTELEM:
                             op = JSOP_GETELEM;
                             break;
-                          case JSOP_SETCALL:
-                            op = JSOP_CALL;
-                            break;
                           case JSOP_GETTHISPROP:
                             /*
                              * NB: JSOP_GETTHISPROP can't fail due to |this|
@@ -3589,7 +3585,6 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
               case JSOP_EVAL:
               case JSOP_FUNCALL:
               case JSOP_FUNAPPLY:
-              case JSOP_SETCALL:
                 argc = GET_ARGC(pc);
                 argv = (char **)
                     cx->malloc((size_t)(argc + 1) * sizeof *argv);
@@ -3655,11 +3650,10 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
                 cx->free(argv);
                 if (!ok)
                     return NULL;
-                if (op == JSOP_SETCALL) {
+                break;
-                    if (!PushOff(ss, todo, op))
+
-                        return NULL;
+              case JSOP_SETCALL:
-                    todo = Sprint(&ss->sprinter, "");
+                todo = Sprint(&ss->sprinter, "");
-                }
                 break;
 
               case JSOP_DELNAME:

js/src/jsopcode.tbl

@@ -201,9 +201,9 @@ OPDEF(JSOP_STRICTNE,  73, "strictne",   "!==",        1,  2,  1, 10,  JOF_BYTE|J
 
 /*
  * Host object extension: given 'o.item(i) = j', the left-hand side compiles
- * JSOP_SETCALL, rather than JSOP_CALL.
+ * JSOP_SETCALL after JSOP_CALL, JSOP_EVAL, JSOP_FUNAPPLY, or JSOP_FUNCALL.
  */
-OPDEF(JSOP_SETCALL,   74, "setcall",    NULL,         3, -1,  2, 18,  JOF_UINT16|JOF_SET)
+OPDEF(JSOP_SETCALL,   74, "setcall",    NULL,         1,  1,  2, 18,  JOF_BYTE)
 
 /*
  * JSOP_ITER sets up a for-in or for-each-in loop using the JSITER_* flag bits
@@ -624,5 +624,7 @@ OPDEF(JSOP_FORGLOBAL,     246,"forglobal",     NULL,  3,  1,  1, 19,  JOF_GLOBAL
 OPDEF(JSOP_BLOCKCHAIN,    247,"blockchain",    NULL,  3,  0,  0,  0, JOF_OBJECT)
 OPDEF(JSOP_NULLBLOCKCHAIN,248,"nullblockchain",NULL,  1,  0,  0,  0, JOF_BYTE)
 
-/* When changing bytecodes, don't forget to update JSXDR_BYTECODE_VERSION. */
+/* Like JSOP_FUNAPPLY but for f.call instead of f.apply. */
 OPDEF(JSOP_FUNCALL,       249,"funcall",       NULL,  3, -1,  1, 18,  JOF_UINT16|JOF_INVOKE)
+
+/* When changing bytecodes, don't forget to update JSXDR_BYTECODE_VERSION. */

js/src/jsparse.cpp

@@ -3770,21 +3770,22 @@ BindVarOrConst(JSContext *cx, BindData *data, JSAtom *atom, JSTreeContext *tc)
     return JS_TRUE;
 }
 
-static JSBool
+static bool
 MakeSetCall(JSContext *cx, JSParseNode *pn, JSTreeContext *tc, uintN msg)
 {
-    JSParseNode *pn2;
-
     JS_ASSERT(pn->pn_arity == PN_LIST);
     JS_ASSERT(pn->pn_op == JSOP_CALL || pn->pn_op == JSOP_EVAL ||
               pn->pn_op == JSOP_FUNCALL || pn->pn_op == JSOP_FUNAPPLY);
-    pn2 = pn->pn_head;
+    if (!ReportStrictModeError(cx, TS(tc->parser), tc, pn, msg))
+        return false;
+
+    JSParseNode *pn2 = pn->pn_head;
     if (pn2->pn_type == TOK_FUNCTION && (pn2->pn_funbox->tcflags & TCF_GENEXP_LAMBDA)) {
         ReportCompileErrorNumber(cx, TS(tc->parser), pn, JSREPORT_ERROR, msg);
-        return JS_FALSE;
+        return false;
     }
-    pn->pn_op = JSOP_SETCALL;
+    pn->pn_xflags |= PNX_SETCALL;
-    return JS_TRUE;
+    return true;
 }
 
 static void
@@ -6610,15 +6611,21 @@ Parser::unaryExpr()
             return NULL;
         switch (pn2->pn_type) {
           case TOK_LP:
-            if (pn2->pn_op != JSOP_SETCALL &&
+            if (!(pn2->pn_xflags & PNX_SETCALL)) {
-                !MakeSetCall(context, pn2, tc, JSMSG_BAD_DELETE_OPERAND)) {
+                /*
-                return NULL;
+                 * Call MakeSetCall to check for errors, but clear PNX_SETCALL
+                 * because the optimizer will eliminate the useless delete.
+                 */
+                if (!MakeSetCall(context, pn2, tc, JSMSG_BAD_DELETE_OPERAND))
+                    return NULL;
+                pn2->pn_xflags &= ~PNX_SETCALL;
             }
             break;
           case TOK_NAME:
             if (!ReportStrictModeError(context, &tokenStream, tc, pn,
-                                       JSMSG_DEPRECATED_DELETE_OPERAND))
+                                       JSMSG_DEPRECATED_DELETE_OPERAND)) {
                 return NULL;
+            }
             pn2->pn_op = JSOP_DELNAME;
             if (pn2->pn_atom == context->runtime->atomState.argumentsAtom)
                 tc->flags |= TCF_FUN_HEAVYWEIGHT;

js/src/jsparse.h

@@ -504,8 +504,8 @@ public:
 #define PNX_XMLROOT     0x20            /* top-most node in XML literal tree */
 #define PNX_GROUPINIT   0x40            /* var [a, b] = [c, d]; unit list */
 #define PNX_NEEDBRACES  0x80            /* braces necessary due to closure */
-#define PNX_FUNCDEFS   0x100            /* contains top-level function
+#define PNX_FUNCDEFS   0x100            /* contains top-level function statements */
-                                           statements */
+#define PNX_SETCALL    0x100            /* call expression in lvalue context */
 #define PNX_DESTRUCT   0x200            /* destructuring special cases:
                                            1. shorthand syntax used, at present
                                               object destructuring ({x,y}) only;

js/src/jsscan.cpp

@@ -666,14 +666,15 @@ js::ReportStrictModeError(JSContext *cx, TokenStream *ts, JSTreeContext *tc, JSP
     JS_ASSERT(ts || tc);
     JS_ASSERT(cx == ts->getContext());
 
-    /* In strict mode code, this is an error, not just a warning. */
+    /* In strict mode code, this is an error, not merely a warning. */
     uintN flags;
-    if ((tc && tc->flags & TCF_STRICT_MODE_CODE) || (ts && ts->isStrictMode()))
+    if ((ts && ts->isStrictMode()) || (tc && (tc->flags & TCF_STRICT_MODE_CODE))) {
         flags = JSREPORT_ERROR;
-    else if (JS_HAS_STRICT_OPTION(cx))
+    } else {
+        if (!JS_HAS_STRICT_OPTION(cx))
+            return true;
         flags = JSREPORT_WARNING;
-    else
+    }
-        return true;
 
     va_list ap;
     va_start(ap, errorNumber);

js/src/jsxdrapi.h

@@ -205,7 +205,7 @@ JS_XDRFindClassById(JSXDRState *xdr, uint32 id);
  * before deserialization of bytecode.  If the saved version does not match
  * the current version, abort deserialization and invalidate the file.
  */
-#define JSXDR_BYTECODE_VERSION      (0xb973c0de - 76)
+#define JSXDR_BYTECODE_VERSION      (0xb973c0de - 77)
 
 /*
  * Library-private functions.

js/src/tests/js1_8_1/decompilation/regress-380237-04.js

@@ -134,42 +134,42 @@ doesNotNeedParens(58, "new a(xx);");
 // Generator expressions cannot be used as LHS, even though they're syntactic 
 // sugar for something that looks a lot like an "lvalue return": (f() = 3).
 
-rejectLHS(59, "++ (xx);");
+rejectLHS(59, "++ (xx);", "ReferenceError");
 rejectLHS(60, "delete xx;");
-rejectLHS(61, "delete (xx);");
+rejectLHS(61, "delete (xx);", "ReferenceError");
 rejectLHS(62, "for (xx in []) { }");
-rejectLHS(63, "for ((xx) in []) { }");
+rejectLHS(63, "for ((xx) in []) { }", "ReferenceError");
 rejectLHS(64, "try { } catch(xx) { }");
 rejectLHS(65, "try { } catch([(xx)]) { }");
 rejectLHS(66, "xx += 3;");
-rejectLHS(67, "(xx) += 3;");
+rejectLHS(67, "(xx) += 3;", "ReferenceError");
 rejectLHS(68, "xx = 3;");
 
 // Assignment
-rejectLHS(69, "        (xx) = 3;");
+rejectLHS(69, "        (xx) = 3;", "ReferenceError");
 rejectLHS(70, "var     (xx) = 3;");
 rejectLHS(71, "const   (xx) = 3;");
 rejectLHS(72, "let     (xx) = 3;");
 
 // Destructuring assignment
-rejectLHS(73, "        [(xx)] = 3;");
+rejectLHS(73, "        [(xx)] = 3;", "ReferenceError");
 rejectLHS(74, "var     [(xx)] = 3;");
 rejectLHS(75, "const   [(xx)] = 3;");
 rejectLHS(76, "let     [(xx)] = 3;");
 
 // Group assignment (Spidermonkey optimization for certain
 // destructuring assignments)
-rejectLHS(77, "        [(xx)] = [3];");
+rejectLHS(77, "        [(xx)] = [3];", "ReferenceError");
 rejectLHS(78, "var     [(xx)] = [3];");
 rejectLHS(79, "const   [(xx)] = [3];");
 rejectLHS(80, "let     [(xx)] = [3];");
 
 // Destructuring & group assignment for array comprehensions, just for kicks.
-rejectLHS(81, "        [xx] = [3];");
+rejectLHS(81, "        [xx] = [3];", "ReferenceError");
 rejectLHS(82, "var     [xx] = [3];");
 rejectLHS(83, "const   [xx] = [3];");
 rejectLHS(84, "let     [xx] = 3;");
-rejectLHS(85, "        [xx] = 3;");
+rejectLHS(85, "        [xx] = 3;", "ReferenceError");
 rejectLHS(86, "var     [xx] = 3;");
 rejectLHS(87, "const   [xx] = 3;");
 rejectLHS(88, "let     [xx] = 3;");
@@ -244,7 +244,7 @@ function needParens(section, pat, exp)
   overParenTest(section, f, exp);
 }
 
-function rejectLHS(section, pat)
+function rejectLHS(section, pat, expect)
 {
   print("Testing section " + section + " pattern " + pat);
 
@@ -252,7 +252,7 @@ function rejectLHS(section, pat)
     
   var ft;
     
-  expect = 'SyntaxError';
+  expect = expect || 'SyntaxError';
   actual = '';
   ft = pat.replace(/xx/, genexp)
     try {

js/src/tests/js1_8_5/regress/jstests.list

@@ -50,3 +50,4 @@ script regress-600137.js
 script regress-601399.js
 script regress-602621.js
 fails-if(!xulRuntime.shell) script regress-607863.js
+script regress-609617.js

js/src/tests/js1_8_5/regress/regress-609617.js

@@ -0,0 +1,83 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var actual;
+var expect = "pass";
+
+var x = "fail";
+function f() {
+    var x = "pass"; 
+    delete(eval("actual = x"));
+}
+f();
+assertEq(actual, expect);
+
+function g() { return 1 }
+function h() { function g() { throw 2; } eval('g()')++; } 
+
+try {
+    h();
+    assertEq(0, -1);
+} catch (e) {
+    assertEq(e, 2);
+}
+
+var lhs_prefix = ["",        "++", "--", "",   "",   "[",             "[y, "      ];
+var lhs_suffix = [" = 'no'", "",   "",   "++", "--", ", y] = [3, 4]", "] = [5, 6]"];
+
+for (var i = 0; i < lhs_prefix.length; i++) {
+    try {
+        eval(lhs_prefix[i] + "eval('x')" + lhs_suffix[i]);
+        assertEq(i, -2);
+    } catch (e) {
+        /*
+         * NB: JSOP_SETCALL throws only JSMSG_BAD_LEFTSIDE_OF_ASS, it does not
+         * specialize for ++ and -- as the compiler's error reporting does. See
+         * the next section's forked assertEq code.
+         */
+        assertEq(e.message, "invalid assignment left-hand side");
+    }
+}
+
+/* Destructuring desugars in the obvious way, so y must be 5 here. */
+assertEq(y, 5);
+
+/* Now test for strict mode rejecting any SETCALL variant at compile time. */
+for (var i = 0; i < lhs_prefix.length; i++) {
+    try {
+        eval("(function () { 'use strict'; " + lhs_prefix[i] + "foo('x')" + lhs_suffix[i] + "; })");
+        assertEq(i, -3);
+    } catch (e) {
+        if (/\+\+|\-\-/.test(lhs_prefix[i] || lhs_suffix[i]))
+            assertEq(e.message, "invalid increment/decrement operand");
+        else
+            assertEq(e.message, "invalid assignment left-hand side");
+    }
+}
+
+/*
+ * The useless delete is optimized away, but the SETCALL must not be. It's not
+ * an early error, though.
+ */
+var fooArg;
+function foo(arg) { fooArg = arg; }
+try {
+    eval("delete (foo('x') = 42);");
+    assertEq(0, -4);
+} catch (e) {
+    assertEq(e.message, "invalid assignment left-hand side");
+}
+assertEq(fooArg, 'x');
+
+/* We extend ES5 by making delete of a call expression a strict mode error. */
+try {
+    eval("(function () { 'use strict'; delete foo('x'); })");
+    assertEq(0, -5);
+} catch (e) {
+    assertEq(e.message, "invalid delete operand");
+}
+
+reportCompare(0, 0, "ok");