Use correct native context when instantiating AccessorPairs

This CL changes the way AccessorPairs are collected for instantiation
when debug break trampolines are installed.
Instead of walking the heap and looking at AccessorPairs directly, we
look at all JSObjects and collect AccessorPairs via each objects
descriptor array. This way, we can associate the correct native
context with each collected AccessorPair.

The current native context is not always the correct context to instantiate
the getter and setter JSFunctions for an AccessorPair.

Bug: chromium:986063
Change-Id: I124a0802f4938b95f1ad75efc65eb05b66bcfc67
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1735310
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63071}

src/debug/debug.cc

@@ -1224,8 +1224,12 @@ void Debug::InstallDebugBreakTrampoline() {
 
   Handle<Code> trampoline = BUILTIN_CODE(isolate_, DebugBreakTrampoline);
   std::vector<Handle<JSFunction>> needs_compile;
-  std::vector<Handle<AccessorPair>> needs_instantiate;
+  using AccessorPairWithContext =
+      std::pair<Handle<AccessorPair>, Handle<NativeContext>>;
+  std::vector<AccessorPairWithContext> needs_instantiate;
   {
+    // Deduplicate {needs_instantiate} by recording all collected AccessorPairs.
+    std::set<AccessorPair> recorded;
     HeapObjectIterator iterator(isolate_->heap());
     for (HeapObject obj = iterator.Next(); !obj.is_null();
          obj = iterator.Next()) {
@@ -1242,11 +1246,26 @@ void Debug::InstallDebugBreakTrampoline() {
         } else {
           fun.set_code(*trampoline);
         }
-      } else if (obj.IsAccessorPair()) {
-        AccessorPair accessor_pair = AccessorPair::cast(obj);
-        if (accessor_pair.getter().IsFunctionTemplateInfo() ||
-            accessor_pair.setter().IsFunctionTemplateInfo()) {
-          needs_instantiate.push_back(handle(accessor_pair, isolate_));
+      } else if (obj.IsJSObject()) {
+        JSObject object = JSObject::cast(obj);
+        DescriptorArray descriptors = object.map().instance_descriptors();
+
+        for (int i = 0; i < object.map().NumberOfOwnDescriptors(); ++i) {
+          if (descriptors.GetDetails(i).kind() == PropertyKind::kAccessor) {
+            Object value = descriptors.GetStrongValue(i);
+            if (!value.IsAccessorPair()) continue;
+
+            AccessorPair accessor_pair = AccessorPair::cast(value);
+            if (!accessor_pair.getter().IsFunctionTemplateInfo() &&
+                !accessor_pair.setter().IsFunctionTemplateInfo()) {
+              continue;
+            }
+            if (recorded.find(accessor_pair) != recorded.end()) continue;
+
+            needs_instantiate.emplace_back(handle(accessor_pair, isolate_),
+                                           object.GetCreationContext());
+            recorded.insert(accessor_pair);
+          }
         }
       }
     }
@@ -1254,10 +1273,13 @@ void Debug::InstallDebugBreakTrampoline() {
 
   // Forcibly instantiate all lazy accessor pairs to make sure that they
   // properly hit the debug break trampoline.
-  for (Handle<AccessorPair> accessor_pair : needs_instantiate) {
+  for (AccessorPairWithContext tuple : needs_instantiate) {
+    Handle<AccessorPair> accessor_pair = tuple.first;
+    Handle<NativeContext> native_context = tuple.second;
     if (accessor_pair->getter().IsFunctionTemplateInfo()) {
       Handle<JSFunction> fun =
           ApiNatives::InstantiateFunction(
+              isolate_, native_context,
               handle(FunctionTemplateInfo::cast(accessor_pair->getter()),
                      isolate_))
               .ToHandleChecked();
@@ -1266,6 +1288,7 @@ void Debug::InstallDebugBreakTrampoline() {
     if (accessor_pair->setter().IsFunctionTemplateInfo()) {
       Handle<JSFunction> fun =
           ApiNatives::InstantiateFunction(
+              isolate_, native_context,
               handle(FunctionTemplateInfo::cast(accessor_pair->setter()),
                      isolate_))
               .ToHandleChecked();

test/unittests/unittests.status

@@ -7,7 +7,6 @@
   # https://crbug.com/986063.
   # TODO(szuend): Re-enable test once the fix has landed.
   'AccessRegressionTest.InstantiatedLazyAccessorPairsHaveCorrectNativeContext': [FAIL],
-  'AccessRegressionTest.InstantiatedLazyAccessorPairsHaveCorrectNativeContextDebug': [FAIL],
 }],  # ALWAYS
 
 ['system == macos and asan', {