Add regression tests that check the native context of accessors

This CL adds regression tests for two bugs where the wrong native
context is used when lazy accessors are instantiated.

The first bug injects an object created in context 1, into another
context 2. The object has an accessor pair installed via
FunctionTemplate. In context 2, the property descriptor of this
accessor is retrieved, causing the JSFunction to be instantiated
with the current context (context 2) instead of the creation
context of the object (context 1).

The second bug is similar. When breakpoints are set, the whole heap
is walked and all lazy accessor pairs are instantiated. This again
uses the current context instead of using the context from which
a AccessorPair originates.

Bug: chromium:986063, chromium:989909
Change-Id: Iaaea6e81f1b9f6b55fc7583b260aa9aea035a8d3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1730999Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63048}

test/unittests/api/access-check-unittest.cc

@@ -27,6 +27,12 @@ MaybeLocal<Value> CompileRun(Isolate* isolate, const char* source) {
   return script->Run(context);
 }
 
+v8::Local<v8::String> v8_str(const char* x) {
+  return v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), x,
+                                 v8::NewStringType::kNormal)
+      .ToLocalChecked();
+}
+
 }  // namespace
 
 TEST_F(AccessCheckTest, GetOwnPropertyDescriptor) {
@@ -42,10 +48,8 @@ TEST_F(AccessCheckTest, GetOwnPropertyDescriptor) {
   Local<FunctionTemplate> setter_template = FunctionTemplate::New(
       isolate(), [](const FunctionCallbackInfo<v8::Value>& info) { FAIL(); });
   setter_template->SetAcceptAnyReceiver(false);
-  global_template->SetAccessorProperty(
-      String::NewFromUtf8(isolate(), "property", NewStringType::kNormal)
-          .ToLocalChecked(),
-      getter_template, setter_template);
+  global_template->SetAccessorProperty(v8_str("property"), getter_template,
+                                       setter_template);
 
   Local<Context> target_context =
       Context::New(isolate(), nullptr, global_template);
@@ -53,10 +57,7 @@ TEST_F(AccessCheckTest, GetOwnPropertyDescriptor) {
       Context::New(isolate(), nullptr, global_template);
 
   accessing_context->Global()
-      ->Set(accessing_context,
-            String::NewFromUtf8(isolate(), "other", NewStringType::kNormal)
-                .ToLocalChecked(),
-            target_context->Global())
+      ->Set(accessing_context, v8_str("other"), target_context->Global())
       .FromJust();
 
   Context::Scope context_scope(accessing_context);
@@ -71,4 +72,113 @@ TEST_F(AccessCheckTest, GetOwnPropertyDescriptor) {
              "    .set.call(other, 42);");
 }
 
+class AccessRegressionTest : public AccessCheckTest {
+ protected:
+  i::Handle<i::JSFunction> RetrieveFunctionFrom(Local<Context> context,
+                                                const char* script) {
+    Context::Scope context_scope(context);
+    Local<Value> getter = CompileRun(isolate(), script).ToLocalChecked();
+    EXPECT_TRUE(getter->IsFunction());
+
+    i::Handle<i::JSReceiver> r =
+        Utils::OpenHandle(*Local<Function>::Cast(getter));
+    EXPECT_TRUE(r->IsJSFunction());
+    return i::Handle<i::JSFunction>::cast(r);
+  }
+};
+
+TEST_F(AccessRegressionTest,
+       InstantiatedLazyAccessorPairsHaveCorrectNativeContext) {
+  // The setup creates two contexts and sets an object created
+  // in context 1 on the global of context 2.
+  // The object has an accessor pair {property}. Accessing the
+  // property descriptor of {property} causes instantiation of the
+  // accessor pair. The test checks that the access pair has the
+  // correct native context.
+  Local<FunctionTemplate> getter_template = FunctionTemplate::New(
+      isolate(), [](const FunctionCallbackInfo<Value>&) { FAIL(); });
+  Local<FunctionTemplate> setter_template = FunctionTemplate::New(
+      isolate(), [](const FunctionCallbackInfo<v8::Value>&) { FAIL(); });
+
+  Local<ObjectTemplate> object_template = ObjectTemplate::New(isolate());
+  object_template->SetAccessorProperty(v8_str("property"), getter_template,
+                                       setter_template);
+
+  Local<Context> context1 = Context::New(isolate(), nullptr);
+  Local<Context> context2 = Context::New(isolate(), nullptr);
+
+  Local<Object> object =
+      object_template->NewInstance(context1).ToLocalChecked();
+  context2->Global()
+      ->Set(context2, v8_str("object_from_context1"), object)
+      .Check();
+
+  i::Handle<i::JSFunction> getter = RetrieveFunctionFrom(
+      context2,
+      "Object.getOwnPropertyDescriptor(object_from_context1, 'property').get");
+
+  ASSERT_EQ(getter->native_context(), *Utils::OpenHandle(*context1));
+}
+
+// Regression test for https://crbug.com/986063.
+TEST_F(AccessRegressionTest,
+       InstantiatedLazyAccessorPairsHaveCorrectNativeContextDebug) {
+  // The setup creates two contexts and installs an object "object"
+  // on the global this for each context.
+  // The object consists of:
+  //    - an accessor pair "property".
+  //    - a normal function "breakfn".
+  //
+  // The test sets a break point on {object.breakfn} in the first context.
+  // This forces instantation of the JSFunction for the {object.property}
+  // accessor pair. The test verifies afterwards that the respective
+  // JSFunction of the getter have the correct native context.
+
+  Local<FunctionTemplate> getter_template = FunctionTemplate::New(
+      isolate(), [](const FunctionCallbackInfo<Value>&) { FAIL(); });
+  Local<FunctionTemplate> setter_template = FunctionTemplate::New(
+      isolate(), [](const FunctionCallbackInfo<v8::Value>&) { FAIL(); });
+  Local<FunctionTemplate> break_template = FunctionTemplate::New(
+      isolate(), [](const FunctionCallbackInfo<v8::Value>&) { FAIL(); });
+
+  Local<Context> context1 = Context::New(isolate(), nullptr);
+  Local<Context> context2 = Context::New(isolate(), nullptr);
+
+  Local<ObjectTemplate> object_template = ObjectTemplate::New(isolate());
+  object_template->Set(isolate(), "breakfn", break_template);
+  object_template->SetAccessorProperty(v8_str("property"), getter_template,
+                                       setter_template);
+
+  Local<Object> object1 =
+      object_template->NewInstance(context1).ToLocalChecked();
+  EXPECT_TRUE(
+      context1->Global()->Set(context1, v8_str("object"), object1).IsJust());
+
+  Local<Object> object2 =
+      object_template->NewInstance(context2).ToLocalChecked();
+  EXPECT_TRUE(
+      context2->Global()->Set(context2, v8_str("object"), object2).IsJust());
+
+  // Force instantiation of the JSFunction for the getter and setter
+  // of {object.property} by setting a break point on {object.breakfn}
+  {
+    Context::Scope context_scope(context1);
+    i::Isolate* iso = reinterpret_cast<i::Isolate*>(isolate());
+    i::Handle<i::JSFunction> break_fn =
+        RetrieveFunctionFrom(context1, "object.breakfn");
+
+    int id;
+    iso->debug()->SetBreakpointForFunction(i::handle(break_fn->shared(), iso),
+                                           iso->factory()->empty_string(), &id);
+  }
+
+  i::Handle<i::JSFunction> getter_c1 = RetrieveFunctionFrom(
+      context1, "Object.getOwnPropertyDescriptor(object, 'property').get");
+  i::Handle<i::JSFunction> getter_c2 = RetrieveFunctionFrom(
+      context2, "Object.getOwnPropertyDescriptor(object, 'property').get");
+
+  ASSERT_EQ(getter_c1->native_context(), *Utils::OpenHandle(*context1));
+  ASSERT_EQ(getter_c2->native_context(), *Utils::OpenHandle(*context2));
+}
+
 }  // namespace v8

test/unittests/unittests.status

@@ -3,6 +3,13 @@
 # found in the LICENSE file.
 
 [
+[ALWAYS, {
+  # 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', {
   # BUG(820416).
   'BitsDeathTest*': [SKIP],