Browse Source

chore: cherry-pick f599381978f2 from v8 (#33606)

Jeremy Rose 3 years ago
parent
commit
2c3081afb8
2 changed files with 154 additions and 0 deletions
  1. 1 0
      patches/v8/.patches
  2. 153 0
      patches/v8/cherry-pick-f599381978f2.patch

+ 1 - 0
patches/v8/.patches

@@ -22,3 +22,4 @@ regexp_fix_uaf_in_regexpmacroassembler.patch
 cherry-pick-26b7ad6967b1.patch
 cherry-pick-c46fb3a15ec2.patch
 cherry-pick-a1427aad7cef.patch
+cherry-pick-f599381978f2.patch

+ 153 - 0
patches/v8/cherry-pick-f599381978f2.patch

@@ -0,0 +1,153 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Igor Sheludko <[email protected]>
+Date: Mon, 4 Apr 2022 14:42:56 +0200
+Subject: Fix handling of interceptors, pt.3
+
+... in JSObject::DefineOwnPropertyIgnoreAttributes().
+Don't execute interceptor again if it declined to handle the operation.
+
+(cherry picked from commit c4e66b89b4ecd0e90b31e9e4ed08d38085a84c49)
+
+Bug: chromium:1311641
+No-Try: true
+No-Presubmit: true
+No-Tree-Checks: true
+Change-Id: Ie9aef5a98959403f6a26e6bef7f4a77d312bd62a
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3568921
+Reviewed-by: Lutz Vahl <[email protected]>
+Reviewed-by: Igor Sheludko <[email protected]>
+Commit-Queue: Igor Sheludko <[email protected]>
+Cr-Commit-Position: refs/branch-heads/9.6@{#56}
+Cr-Branched-From: 0b7bda016178bf438f09b3c93da572ae3663a1f7-refs/heads/9.6.180@{#1}
+Cr-Branched-From: 41a5a247d9430b953e38631e88d17790306f7a4c-refs/heads/main@{#77244}
+
+diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc
+index e329cba144b271e2e35af67deb324bc7834e7a2a..91c4949b136ac74d55e4ce4cfb929aacd9828dcc 100644
+--- a/src/objects/js-objects.cc
++++ b/src/objects/js-objects.cc
+@@ -3348,13 +3348,24 @@ Maybe<bool> JSObject::DefineOwnPropertyIgnoreAttributes(
+       // TODO(verwaest): JSProxy afterwards verify the attributes that the
+       // JSProxy claims it has, and verifies that they are compatible. If not,
+       // they throw. Here we should do the same.
+-      case LookupIterator::INTERCEPTOR:
++      case LookupIterator::INTERCEPTOR: {
+         if (handling == DONT_FORCE_FIELD) {
+           Maybe<bool> result =
+               JSObject::SetPropertyWithInterceptor(it, should_throw, value);
+           if (result.IsNothing() || result.FromJust()) return result;
+         }
+-        break;
++
++        // The interceptor declined to handle the operation, so proceed defining
++        // own property without the interceptor.
++        Isolate* isolate = it->isolate();
++        Handle<Object> receiver = it->GetReceiver();
++        LookupIterator::Configuration c = LookupIterator::OWN_SKIP_INTERCEPTOR;
++        LookupIterator own_lookup =
++            it->IsElement() ? LookupIterator(isolate, receiver, it->index(), c)
++                            : LookupIterator(isolate, receiver, it->name(), c);
++        return JSObject::DefineOwnPropertyIgnoreAttributes(
++            &own_lookup, value, attributes, should_throw, handling);
++      }
+ 
+       case LookupIterator::ACCESSOR: {
+         Handle<Object> accessors = it->GetAccessors();
+diff --git a/test/cctest/test-api-interceptors.cc b/test/cctest/test-api-interceptors.cc
+index 475003f73c49b8e462ef1896f3fad97857e24574..393dd7192542d299dfa4fe332c5fcff2dff79903 100644
+--- a/test/cctest/test-api-interceptors.cc
++++ b/test/cctest/test-api-interceptors.cc
+@@ -60,6 +60,16 @@ void EmptyInterceptorDeleter(
+ void EmptyInterceptorEnumerator(
+     const v8::PropertyCallbackInfo<v8::Array>& info) {}
+ 
++void EmptyInterceptorDefinerWithSideEffect(
++    Local<Name> name, const v8::PropertyDescriptor& desc,
++    const v8::PropertyCallbackInfo<v8::Value>& info) {
++  ApiTestFuzzer::Fuzz();
++  v8::Local<v8::Value> result = CompileRun("interceptor_definer_side_effect()");
++  if (!result->IsNull()) {
++    info.GetReturnValue().Set(result);
++  }
++}
++
+ void SimpleAccessorGetter(Local<String> name,
+                           const v8::PropertyCallbackInfo<v8::Value>& info) {
+   Local<Object> self = info.This().As<Object>();
+@@ -869,13 +879,17 @@ THREADED_TEST(InterceptorHasOwnPropertyCausingGC) {
+ namespace {
+ 
+ void CheckInterceptorIC(v8::GenericNamedPropertyGetterCallback getter,
++                        v8::GenericNamedPropertySetterCallback setter,
+                         v8::GenericNamedPropertyQueryCallback query,
+-                        const char* source, int expected) {
++                        v8::GenericNamedPropertyDefinerCallback definer,
++                        v8::PropertyHandlerFlags flags, const char* source,
++                        int expected) {
+   v8::Isolate* isolate = CcTest::isolate();
+   v8::HandleScope scope(isolate);
+   v8::Local<v8::ObjectTemplate> templ = ObjectTemplate::New(isolate);
+   templ->SetHandler(v8::NamedPropertyHandlerConfiguration(
+-      getter, nullptr, query, nullptr, nullptr, v8_str("data")));
++      getter, setter, query, nullptr /* deleter */, nullptr /* enumerator */,
++      definer, nullptr /* descriptor */, v8_str("data"), flags));
+   LocalContext context;
+   context->Global()
+       ->Set(context.local(), v8_str("o"),
+@@ -885,9 +899,17 @@ void CheckInterceptorIC(v8::GenericNamedPropertyGetterCallback getter,
+   CHECK_EQ(expected, value->Int32Value(context.local()).FromJust());
+ }
+ 
++void CheckInterceptorIC(v8::GenericNamedPropertyGetterCallback getter,
++                        v8::GenericNamedPropertyQueryCallback query,
++                        const char* source, int expected) {
++  CheckInterceptorIC(getter, nullptr, query, nullptr,
++                     v8::PropertyHandlerFlags::kNone, source, expected);
++}
++
+ void CheckInterceptorLoadIC(v8::GenericNamedPropertyGetterCallback getter,
+                             const char* source, int expected) {
+-  CheckInterceptorIC(getter, nullptr, source, expected);
++  CheckInterceptorIC(getter, nullptr, nullptr, nullptr,
++                     v8::PropertyHandlerFlags::kNone, source, expected);
+ }
+ 
+ void InterceptorLoadICGetter(Local<Name> name,
+@@ -1581,6 +1603,38 @@ THREADED_TEST(InterceptorStoreICWithSideEffectfulCallbacks) {
+                      19);
+ }
+ 
++THREADED_TEST(InterceptorDefineICWithSideEffectfulCallbacks) {
++  CheckInterceptorIC(EmptyInterceptorGetter, EmptyInterceptorSetter,
++                     EmptyInterceptorQuery,
++                     EmptyInterceptorDefinerWithSideEffect,
++                     v8::PropertyHandlerFlags::kNonMasking,
++                     "let inside_side_effect = false;"
++                     "let interceptor_definer_side_effect = function() {"
++                     "  if (!inside_side_effect) {"
++                     "    inside_side_effect = true;"
++                     "    o.y = 153;"
++                     "    inside_side_effect = false;"
++                     "  }"
++                     "  return null;"
++                     "};"
++                     "class Base {"
++                     "  constructor(arg) {"
++                     "    return arg;"
++                     "  }"
++                     "}"
++                     "class ClassWithField extends Base {"
++                     "  y = (() => {"
++                     "    return 42;"
++                     "  })();"
++                     "  constructor(arg) {"
++                     "    super(arg);"
++                     "  }"
++                     "}"
++                     "new ClassWithField(o);"
++                     "o.y",
++                     42);
++}
++
+ static void InterceptorStoreICSetter(
+     Local<Name> key, Local<Value> value,
+     const v8::PropertyCallbackInfo<v8::Value>& info) {