From d4918463a93b377be9cbf8325b3d2cfa226985d6 Mon Sep 17 00:00:00 2001
From: caitp <caitp@igalia.com>
Date: Wed, 30 Nov 2016 11:13:18 -0800
Subject: [PATCH] [accessors] handle `writable` changing during
 ArrayLengthSetter

The "writable" property descriptor may legally change during the call to
AnythingToArrayLength(). This change needs to be honoured before calling
JSArray::SetLength(). The change is only honoured when the "length"
property was previously writable, so that changes during a call to
DefineOwnPropertyIgnoreAttributes() is ignored.

BUG=v8:5688
R=cbruni@chromium.org, verwaest@chromium.org, jkummerow@chromium.org

Review-Url: https://codereview.chromium.org/2543553002
Cr-Commit-Position: refs/heads/master@{#41396}
---
 src/accessors.cc             | 22 ++++++++++++++++++++++
 test/mjsunit/array-length.js | 27 +++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/src/accessors.cc b/src/accessors.cc
index 7c6d344ec0..085e00cd44 100644
--- a/src/accessors.cc
+++ b/src/accessors.cc
@@ -167,16 +167,38 @@ void Accessors::ArrayLengthSetter(
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
 
+  DCHECK(Utils::OpenHandle(*name)->SameValue(isolate->heap()->length_string()));
+
   Handle<JSReceiver> object = Utils::OpenHandle(*info.Holder());
   Handle<JSArray> array = Handle<JSArray>::cast(object);
   Handle<Object> length_obj = Utils::OpenHandle(*val);
 
+  bool was_readonly = JSArray::HasReadOnlyLength(array);
+
   uint32_t length = 0;
   if (!JSArray::AnythingToArrayLength(isolate, length_obj, &length)) {
     isolate->OptionalRescheduleException(false);
     return;
   }
 
+  if (!was_readonly && V8_UNLIKELY(JSArray::HasReadOnlyLength(array)) &&
+      length != array->length()->Number()) {
+    // AnythingToArrayLength() may have called setter re-entrantly and modified
+    // its property descriptor. Don't perform this check if "length" was
+    // previously readonly, as this may have been called during
+    // DefineOwnPropertyIgnoreAttributes().
+    if (info.ShouldThrowOnError()) {
+      Factory* factory = isolate->factory();
+      isolate->Throw(*factory->NewTypeError(
+          MessageTemplate::kStrictReadOnlyProperty, Utils::OpenHandle(*name),
+          i::Object::TypeOf(isolate, object), object));
+      isolate->OptionalRescheduleException(false);
+    } else {
+      info.GetReturnValue().Set(false);
+    }
+    return;
+  }
+
   JSArray::SetLength(array, length);
 
   uint32_t actual_new_len = 0;
diff --git a/test/mjsunit/array-length.js b/test/mjsunit/array-length.js
index 02103fa371..ea2a6725b7 100644
--- a/test/mjsunit/array-length.js
+++ b/test/mjsunit/array-length.js
@@ -132,3 +132,30 @@ for (var i = 0; i < 7; i++) {
   var frozen_object = Object.freeze({__proto__:[]});
   assertThrows(function () { frozen_object.length = 10 });
 })();
+
+(function sloppyReentrantDescriptorChange() {
+  var b = [];
+  b.length = {
+    valueOf() {
+      Object.defineProperty(b, "length", {writable: false});
+      return 1;
+    }
+  };
+  assertEquals(0, b.length);
+})();
+
+(function strictReentrantDescriptorChange() {
+  var b = [];
+  assertThrows(() => {
+    "use strict";
+    b.length = {
+      valueOf() {
+        Object.defineProperty(b, "length", {writable: false});
+        return 1;
+      }
+    };
+  }, TypeError);
+
+  b.length = { valueOf() { return 0; } };
+  assertEquals(0, b.length);
+})();