Bug 1321437 - Fix ThrowReadOnlyError to match the error message we would throw in the VM. r=nbp
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 16 Jan 2017 13:47:23 +0100
changeset 374584 7aef8faa66668405511227d5523ad8b7968cba5d
parent 374583 b8192f69e1a05783226e43b849f379e1bc04c3c4
child 374585 3015d6154ec57e4b9a70d86e52063804c5d67f47
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1321437
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1321437 - Fix ThrowReadOnlyError to match the error message we would throw in the VM. r=nbp
js/src/jit-test/tests/ion/bug1321437.js
js/src/jit/CodeGenerator.cpp
js/src/jit/VMFunctions.cpp
js/src/jit/VMFunctions.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1321437.js
@@ -0,0 +1,14 @@
+function f(idx) {
+    "use strict";
+    let z = [0, 1, 2, 3, 4, 5, 6, 7, 8, , , ];
+    Object.freeze(z);
+    try {
+        z[idx] = 0;
+    } catch (e) {
+        return e.message;
+    }
+}
+assertEq(f(4), "4 is read-only");
+assertEq(f(-1), 'can\'t define property "-1": Array is not extensible');
+assertEq(f(9), "can't define property 9: Array is not extensible");
+assertEq(f(0xffffffff), 'can\'t define property "4294967295": Array is not extensible');
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -8457,37 +8457,41 @@ CodeGenerator::emitStoreElementHoleV(T* 
 }
 
 void
 CodeGenerator::visitStoreElementHoleV(LStoreElementHoleV* lir)
 {
     emitStoreElementHoleV(lir);
 }
 
-typedef bool (*ThrowReadOnlyFn)(JSContext*, int32_t);
+typedef bool (*ThrowReadOnlyFn)(JSContext*, HandleObject, int32_t);
 static const VMFunction ThrowReadOnlyInfo =
     FunctionInfo<ThrowReadOnlyFn>(ThrowReadOnlyError, "ThrowReadOnlyError");
 
 void
 CodeGenerator::visitFallibleStoreElementT(LFallibleStoreElementT* lir)
 {
     Register elements = ToRegister(lir->elements());
 
     // Handle frozen objects
     Label isFrozen;
     Address flags(elements, ObjectElements::offsetOfFlags());
     if (!lir->mir()->strict()) {
         masm.branchTest32(Assembler::NonZero, flags, Imm32(ObjectElements::FROZEN), &isFrozen);
     } else {
+        Register object = ToRegister(lir->object());
         const LAllocation* index = lir->index();
         OutOfLineCode* ool;
-        if (index->isConstant())
-            ool = oolCallVM(ThrowReadOnlyInfo, lir, ArgList(Imm32(ToInt32(index))), StoreNothing());
-        else
-            ool = oolCallVM(ThrowReadOnlyInfo, lir, ArgList(ToRegister(index)), StoreNothing());
+        if (index->isConstant()) {
+            ool = oolCallVM(ThrowReadOnlyInfo, lir,
+                            ArgList(object, Imm32(ToInt32(index))), StoreNothing());
+        } else {
+            ool = oolCallVM(ThrowReadOnlyInfo, lir,
+                            ArgList(object, ToRegister(index)), StoreNothing());
+        }
         masm.branchTest32(Assembler::NonZero, flags, Imm32(ObjectElements::FROZEN), ool->entry());
         // This OOL code should have thrown an exception, so will never return.
         // So, do not bind ool->rejoin() anywhere, so that it implicitly (and without the cost
         // of a jump) does a masm.assumeUnreachable().
     }
 
     emitStoreElementHoleT(lir);
 
@@ -8500,22 +8504,26 @@ CodeGenerator::visitFallibleStoreElement
     Register elements = ToRegister(lir->elements());
 
     // Handle frozen objects
     Label isFrozen;
     Address flags(elements, ObjectElements::offsetOfFlags());
     if (!lir->mir()->strict()) {
         masm.branchTest32(Assembler::NonZero, flags, Imm32(ObjectElements::FROZEN), &isFrozen);
     } else {
+        Register object = ToRegister(lir->object());
         const LAllocation* index = lir->index();
         OutOfLineCode* ool;
-        if (index->isConstant())
-            ool = oolCallVM(ThrowReadOnlyInfo, lir, ArgList(Imm32(ToInt32(index))), StoreNothing());
-        else
-            ool = oolCallVM(ThrowReadOnlyInfo, lir, ArgList(ToRegister(index)), StoreNothing());
+        if (index->isConstant()) {
+            ool = oolCallVM(ThrowReadOnlyInfo, lir,
+                            ArgList(object, Imm32(ToInt32(index))), StoreNothing());
+        } else {
+            ool = oolCallVM(ThrowReadOnlyInfo, lir,
+                            ArgList(object, ToRegister(index)), StoreNothing());
+        }
         masm.branchTest32(Assembler::NonZero, flags, Imm32(ObjectElements::FROZEN), ool->entry());
         // This OOL code should have thrown an exception, so will never return.
         // So, do not bind ool->rejoin() anywhere, so that it implicitly (and without the cost
         // of a jump) does a masm.assumeUnreachable().
     }
 
     emitStoreElementHoleV(lir);
 
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -1313,20 +1313,31 @@ ThrowRuntimeLexicalError(JSContext* cx, 
 {
     ScriptFrameIter iter(cx);
     RootedScript script(cx, iter.script());
     ReportRuntimeLexicalError(cx, errorNumber, script, iter.pc());
     return false;
 }
 
 bool
-ThrowReadOnlyError(JSContext* cx, int32_t index)
+ThrowReadOnlyError(JSContext* cx, HandleObject obj, int32_t index)
 {
-    RootedValue val(cx, Int32Value(index));
-    ReportValueError(cx, JSMSG_READ_ONLY, JSDVG_IGNORE_STACK, val, nullptr);
+    // We have to throw different errors depending on whether |index| is past
+    // the array length, etc. It's simpler to just call SetProperty to ensure
+    // we match the interpreter.
+
+    RootedValue objVal(cx, ObjectValue(*obj));
+    RootedValue indexVal(cx, Int32Value(index));
+    RootedId id(cx);
+    if (!ValueToId<CanGC>(cx, indexVal, &id))
+        return false;
+
+    ObjectOpResult result;
+    MOZ_ALWAYS_FALSE(SetProperty(cx, obj, id, UndefinedHandleValue, objVal, result) &&
+                     result.checkStrictErrorOrWarning(cx, obj, id, /* strict = */ true));
     return false;
 }
 
 bool
 ThrowBadDerivedReturn(JSContext* cx, HandleValue v)
 {
     ReportValueError(cx, JSMSG_BAD_DERIVED_RETURN, JSDVG_IGNORE_STACK, v, nullptr);
     return false;
--- a/js/src/jit/VMFunctions.h
+++ b/js/src/jit/VMFunctions.h
@@ -792,17 +792,17 @@ IonMarkFunction(MIRType type)
 
 bool ObjectIsCallable(JSObject* obj);
 bool ObjectIsConstructor(JSObject* obj);
 
 MOZ_MUST_USE bool
 ThrowRuntimeLexicalError(JSContext* cx, unsigned errorNumber);
 
 MOZ_MUST_USE bool
-ThrowReadOnlyError(JSContext* cx, int32_t index);
+ThrowReadOnlyError(JSContext* cx, HandleObject obj, int32_t index);
 
 MOZ_MUST_USE bool
 BaselineThrowUninitializedThis(JSContext* cx, BaselineFrame* frame);
 
 MOZ_MUST_USE bool
 ThrowBadDerivedReturn(JSContext* cx, HandleValue v);
 
 MOZ_MUST_USE bool