add interceptors which do not mask existing properties

R=verwaest@chromium.org

BUG=

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

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

include/v8.h

@@ -4188,7 +4188,17 @@ class V8_EXPORT FunctionTemplate : public Template {
 };
 
 
-enum class PropertyHandlerFlags { kNone = 0, kAllCanRead = 1 };
+enum class PropertyHandlerFlags {
+  kNone = 0,
+  // See ALL_CAN_READ above.
+  kAllCanRead = 1,
+  // Will not call into interceptor for properties on the receiver or prototype
+  // chain.  Currently only valid for named interceptors.
+  kNonMasking = 1 << 1,
+  // Will not call into interceptor for symbol lookup.  Only meaningful for
+  // named interceptors.
+  kOnlyInterceptStrings = 1 << 2,
+};
 
 
 struct NamedPropertyHandlerConfiguration {

src/api.cc

@@ -1280,10 +1280,12 @@ void ObjectTemplate::SetAccessor(v8::Handle<Name> name,
 
 template <typename Getter, typename Setter, typename Query, typename Deleter,
           typename Enumerator>
-static void ObjectTemplateSetNamedPropertyHandler(
-    ObjectTemplate* templ, Getter getter, Setter setter, Query query,
-    Deleter remover, Enumerator enumerator, Handle<Value> data,
-    bool can_intercept_symbols, PropertyHandlerFlags flags) {
+static void ObjectTemplateSetNamedPropertyHandler(ObjectTemplate* templ,
+                                                  Getter getter, Setter setter,
+                                                  Query query, Deleter remover,
+                                                  Enumerator enumerator,
+                                                  Handle<Value> data,
+                                                  PropertyHandlerFlags flags) {
   i::Isolate* isolate = Utils::OpenHandle(templ)->GetIsolate();
   ENTER_V8(isolate);
   i::HandleScope scope(isolate);
@@ -1298,9 +1300,13 @@ static void ObjectTemplateSetNamedPropertyHandler(
   if (remover != 0) SET_FIELD_WRAPPED(obj, set_deleter, remover);
   if (enumerator != 0) SET_FIELD_WRAPPED(obj, set_enumerator, enumerator);
   obj->set_flags(0);
-  obj->set_can_intercept_symbols(can_intercept_symbols);
+  obj->set_can_intercept_symbols(
+      !(static_cast<int>(flags) &
+        static_cast<int>(PropertyHandlerFlags::kOnlyInterceptStrings)));
   obj->set_all_can_read(static_cast<int>(flags) &
                         static_cast<int>(PropertyHandlerFlags::kAllCanRead));
+  obj->set_non_masking(static_cast<int>(flags) &
+                       static_cast<int>(PropertyHandlerFlags::kNonMasking));
 
   if (data.IsEmpty()) {
     data = v8::Undefined(reinterpret_cast<v8::Isolate*>(isolate));
@@ -1314,9 +1320,9 @@ void ObjectTemplate::SetNamedPropertyHandler(
     NamedPropertyGetterCallback getter, NamedPropertySetterCallback setter,
     NamedPropertyQueryCallback query, NamedPropertyDeleterCallback remover,
     NamedPropertyEnumeratorCallback enumerator, Handle<Value> data) {
-  ObjectTemplateSetNamedPropertyHandler(this, getter, setter, query, remover,
-                                        enumerator, data, false,
-                                        PropertyHandlerFlags::kNone);
+  ObjectTemplateSetNamedPropertyHandler(
+      this, getter, setter, query, remover, enumerator, data,
+      PropertyHandlerFlags::kOnlyInterceptStrings);
 }
 
 
@@ -1324,7 +1330,7 @@ void ObjectTemplate::SetHandler(
     const NamedPropertyHandlerConfiguration& config) {
   ObjectTemplateSetNamedPropertyHandler(
       this, config.getter, config.setter, config.query, config.deleter,
-      config.enumerator, config.data, true, config.flags);
+      config.enumerator, config.data, config.flags);
 }
 
 

src/ic/handler-compiler.cc

@@ -312,10 +312,32 @@ Handle<Code> NamedLoadHandlerCompiler::CompileLoadInterceptor(
 
   Label miss;
   InterceptorVectorSlotPush(receiver());
+  bool lost_holder_register = false;
+  auto holder_orig = holder();
+  // non masking interceptors must check the entire chain, so temporarily reset
+  // the holder to be that last element for the FrontendHeader call.
+  if (holder()->GetNamedInterceptor()->non_masking()) {
+    DCHECK(!inline_followup);
+    JSObject* last = *holder();
+    PrototypeIterator iter(isolate(), last);
+    while (!iter.IsAtEnd()) {
+      lost_holder_register = true;
+      last = JSObject::cast(iter.GetCurrent());
+      iter.Advance();
+    }
+    auto last_handle = handle(last);
+    set_holder(last_handle);
+  }
   Register reg = FrontendHeader(receiver(), it->name(), &miss);
+  // Reset the holder so further calculations are correct.
+  set_holder(holder_orig);
+  if (lost_holder_register) {
+    // Reload lost holder register.
+    auto cell = isolate()->factory()->NewWeakCell(holder());
+    __ LoadWeakValue(reg, cell, &miss);
+  }
   FrontendFooter(it->name(), &miss);
   InterceptorVectorSlotPop(reg);
-
   if (inline_followup) {
     // TODO(368): Compile in the whole chain: all the interceptors in
     // prototypes and ultimate answer.

src/ic/ic.cc

@@ -2914,30 +2914,10 @@ RUNTIME_FUNCTION(LoadPropertyWithInterceptorOnly) {
   Handle<JSObject> holder =
       args.at<JSObject>(NamedLoadHandlerCompiler::kInterceptorArgsHolderIndex);
   HandleScope scope(isolate);
-  Handle<InterceptorInfo> interceptor_info(holder->GetNamedInterceptor());
-
-  if (name->IsSymbol() && !interceptor_info->can_intercept_symbols())
-    return isolate->heap()->no_interceptor_result_sentinel();
-
-  Address getter_address = v8::ToCData<Address>(interceptor_info->getter());
-  v8::GenericNamedPropertyGetterCallback getter =
-      FUNCTION_CAST<v8::GenericNamedPropertyGetterCallback>(getter_address);
-  DCHECK(getter != NULL);
-
-  PropertyCallbackArguments callback_args(isolate, interceptor_info->data(),
-                                          *receiver, *holder);
-  {
-    // Use the interceptor getter.
-    v8::Handle<v8::Value> r =
-        callback_args.Call(getter, v8::Utils::ToLocal(name));
-    RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
-    if (!r.IsEmpty()) {
-      Handle<Object> result = v8::Utils::OpenHandle(*r);
-      result->VerifyApiCallResultType();
-      return *v8::Utils::OpenHandle(*r);
-    }
-  }
-
+  auto res = JSObject::GetPropertyWithInterceptor(holder, receiver, name);
+  RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
+  Handle<Object> result;
+  if (res.ToHandle(&result)) return *result;
   return isolate->heap()->no_interceptor_result_sentinel();
 }
 

src/lookup-inl.h

@@ -31,10 +31,13 @@ JSReceiver* LookupIterator::NextHolder(Map* map) {
 }
 
 
-LookupIterator::State LookupIterator::LookupInHolder(Map* map,
-                                                     JSReceiver* holder) {
+LookupIterator::State LookupIterator::LookupInHolder(Map* const map,
+                                                     JSReceiver* const holder) {
   STATIC_ASSERT(INTERCEPTOR == BEFORE_PROPERTY);
   DisallowHeapAllocation no_gc;
+  if (interceptor_state_ == InterceptorState::kProcessNonMasking) {
+    return LookupNonMaskingInterceptorInHolder(map, holder);
+  }
   switch (state_) {
     case NOT_FOUND:
       if (map->IsJSProxyMap()) return JSPROXY;
@@ -48,7 +51,8 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* map,
           IsIntegerIndexedExotic(holder)) {
         return INTEGER_INDEXED_EXOTIC;
       }
-      if (check_interceptor() && map->has_named_interceptor()) {
+      if (check_interceptor() && map->has_named_interceptor() &&
+          !SkipInterceptor(JSObject::cast(holder))) {
         return INTERCEPTOR;
       }
     // Fall through.
@@ -86,6 +90,23 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* map,
   UNREACHABLE();
   return state_;
 }
+
+
+LookupIterator::State LookupIterator::LookupNonMaskingInterceptorInHolder(
+    Map* const map, JSReceiver* const holder) {
+  switch (state_) {
+    case NOT_FOUND:
+      if (check_interceptor() && map->has_named_interceptor() &&
+          !SkipInterceptor(JSObject::cast(holder))) {
+        return INTERCEPTOR;
+      }
+    // Fall through.
+    default:
+      return NOT_FOUND;
+  }
+  UNREACHABLE();
+  return state_;
+}
 }
 }  // namespace v8::internal
 

src/lookup.cc

@@ -29,7 +29,13 @@ void LookupIterator::Next() {
   // Continue lookup if lookup on current holder failed.
   do {
     JSReceiver* maybe_holder = NextHolder(map);
-    if (maybe_holder == NULL) break;
+    if (maybe_holder == nullptr) {
+      if (interceptor_state_ == InterceptorState::kSkipNonMasking) {
+        RestartLookupForNonMaskingInterceptors();
+        return;
+      }
+      break;
+    }
     holder = maybe_holder;
     map = holder->map();
     state_ = LookupInHolder(map, holder);
@@ -42,10 +48,21 @@ void LookupIterator::Next() {
 }
 
 
-Handle<JSReceiver> LookupIterator::GetRoot() const {
-  if (receiver_->IsJSReceiver()) return Handle<JSReceiver>::cast(receiver_);
-  Handle<Object> root =
-      handle(receiver_->GetRootMap(isolate_)->prototype(), isolate_);
+void LookupIterator::RestartLookupForNonMaskingInterceptors() {
+  interceptor_state_ = InterceptorState::kProcessNonMasking;
+  state_ = NOT_FOUND;
+  property_details_ = PropertyDetails::Empty();
+  number_ = DescriptorArray::kNotFound;
+  holder_ = initial_holder_;
+  holder_map_ = handle(holder_->map(), isolate_);
+  Next();
+}
+
+
+Handle<JSReceiver> LookupIterator::GetRoot(Handle<Object> receiver,
+                                           Isolate* isolate) {
+  if (receiver->IsJSReceiver()) return Handle<JSReceiver>::cast(receiver);
+  auto root = handle(receiver->GetRootMap(isolate)->prototype(), isolate);
   CHECK(!root->IsNull());
   return Handle<JSReceiver>::cast(root);
 }
@@ -80,6 +97,7 @@ bool LookupIterator::HasAccess() const {
 
 void LookupIterator::ReloadPropertyInformation() {
   state_ = BEFORE_PROPERTY;
+  interceptor_state_ = InterceptorState::kUninitialized;
   state_ = LookupInHolder(*holder_map_, *holder_);
   DCHECK(IsFound() || holder_map_->is_dictionary_map());
 }
@@ -354,4 +372,22 @@ void LookupIterator::InternalizeName() {
   if (name_->IsUniqueName()) return;
   name_ = factory()->InternalizeString(Handle<String>::cast(name_));
 }
+
+
+bool LookupIterator::SkipInterceptor(JSObject* holder) {
+  auto info = holder->GetNamedInterceptor();
+  // TODO(dcarney): check for symbol/can_intercept_symbols here as well.
+  if (info->non_masking()) {
+    switch (interceptor_state_) {
+      case InterceptorState::kUninitialized:
+        interceptor_state_ = InterceptorState::kSkipNonMasking;
+      // Fall through.
+      case InterceptorState::kSkipNonMasking:
+        return true;
+      case InterceptorState::kProcessNonMasking:
+        return false;
+    }
+  }
+  return interceptor_state_ == InterceptorState::kProcessNonMasking;
+}
 } }  // namespace v8::internal

src/lookup.h

@@ -48,13 +48,15 @@ class LookupIterator FINAL BASE_EMBEDDED {
       : configuration_(ComputeConfiguration(configuration, name)),
         state_(NOT_FOUND),
         exotic_index_state_(ExoticIndexState::kUninitialized),
+        interceptor_state_(InterceptorState::kUninitialized),
         property_details_(PropertyDetails::Empty()),
         isolate_(name->GetIsolate()),
         name_(name),
         receiver_(receiver),
+        holder_(GetRoot(receiver_, isolate_)),
+        holder_map_(holder_->map(), isolate_),
+        initial_holder_(holder_),
         number_(DescriptorArray::kNotFound) {
-    holder_ = GetRoot();
-    holder_map_ = handle(holder_->map(), isolate_);
     Next();
   }
 
@@ -64,12 +66,14 @@ class LookupIterator FINAL BASE_EMBEDDED {
       : configuration_(ComputeConfiguration(configuration, name)),
         state_(NOT_FOUND),
         exotic_index_state_(ExoticIndexState::kUninitialized),
+        interceptor_state_(InterceptorState::kUninitialized),
         property_details_(PropertyDetails::Empty()),
         isolate_(name->GetIsolate()),
         name_(name),
-        holder_map_(holder->map(), isolate_),
         receiver_(receiver),
         holder_(holder),
+        holder_map_(holder_->map(), isolate_),
+        initial_holder_(holder_),
         number_(DescriptorArray::kNotFound) {
     Next();
   }
@@ -98,7 +102,7 @@ class LookupIterator FINAL BASE_EMBEDDED {
     DCHECK(IsFound());
     return Handle<T>::cast(holder_);
   }
-  Handle<JSReceiver> GetRoot() const;
+  static Handle<JSReceiver> GetRoot(Handle<Object> receiver, Isolate* isolate);
   bool HolderIsReceiverOrHiddenPrototype() const;
 
   /* ACCESS_CHECK */
@@ -146,12 +150,21 @@ class LookupIterator FINAL BASE_EMBEDDED {
   void InternalizeName();
 
  private:
+  enum class InterceptorState {
+    kUninitialized,
+    kSkipNonMasking,
+    kProcessNonMasking
+  };
+
   Handle<Map> GetReceiverMap() const;
 
   MUST_USE_RESULT inline JSReceiver* NextHolder(Map* map);
   inline State LookupInHolder(Map* map, JSReceiver* holder);
+  void RestartLookupForNonMaskingInterceptors();
+  State LookupNonMaskingInterceptorInHolder(Map* map, JSReceiver* holder);
   Handle<Object> FetchValue() const;
   void ReloadPropertyInformation();
+  bool SkipInterceptor(JSObject* holder);
 
   bool IsBootstrapping() const;
 
@@ -188,18 +201,19 @@ class LookupIterator FINAL BASE_EMBEDDED {
 
   // If configuration_ becomes mutable, update
   // HolderIsReceiverOrHiddenPrototype.
-  Configuration configuration_;
+  const Configuration configuration_;
   State state_;
   bool has_property_;
   ExoticIndexState exotic_index_state_;
+  InterceptorState interceptor_state_;
   PropertyDetails property_details_;
-  Isolate* isolate_;
+  Isolate* const isolate_;
   Handle<Name> name_;
-  Handle<Map> holder_map_;
   Handle<Object> transition_;
-  Handle<Object> receiver_;
+  const Handle<Object> receiver_;
   Handle<JSReceiver> holder_;
-
+  Handle<Map> holder_map_;
+  const Handle<JSReceiver> initial_holder_;
   int number_;
 };
 

src/objects-inl.h

@@ -5452,6 +5452,7 @@ SMI_ACCESSORS(InterceptorInfo, flags, kFlagsOffset)
 BOOL_ACCESSORS(InterceptorInfo, flags, can_intercept_symbols,
                kCanInterceptSymbolsBit)
 BOOL_ACCESSORS(InterceptorInfo, flags, all_can_read, kAllCanReadBit)
+BOOL_ACCESSORS(InterceptorInfo, flags, non_masking, kNonMasking)
 
 ACCESSORS(CallHandlerInfo, callback, Object, kCallbackOffset)
 ACCESSORS(CallHandlerInfo, data, Object, kDataOffset)

src/objects.h

@@ -10573,6 +10573,7 @@ class InterceptorInfo: public Struct {
   DECL_ACCESSORS(data, Object)
   DECL_BOOLEAN_ACCESSORS(can_intercept_symbols)
   DECL_BOOLEAN_ACCESSORS(all_can_read)
+  DECL_BOOLEAN_ACCESSORS(non_masking)
 
   inline int flags() const;
   inline void set_flags(int flags);
@@ -10594,6 +10595,7 @@ class InterceptorInfo: public Struct {
 
   static const int kCanInterceptSymbolsBit = 0;
   static const int kAllCanReadBit = 1;
+  static const int kNonMasking = 2;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(InterceptorInfo);

test/cctest/test-api-interceptors.cc

@@ -3100,3 +3100,130 @@ THREADED_TEST(IndexedAllCanReadInterceptor) {
   ExpectInt32("checked[15]", intercept_data_1.value);
   CHECK_EQ(3, access_check_data.count);
 }
+
+
+THREADED_TEST(NonMaskingInterceptorOwnProperty) {
+  auto isolate = CcTest::isolate();
+  v8::HandleScope handle_scope(isolate);
+  LocalContext context;
+
+  ShouldInterceptData intercept_data;
+  intercept_data.value = 239;
+  intercept_data.should_intercept = true;
+
+  auto interceptor_templ = v8::ObjectTemplate::New(isolate);
+  v8::NamedPropertyHandlerConfiguration conf(ShouldNamedInterceptor);
+  conf.flags = v8::PropertyHandlerFlags::kNonMasking;
+  conf.data = BuildWrappedObject<ShouldInterceptData>(isolate, &intercept_data);
+  interceptor_templ->SetHandler(conf);
+
+  auto interceptor = interceptor_templ->NewInstance();
+  context->Global()->Set(v8_str("obj"), interceptor);
+
+  ExpectInt32("obj.whatever", 239);
+
+  CompileRun("obj.whatever = 4;");
+  ExpectInt32("obj.whatever", 4);
+
+  CompileRun("delete obj.whatever;");
+  ExpectInt32("obj.whatever", 239);
+}
+
+
+THREADED_TEST(NonMaskingInterceptorPrototypeProperty) {
+  auto isolate = CcTest::isolate();
+  v8::HandleScope handle_scope(isolate);
+  LocalContext context;
+
+  ShouldInterceptData intercept_data;
+  intercept_data.value = 239;
+  intercept_data.should_intercept = true;
+
+  auto interceptor_templ = v8::ObjectTemplate::New(isolate);
+  v8::NamedPropertyHandlerConfiguration conf(ShouldNamedInterceptor);
+  conf.flags = v8::PropertyHandlerFlags::kNonMasking;
+  conf.data = BuildWrappedObject<ShouldInterceptData>(isolate, &intercept_data);
+  interceptor_templ->SetHandler(conf);
+
+  auto interceptor = interceptor_templ->NewInstance();
+  context->Global()->Set(v8_str("obj"), interceptor);
+
+  ExpectInt32("obj.whatever", 239);
+
+  CompileRun("obj.__proto__ = {'whatever': 4};");
+  ExpectInt32("obj.whatever", 4);
+
+  CompileRun("delete obj.__proto__.whatever;");
+  ExpectInt32("obj.whatever", 239);
+}
+
+
+THREADED_TEST(NonMaskingInterceptorPrototypePropertyIC) {
+  auto isolate = CcTest::isolate();
+  v8::HandleScope handle_scope(isolate);
+  LocalContext context;
+
+  ShouldInterceptData intercept_data;
+  intercept_data.value = 239;
+  intercept_data.should_intercept = true;
+
+  auto interceptor_templ = v8::ObjectTemplate::New(isolate);
+  v8::NamedPropertyHandlerConfiguration conf(ShouldNamedInterceptor);
+  conf.flags = v8::PropertyHandlerFlags::kNonMasking;
+  conf.data = BuildWrappedObject<ShouldInterceptData>(isolate, &intercept_data);
+  interceptor_templ->SetHandler(conf);
+
+  auto interceptor = interceptor_templ->NewInstance();
+  context->Global()->Set(v8_str("obj"), interceptor);
+
+  CompileRun(
+      "outer = {};"
+      "outer.__proto__ = obj;"
+      "function f(obj) {"
+      "  var x;"
+      "  for (var i = 0; i < 4; i++) {"
+      "    x = obj.whatever;"
+      "  }"
+      "  return x;"
+      "}");
+
+  // Receiver == holder.
+  CompileRun("obj.__proto__ = null;");
+  ExpectInt32("f(obj)", 239);
+  ExpectInt32("f(outer)", 239);
+
+  // Receiver != holder.
+  CompileRun("Object.setPrototypeOf(obj, {});");
+  ExpectInt32("f(obj)", 239);
+  ExpectInt32("f(outer)", 239);
+
+  // Masked value on prototype.
+  CompileRun("obj.__proto__.whatever = 4;");
+  CompileRun("obj.__proto__.__proto__ = { 'whatever' : 5 };");
+  ExpectInt32("f(obj)", 4);
+  ExpectInt32("f(outer)", 4);
+
+  // Masked value on prototype prototype.
+  CompileRun("delete obj.__proto__.whatever;");
+  ExpectInt32("f(obj)", 5);
+  ExpectInt32("f(outer)", 5);
+
+  // Reset.
+  CompileRun("delete obj.__proto__.__proto__.whatever;");
+  ExpectInt32("f(obj)", 239);
+  ExpectInt32("f(outer)", 239);
+
+  // Masked value on self.
+  CompileRun("obj.whatever = 4;");
+  ExpectInt32("f(obj)", 4);
+  ExpectInt32("f(outer)", 4);
+
+  // Reset.
+  CompileRun("delete obj.whatever;");
+  ExpectInt32("f(obj)", 239);
+  ExpectInt32("f(outer)", 239);
+
+  CompileRun("outer.whatever = 4;");
+  ExpectInt32("f(obj)", 239);
+  ExpectInt32("f(outer)", 4);
+}