[left-trimming] Avoid creating duplicate handles in builtins.cc

EnsureJSArrayWithWritableFastElements

Having several handles pointing to the backing store of an array that gets
left-trimmed might cause the gc to start marking a stale-handle still pointing
to the old backing-store start. By introducing a separate handle scope for
EnsureJSArrayWithWritableFastElements we avoid this issue. Additionally a
SLOW_DCHECK in Heap::LeftTrimFixedArray ensurse that there are no more than one
active handle pointing to the backing store.

BUG=chr:585787
LOG=n

Review URL: https://codereview.chromium.org/1699733003

Cr-Commit-Position: refs/heads/master@{#34022}

src/builtins.cc

@@ -251,6 +251,9 @@ MUST_USE_RESULT
 inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
     Isolate* isolate, Handle<Object> receiver, Arguments* args,
     int first_added_arg) {
+  // We explicitly add a HandleScope to avoid creating several copies of the
+  // same handle which would otherwise cause issue when left-trimming later-on.
+  HandleScope scope(isolate);
   if (!receiver->IsJSArray()) return MaybeHandle<FixedArrayBase>();
   Handle<JSArray> array = Handle<JSArray>::cast(receiver);
   // If there may be elements accessors in the prototype chain, the fast path
@@ -264,12 +267,18 @@ inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
   Handle<FixedArrayBase> elms(array->elements(), isolate);
   Map* map = elms->map();
   if (map == heap->fixed_array_map()) {
-    if (args == NULL || array->HasFastObjectElements()) return elms;
+    if (args == NULL || array->HasFastObjectElements()) {
+      return scope.CloseAndEscape(elms);
+    }
   } else if (map == heap->fixed_cow_array_map()) {
     elms = JSObject::EnsureWritableFastElements(array);
-    if (args == NULL || array->HasFastObjectElements()) return elms;
+    if (args == NULL || array->HasFastObjectElements()) {
+      return scope.CloseAndEscape(elms);
+    }
   } else if (map == heap->fixed_double_array_map()) {
-    if (args == NULL) return elms;
+    if (args == NULL) {
+      return scope.CloseAndEscape(elms);
+    }
   } else {
     return MaybeHandle<FixedArrayBase>();
   }
@@ -283,7 +292,9 @@ inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
   // Need to ensure that the arguments passed in args can be contained in
   // the array.
   int args_length = args->length();
-  if (first_added_arg >= args_length) return handle(array->elements(), isolate);
+  if (first_added_arg >= args_length) {
+    return scope.CloseAndEscape(elms);
+  }
 
   ElementsKind origin_kind = array->map()->elements_kind();
   DCHECK(!IsFastObjectElementsKind(origin_kind));
@@ -306,9 +317,9 @@ inline MaybeHandle<FixedArrayBase> EnsureJSArrayWithWritableFastElements(
   }
   if (target_kind != origin_kind) {
     JSObject::TransitionElementsKind(array, target_kind);
-    return handle(array->elements(), isolate);
+    elms = handle(array->elements(), isolate);
   }
-  return elms;
+  return scope.CloseAndEscape(elms);
 }
 
 

src/heap/heap.cc

@@ -3109,6 +3109,10 @@ FixedArrayBase* Heap::LeftTrimFixedArray(FixedArrayBase* object,
   DCHECK(!lo_space()->Contains(object));
   DCHECK(object->map() != fixed_cow_array_map());
 
+  // Ensure that the no handle-scope has more than one pointer to the same
+  // backing-store.
+  SLOW_DCHECK(CountHandlesForObject(object) <= 1);
+
   STATIC_ASSERT(FixedArrayBase::kMapOffset == 0);
   STATIC_ASSERT(FixedArrayBase::kLengthOffset == kPointerSize);
   STATIC_ASSERT(FixedArrayBase::kHeaderSize == 2 * kPointerSize);
@@ -5475,6 +5479,32 @@ void Heap::PrintHandles() {
 
 #endif
 
+#ifdef ENABLE_SLOW_DCHECKS
+
+class CountHandleVisitor : public ObjectVisitor {
+ public:
+  explicit CountHandleVisitor(Object* object) : object_(object) {}
+
+  void VisitPointers(Object** start, Object** end) override {
+    for (Object** p = start; p < end; p++) {
+      if (object_ == reinterpret_cast<Object*>(*p)) count_++;
+    }
+  }
+
+  int count() { return count_; }
+
+ private:
+  Object* object_;
+  int count_ = 0;
+};
+
+int Heap::CountHandlesForObject(Object* object) {
+  CountHandleVisitor v(object);
+  isolate_->handle_scope_implementer()->Iterate(&v);
+  return v.count();
+}
+#endif
+
 class CheckHandleCountVisitor : public ObjectVisitor {
  public:
   CheckHandleCountVisitor() : handle_count_(0) {}

src/heap/heap.h

@@ -1362,6 +1362,9 @@ class Heap {
   void ReportHeapStatistics(const char* title);
   void ReportCodeStatistics(const char* title);
 #endif
+#ifdef ENABLE_SLOW_DCHECKS
+  int CountHandlesForObject(Object* object);
+#endif
 
  private:
   class PretenuringScope;