[debug] use flag to decide whether interceptor has side effect

Adds a flag onto InterceptorInfo to mark an interceptor's getter,
query, and enumerator callbacks as side-effect-free.

Bug: v8:7515
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Iafc5d2fa554d6d9a38604e179ea5b884c3b77af0
Reviewed-on: https://chromium-review.googlesource.com/957870
Commit-Queue: Erik Luo <luoe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51969}

src/api-arguments-inl.h

@@ -13,9 +13,9 @@
 namespace v8 {
 namespace internal {
 
-#define FOR_EACH_CALLBACK(F)           \
-  F(Query, query, Object, v8::Integer) \
-  F(Deleter, deleter, Object, v8::Boolean)
+#define FOR_EACH_CALLBACK(F)                        \
+  F(Query, query, Object, v8::Integer, interceptor) \
+  F(Deleter, deleter, Object, v8::Boolean, Handle<Object>())
 
 #define DCHECK_NAME_COMPATIBLE(interceptor, name) \
   DCHECK(interceptor->is_named());                \
@@ -32,44 +32,44 @@ namespace internal {
   ExternalCallbackScope call_scope(ISOLATE, FUNCTION_ADDR(F));           \
   PropertyCallbackInfo<API_RETURN_TYPE> callback_info(begin());
 
-#define CREATE_NAMED_CALLBACK(Function, type, ReturnType, ApiReturnType)      \
-  Handle<ReturnType> PropertyCallbackArguments::CallNamed##Function(          \
+#define CREATE_NAMED_CALLBACK(FUNCTION, TYPE, RETURN_TYPE, API_RETURN_TYPE,   \
+                              INFO_FOR_SIDE_EFFECT)                           \
+  Handle<RETURN_TYPE> PropertyCallbackArguments::CallNamed##FUNCTION(         \
       Handle<InterceptorInfo> interceptor, Handle<Name> name) {               \
     DCHECK_NAME_COMPATIBLE(interceptor, name);                                \
     Isolate* isolate = this->isolate();                                       \
     RuntimeCallTimerScope timer(                                              \
-        isolate, RuntimeCallCounterId::kNamed##Function##Callback);           \
-    GenericNamedProperty##Function##Callback f =                              \
-        ToCData<GenericNamedProperty##Function##Callback>(                    \
-            interceptor->type());                                             \
-    Handle<Object> side_effect_check_not_supported;                           \
-    PREPARE_CALLBACK_INFO(isolate, f, Handle<ReturnType>, ApiReturnType,      \
-                          side_effect_check_not_supported);                   \
+        isolate, RuntimeCallCounterId::kNamed##FUNCTION##Callback);           \
+    GenericNamedProperty##FUNCTION##Callback f =                              \
+        ToCData<GenericNamedProperty##FUNCTION##Callback>(                    \
+            interceptor->TYPE());                                             \
+    PREPARE_CALLBACK_INFO(isolate, f, Handle<RETURN_TYPE>, API_RETURN_TYPE,   \
+                          INFO_FOR_SIDE_EFFECT);                              \
     LOG(isolate,                                                              \
-        ApiNamedPropertyAccess("interceptor-named-" #type, holder(), *name)); \
+        ApiNamedPropertyAccess("interceptor-named-" #TYPE, holder(), *name)); \
     f(v8::Utils::ToLocal(name), callback_info);                               \
-    return GetReturnValue<ReturnType>(isolate);                               \
+    return GetReturnValue<RETURN_TYPE>(isolate);                              \
   }
 
 FOR_EACH_CALLBACK(CREATE_NAMED_CALLBACK)
 #undef CREATE_NAMED_CALLBACK
 
-#define CREATE_INDEXED_CALLBACK(Function, type, ReturnType, ApiReturnType) \
-  Handle<ReturnType> PropertyCallbackArguments::CallIndexed##Function(     \
-      Handle<InterceptorInfo> interceptor, uint32_t index) {               \
-    DCHECK(!interceptor->is_named());                                      \
-    Isolate* isolate = this->isolate();                                    \
-    RuntimeCallTimerScope timer(                                           \
-        isolate, RuntimeCallCounterId::kIndexed##Function##Callback);      \
-    IndexedProperty##Function##Callback f =                                \
-        ToCData<IndexedProperty##Function##Callback>(interceptor->type()); \
-    Handle<Object> side_effect_check_not_supported;                        \
-    PREPARE_CALLBACK_INFO(isolate, f, Handle<ReturnType>, ApiReturnType,   \
-                          side_effect_check_not_supported);                \
-    LOG(isolate, ApiIndexedPropertyAccess("interceptor-indexed-" #type,    \
-                                          holder(), index));               \
-    f(index, callback_info);                                               \
-    return GetReturnValue<ReturnType>(isolate);                            \
+#define CREATE_INDEXED_CALLBACK(FUNCTION, TYPE, RETURN_TYPE, API_RETURN_TYPE, \
+                                INFO_FOR_SIDE_EFFECT)                         \
+  Handle<RETURN_TYPE> PropertyCallbackArguments::CallIndexed##FUNCTION(       \
+      Handle<InterceptorInfo> interceptor, uint32_t index) {                  \
+    DCHECK(!interceptor->is_named());                                         \
+    Isolate* isolate = this->isolate();                                       \
+    RuntimeCallTimerScope timer(                                              \
+        isolate, RuntimeCallCounterId::kIndexed##FUNCTION##Callback);         \
+    IndexedProperty##FUNCTION##Callback f =                                   \
+        ToCData<IndexedProperty##FUNCTION##Callback>(interceptor->TYPE());    \
+    PREPARE_CALLBACK_INFO(isolate, f, Handle<RETURN_TYPE>, API_RETURN_TYPE,   \
+                          INFO_FOR_SIDE_EFFECT);                              \
+    LOG(isolate, ApiIndexedPropertyAccess("interceptor-indexed-" #TYPE,       \
+                                          holder(), index));                  \
+    f(index, callback_info);                                                  \
+    return GetReturnValue<RETURN_TYPE>(isolate);                              \
   }
 
 FOR_EACH_CALLBACK(CREATE_INDEXED_CALLBACK)
@@ -87,8 +87,7 @@ Handle<Object> PropertyCallbackArguments::CallNamedGetter(
       ApiNamedPropertyAccess("interceptor-named-getter", holder(), *name));
   GenericNamedPropertyGetterCallback f =
       ToCData<GenericNamedPropertyGetterCallback>(interceptor->getter());
-  Handle<Object> side_effect_check_not_supported;
-  return BasicCallNamedGetterCallback(f, name, side_effect_check_not_supported);
+  return BasicCallNamedGetterCallback(f, name, interceptor);
 }
 
 Handle<Object> PropertyCallbackArguments::CallNamedDescriptor(
@@ -102,8 +101,7 @@ Handle<Object> PropertyCallbackArguments::CallNamedDescriptor(
   GenericNamedPropertyDescriptorCallback f =
       ToCData<GenericNamedPropertyDescriptorCallback>(
           interceptor->descriptor());
-  Handle<Object> side_effect_check_not_supported;
-  return BasicCallNamedGetterCallback(f, name, side_effect_check_not_supported);
+  return BasicCallNamedGetterCallback(f, name, interceptor);
 }
 
 Handle<Object> PropertyCallbackArguments::BasicCallNamedGetterCallback(
@@ -204,7 +202,7 @@ Handle<Object> PropertyCallbackArguments::CallIndexedGetter(
       ApiIndexedPropertyAccess("interceptor-indexed-getter", holder(), index));
   IndexedPropertyGetterCallback f =
       ToCData<IndexedPropertyGetterCallback>(interceptor->getter());
-  return BasicCallIndexedGetterCallback(f, index);
+  return BasicCallIndexedGetterCallback(f, index, interceptor);
 }
 
 Handle<Object> PropertyCallbackArguments::CallIndexedDescriptor(
@@ -217,15 +215,13 @@ Handle<Object> PropertyCallbackArguments::CallIndexedDescriptor(
                                         holder(), index));
   IndexedPropertyDescriptorCallback f =
       ToCData<IndexedPropertyDescriptorCallback>(interceptor->descriptor());
-  return BasicCallIndexedGetterCallback(f, index);
+  return BasicCallIndexedGetterCallback(f, index, interceptor);
 }
 
 Handle<Object> PropertyCallbackArguments::BasicCallIndexedGetterCallback(
-    IndexedPropertyGetterCallback f, uint32_t index) {
+    IndexedPropertyGetterCallback f, uint32_t index, Handle<Object> info) {
   Isolate* isolate = this->isolate();
-  Handle<Object> side_effect_check_not_supported;
-  PREPARE_CALLBACK_INFO(isolate, f, Handle<Object>, v8::Value,
-                        side_effect_check_not_supported);
+  PREPARE_CALLBACK_INFO(isolate, f, Handle<Object>, v8::Value, info);
   f(index, callback_info);
   return GetReturnValue<Object>(isolate);
 }
@@ -237,9 +233,7 @@ Handle<JSObject> PropertyCallbackArguments::CallPropertyEnumerator(
       v8::ToCData<IndexedPropertyEnumeratorCallback>(interceptor->enumerator());
   // TODO(cbruni): assert same type for indexed and named callback.
   Isolate* isolate = this->isolate();
-  Handle<Object> side_effect_check_not_supported;
-  PREPARE_CALLBACK_INFO(isolate, f, Handle<JSObject>, v8::Array,
-                        side_effect_check_not_supported);
+  PREPARE_CALLBACK_INFO(isolate, f, Handle<JSObject>, v8::Array, interceptor);
   f(callback_info);
   return GetReturnValue<JSObject>(isolate);
 }

src/api-arguments.h

@@ -158,7 +158,7 @@ class PropertyCallbackArguments
       Handle<InterceptorInfo> interceptor);
 
   inline Handle<Object> BasicCallIndexedGetterCallback(
-      IndexedPropertyGetterCallback f, uint32_t index);
+      IndexedPropertyGetterCallback f, uint32_t index, Handle<Object> info);
   inline Handle<Object> BasicCallNamedGetterCallback(
       GenericNamedPropertyGetterCallback f, Handle<Name> name,
       Handle<Object> info);

src/debug/debug-evaluate.cc

@@ -937,6 +937,12 @@ bool DebugEvaluate::CallbackHasNoSideEffect(Object* callback_info) {
       info->name()->ShortPrint();
       PrintF("' may cause side effect.\n");
     }
+  } else if (callback_info->IsInterceptorInfo()) {
+    InterceptorInfo* info = InterceptorInfo::cast(callback_info);
+    if (info->has_no_side_effect()) return true;
+    if (FLAG_trace_side_effect_free_debug_evaluate) {
+      PrintF("[debug-evaluate] API Interceptor may cause side effect.\n");
+    }
   }
   return false;
 }

src/objects-inl.h

@@ -2401,6 +2401,7 @@ BOOL_ACCESSORS(InterceptorInfo, flags, can_intercept_symbols,
 BOOL_ACCESSORS(InterceptorInfo, flags, all_can_read, kAllCanReadBit)
 BOOL_ACCESSORS(InterceptorInfo, flags, non_masking, kNonMasking)
 BOOL_ACCESSORS(InterceptorInfo, flags, is_named, kNamed)
+BOOL_ACCESSORS(InterceptorInfo, flags, has_no_side_effect, kHasNoSideEffect)
 
 ACCESSORS(CallHandlerInfo, callback, Object, kCallbackOffset)
 ACCESSORS(CallHandlerInfo, js_callback, Object, kJsCallbackOffset)

src/objects.h

@@ -4557,6 +4557,7 @@ class InterceptorInfo: public Struct {
   DECL_BOOLEAN_ACCESSORS(all_can_read)
   DECL_BOOLEAN_ACCESSORS(non_masking)
   DECL_BOOLEAN_ACCESSORS(is_named)
+  DECL_BOOLEAN_ACCESSORS(has_no_side_effect)
 
   inline int flags() const;
   inline void set_flags(int flags);
@@ -4582,6 +4583,7 @@ class InterceptorInfo: public Struct {
   static const int kAllCanReadBit = 1;
   static const int kNonMasking = 2;
   static const int kNamed = 3;
+  static const int kHasNoSideEffect = 4;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(InterceptorInfo);

test/cctest/test-api-interceptors.cc

@@ -64,6 +64,14 @@ void EmptyInterceptorGetter(Local<Name> name,
 void EmptyInterceptorSetter(Local<Name> name, Local<Value> value,
                             const v8::PropertyCallbackInfo<v8::Value>& info) {}
 
+void EmptyInterceptorQuery(Local<Name> name,
+                           const v8::PropertyCallbackInfo<v8::Integer>& info) {}
+
+void EmptyInterceptorDeleter(
+    Local<Name> name, const v8::PropertyCallbackInfo<v8::Boolean>& info) {}
+
+void EmptyInterceptorEnumerator(
+    const v8::PropertyCallbackInfo<v8::Array>& info) {}
 
 void SimpleAccessorGetter(Local<String> name,
                           const v8::PropertyCallbackInfo<v8::Value>& info) {
@@ -2587,6 +2595,47 @@ static void InterceptorForHiddenProperties(
   interceptor_for_hidden_properties_called = true;
 }
 
+THREADED_TEST(NoSideEffectPropertyHandler) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
+  templ->SetHandler(v8::NamedPropertyHandlerConfiguration(
+      EmptyInterceptorGetter, EmptyInterceptorSetter, EmptyInterceptorQuery,
+      EmptyInterceptorDeleter, EmptyInterceptorEnumerator));
+
+  LocalContext context;
+  v8::Local<v8::Object> object =
+      templ->NewInstance(context.local()).ToLocalChecked();
+  context->Global()->Set(context.local(), v8_str("obj"), object).FromJust();
+
+  CHECK(v8::debug::EvaluateGlobal(isolate, v8_str("obj.x"), true).IsEmpty());
+  CHECK(
+      v8::debug::EvaluateGlobal(isolate, v8_str("obj.x = 1"), true).IsEmpty());
+  CHECK(
+      v8::debug::EvaluateGlobal(isolate, v8_str("'x' in obj"), true).IsEmpty());
+  CHECK(v8::debug::EvaluateGlobal(isolate, v8_str("delete obj.x"), true)
+            .IsEmpty());
+  // Wrap the variable declaration since declaring globals is a side effect.
+  CHECK(v8::debug::EvaluateGlobal(
+            isolate, v8_str("(function() { for (var p in obj) ; })()"), true)
+            .IsEmpty());
+
+  // TODO(luoe): turn on has_no_side_effect flag from API once it is exposed.
+  i::Handle<i::JSObject> internal_object =
+      i::Handle<i::JSObject>::cast(v8::Utils::OpenHandle(*object));
+  internal_object->GetNamedInterceptor()->set_has_no_side_effect(true);
+
+  v8::debug::EvaluateGlobal(isolate, v8_str("obj.x"), true).ToLocalChecked();
+  CHECK(
+      v8::debug::EvaluateGlobal(isolate, v8_str("obj.x = 1"), true).IsEmpty());
+  v8::debug::EvaluateGlobal(isolate, v8_str("'x' in obj"), true)
+      .ToLocalChecked();
+  CHECK(v8::debug::EvaluateGlobal(isolate, v8_str("delete obj.x"), true)
+            .IsEmpty());
+  v8::debug::EvaluateGlobal(
+      isolate, v8_str("(function() { for (var p in obj) ; })()"), true)
+      .ToLocalChecked();
+}
 
 THREADED_TEST(HiddenPropertiesWithInterceptors) {
   LocalContext context;