Bug 1304638 - Tests for decompilation of SETELEM/STRICTSETELEM. r?nbp draft
authorEmilio Cobos Álvarez <ecoal95@gmail.com>
Mon, 10 Oct 2016 15:41:19 +0200
changeset 423261 0e193dcafffdf51970c69e91d80eda570b519ddb
parent 422994 10409beaa604513db385c8d1f408b5c3a67e746e
child 423262 a6e940b9ca9c902d3b9ac1ae50df276eb9b61caa
push id31851
push userbmo:ecoal95@gmail.com
push dateMon, 10 Oct 2016 18:07:50 +0000
reviewersnbp
bugs1304638
milestone52.0a1
Bug 1304638 - Tests for decompilation of SETELEM/STRICTSETELEM. r?nbp Note that I'm not super-happy with the output of this per se, but since it's used in error messages, it makes more sense to get "a[0] is read-only" than "a[0] = foo is read-only". Can be changed though, I guess. I added a note about it. There's one failing test with the new case, though it was pre-existing, and it wasn't as trivial to fix as I thought. I don't know if we care a lot about it or not, but if we do I'd prefer to move it to another bug. MozReview-Commit-ID: FHfOhsJMd1C
js/src/jit-test/tests/basic/expression-autopsy.js
js/src/jsopcode.cpp
--- a/js/src/jit-test/tests/basic/expression-autopsy.js
+++ b/js/src/jit-test/tests/basic/expression-autopsy.js
@@ -11,17 +11,17 @@ function check_one(expected, f, err) {
         assertEq(s.slice(0, 11), "TypeError: ");
         assertEq(s.slice(-err.length), err, "" + f);
         assertEq(s.slice(11, -err.length), expected);
     }
     if (!failed)
         throw new Error("didn't fail");
 }
 ieval = eval;
-function check(expr, expected=expr) {
+function check(expr, expected=expr, testStrict=true) {
     var end, err;
     for ([end, err] of [[".random_prop", " is undefined"], ["()", " is not a function"]]) {
         var statement = "o = {};" + expr + end, f;
         var cases = [
             // Global scope
             function () {
                 ieval("var o, undef;\n" + statement);
             },
@@ -42,16 +42,21 @@ function check(expr, expected=expr) {
             // Let in a switch
             Function("var x = 4; switch (x) { case 4: let o, undef;" + statement + "\ncase 6: break;}"),
             // Try-catch blocks
             Function("o", "undef", "try { let q = 4; try { let p = 4; } catch (e) {} } catch (e) {} { let o, undef; " + statement + " }"),
             // Let in for-in (uses with to prevent jit compilation: bug 942804, bug 831120 and bug 1041586)
             Function("with ({}) {} var undef, o; for (let z in [1, 2]) { " + statement + " }"),
         ];
 
+        if (testStrict) {
+            // Strict mode.
+            cases.push(Function("o", "undef", "\"use strict\";\n" + statement));
+        }
+
         for (var f of cases) {
             check_one(expected, f, err);
         }
     }
 }
 
 check("undef");
 check("o.b");
@@ -62,22 +67,25 @@ check("o[null]");
 check("o[0]");
 check("o[1]");
 check("o[3]");
 check("o[256]");
 check("o[65536]");
 check("o[268435455]");
 check("o['1.1']");
 check("o[4 + 'h']", "o['4h']");
-check("this.x");
 check("ieval(undef)", "ieval(...)");
 check("ieval.call()", "ieval.call(...)");
 check("ieval(...[])", "ieval(...)");
 check("ieval(...[undef])", "ieval(...)");
 check("ieval(...[undef, undef])", "ieval(...)");
+check("(o[0] = 4).foo", "o[0].foo");
+// NOTE: This one produces different output in strict mode since "this" is
+// undefined in that case.
+check("this.x", "this.x", false);
 
 for (let tok of ["|", "^", "&", "==", "!==", "===", "!==", "<", "<=", ">", ">=",
                  ">>", "<<", ">>>", "+", "-", "*", "/", "%"]) {
     check("o[(undef " + tok + " 4)]");
 }
 
 check("o[!(o)]");
 check("o[~(o)]");
@@ -87,16 +95,22 @@ check("o[- (o)]");
 
 // A few one off tests
 check_one("6", (function () { 6() }), " is not a function");
 check_one("0", (function () { Array.prototype.reverse.call('123'); }), " is read-only");
 check_one("(intermediate value)[Symbol.iterator](...).next(...).value",
           function () { ieval("{ let x; var [a, b, [c0, c1]] = [x, x, x]; }") }, " is undefined");
 check_one("void 1", function() { (void 1)(); }, " is not a function");
 check_one("void o[1]", function() { var o = []; (void o[1])() }, " is not a function");
+check_one("0", function() {
+  "use strict";
+  var o = [0];
+  Object.freeze(o);
+  o[0] = "foo";
+}, " is read-only");
 
 // Manual testing for this case: the only way to trigger an error is *not* on
 // an attempted property access during destructuring, and the error message
 // invoking ToObject(null) is different: "can't convert {0} to object".
 try
 {
   (function() {
     var [{x}] = [null, {}];
@@ -105,10 +119,25 @@ try
 }
 catch (e)
 {
   assertEq(e instanceof TypeError, true,
            "expected TypeError, got " + e);
   assertEq(e.message, "can't convert null to object");
 }
 
+try {
+  (function() {
+    "use strict";
+    var o = [];
+    Object.freeze(o);
+    o[0] = "foo";
+  }());
+  throw new Error("didn't throw");
+} catch (e) {
+  assertEq(e instanceof TypeError, true,
+           "expected TypeError, got " + e);
+  assertEq(e.message,
+           "can't define array index property past the end of an array with non-writable length");
+}
+
 // Check fallback behavior
 assertThrowsInstanceOf(function () { for (let x of undefined) {} }, TypeError);
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -1262,16 +1262,20 @@ ExpressionDecompiler::decompilePC(jsbyte
       case JSOP_GETPROP_SUPER:
       {
         RootedAtom prop(cx, loadAtom(pc));
         return write("super.") &&
                quote(prop, '\0');
       }
       case JSOP_SETELEM:
       case JSOP_STRICTSETELEM:
+        // NOTE: We don't show the right hand side of the operation because
+        // it's used on error messages like: "a[0] is not readable".
+        //
+        // We could though.
         return decompilePCForStackOperand(pc, -3) &&
                write("[") &&
                decompilePCForStackOperand(pc, -2) &&
                write("]");
       case JSOP_GETELEM:
       case JSOP_CALLELEM:
         return decompilePCForStackOperand(pc, -2) &&
                write("[") &&