Bug 1395095 - Fix MappedArgumentsObject::obj_defineProperty to not create slotful getters/setters. r=evilpie
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 31 Aug 2017 12:20:31 +0200
changeset 377843 c3094f515e809b477bdf056220f1034fc64b5313
parent 377842 7599d3db781e2855da92f29a654eb18e51192b6b
child 377844 b7f2394e1cfee8e5f5e892547a00970c2de7b8b9
push id94350
push userjandemooij@gmail.com
push dateThu, 31 Aug 2017 10:22:15 +0000
treeherdermozilla-inbound@c3094f515e80 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1395095
milestone57.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 1395095 - Fix MappedArgumentsObject::obj_defineProperty to not create slotful getters/setters. r=evilpie
js/src/jit-test/tests/arguments/mapped-define.js
js/src/vm/ArgumentsObject.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/arguments/mapped-define.js
@@ -0,0 +1,20 @@
+function f(a) {
+    assertEq(arguments[0], 1);
+
+    Object.defineProperty(arguments, 0, {value: 23, writable: true, configurable: true});
+    assertEq(arguments[0], 23);
+    assertEq(a, 23);
+
+    a = 12;
+    assertEq(a, 12);
+    assertEq(arguments[0], 12);
+
+    Object.defineProperty(arguments, 0, {value: 9, writable: false, configurable: false});
+    assertEq(arguments[0], 9);
+    assertEq(a, 9);
+
+    a = 4;
+    assertEq(arguments[0], 9);
+    assertEq(a, 4);
+}
+f(1);
--- a/js/src/vm/ArgumentsObject.cpp
+++ b/js/src/vm/ArgumentsObject.cpp
@@ -646,24 +646,39 @@ MappedArgumentsObject::obj_definePropert
     bool isMapped = false;
     if (JSID_IS_INT(id)) {
         unsigned arg = unsigned(JSID_TO_INT(id));
         isMapped = arg < argsobj->initialLength() && !argsobj->isElementDeleted(arg);
     }
 
     // Step 4.
     Rooted<PropertyDescriptor> newArgDesc(cx, desc);
+
+    // Step 5.
     if (!desc.isAccessorDescriptor() && isMapped) {
-        // In this case the live mapping is supposed to keep working,
-        // we have to pass along the Getter/Setter otherwise they are overwritten.
-        newArgDesc.setGetter(MappedArgGetter);
-        newArgDesc.setSetter(MappedArgSetter);
+        // Step 5.a.
+        if (desc.hasWritable() && !desc.writable()) {
+            if (!desc.hasValue()) {
+                RootedValue v(cx, argsobj->element(JSID_TO_INT(id)));
+                newArgDesc.setValue(v);
+            }
+            newArgDesc.setGetter(nullptr);
+            newArgDesc.setSetter(nullptr);
+        } else {
+            // In this case the live mapping is supposed to keep working,
+            // we have to pass along the Getter/Setter otherwise they are
+            // overwritten.
+            newArgDesc.setGetter(MappedArgGetter);
+            newArgDesc.setSetter(MappedArgSetter);
+            newArgDesc.value().setUndefined();
+            newArgDesc.attributesRef() |= JSPROP_IGNORE_VALUE;
+        }
     }
 
-    // Steps 5-6. NativeDefineProperty will lookup [[Value]] for us.
+    // Step 6. NativeDefineProperty will lookup [[Value]] for us.
     if (!NativeDefineProperty(cx, obj.as<NativeObject>(), id, newArgDesc, result))
         return false;
     // Step 7.
     if (!result.ok())
         return true;
 
     // Step 8.
     if (isMapped) {