[regexp] Force RegExpResult to slow lookup hidden internal fields.

Currently, RegExpResult builds match indices lazily using data stored
in hidden internal fields on the result object itself. Unfortunately,
if an internal field is deleted, it can cause these hidden fields
to migrate to a dictionary, making indexed lookup unsafe. This CL
forces slow but safe lookup for these fields when lazily building
indices.

Bug: v8:9548, chromium:1013133
Change-Id: Ide87d9ca6a73644ced3de8e35ecac26330d365e4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1871756Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Joshua Litt <joshualitt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64474}

src/objects/js-regexp-inl.h

@@ -27,10 +27,6 @@ CAST_ACCESSOR(JSRegExpResultIndices)
 
 ACCESSORS(JSRegExp, last_index, Object, kLastIndexOffset)
 
-ACCESSORS(JSRegExpResult, cached_indices_or_match_info, Object,
-          kCachedIndicesOrMatchInfoOffset)
-ACCESSORS(JSRegExpResult, names, Object, kNamesOffset)
-
 JSRegExp::Type JSRegExp::TypeTag() const {
   Object data = this->data();
   if (data.IsUndefined()) return JSRegExp::NOT_COMPILED;

src/objects/js-regexp.cc

@@ -11,9 +11,14 @@ namespace v8 {
 namespace internal {
 Handle<JSArray> JSRegExpResult::GetAndCacheIndices(
     Isolate* isolate, Handle<JSRegExpResult> regexp_result) {
-  // Check for cached indices.
+  // Check for cached indices. We do a slow lookup and set of
+  // the cached_indices_or_match_info and names fields just in
+  // case they have been migrated to dictionaries.
   Handle<Object> indices_or_match_info(
-      regexp_result->cached_indices_or_match_info(), isolate);
+      GetProperty(isolate, regexp_result,
+                  isolate->factory()
+                      ->regexp_result_cached_indices_or_match_info_symbol())
+          .ToHandleChecked());
   if (indices_or_match_info->IsRegExpMatchInfo()) {
     // Build and cache indices for next lookup.
     // TODO(joshualitt): Instead of caching the indices, we could call
@@ -21,15 +26,24 @@ Handle<JSArray> JSRegExpResult::GetAndCacheIndices(
     // newly created array. However, care would have to be taken to ensure
     // a new map is not created each time.
     Handle<RegExpMatchInfo> match_info(
-        RegExpMatchInfo::cast(regexp_result->cached_indices_or_match_info()),
-        isolate);
-    Handle<Object> maybe_names(regexp_result->names(), isolate);
+        RegExpMatchInfo::cast(*indices_or_match_info), isolate);
+    Handle<Object> maybe_names(
+        GetProperty(isolate, regexp_result,
+                    isolate->factory()->regexp_result_names_symbol())
+            .ToHandleChecked());
     indices_or_match_info =
         JSRegExpResultIndices::BuildIndices(isolate, match_info, maybe_names);
 
     // Cache the result and clear the names array.
-    regexp_result->set_cached_indices_or_match_info(*indices_or_match_info);
-    regexp_result->set_names(ReadOnlyRoots(isolate).undefined_value());
+    SetProperty(
+        isolate, regexp_result,
+        isolate->factory()->regexp_result_cached_indices_or_match_info_symbol(),
+        indices_or_match_info)
+        .ToHandleChecked();
+    SetProperty(isolate, regexp_result,
+                isolate->factory()->regexp_result_names_symbol(),
+                isolate->factory()->undefined_value())
+        .ToHandleChecked();
   }
   return Handle<JSArray>::cast(indices_or_match_info);
 }

src/objects/js-regexp.h

@@ -235,11 +235,6 @@ class JSRegExpResult : public JSArray {
   // JSRegExpResult, and maybe JSRegExpResultIndices, but both have the same
   // instance type as JSArray.
 
-  // cached_indices_or_match_info and names, are used to construct the
-  // JSRegExpResultIndices returned from the indices property lazily.
-  DECL_ACCESSORS(cached_indices_or_match_info, Object)
-  DECL_ACCESSORS(names, Object)
-
   // Layout description.
   DEFINE_FIELD_OFFSET_CONSTANTS(JSArray::kSize,
                                 TORQUE_GENERATED_JS_REG_EXP_RESULT_FIELDS)

test/mjsunit/harmony/regexp-match-indices.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Flags: --harmony-regexp-match-indices
+// Flags: --harmony-regexp-match-indices --expose-gc
 
 // Sanity test.
 {
@@ -103,3 +103,11 @@
 
   assertEquals(m.indices.groups, {'Z': [4, 5]})
 }
+
+// Test deleting unrelated fields does not break.
+{
+  const m = /undefined/.exec();
+  delete m['index'];
+  gc();
+  assertEquals(m.indices, [[0, 9]]);
+}