[debug-evaluate] extend accessors by runtime receiver checks

Also extend the API to reflect this new feature.


R=jgruber@chromium.org, szuend@google.com, ulan@chromium.org

Bug: v8:8125
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Ic7a7604a8c663ba04b324eb8902ff325a25654e7
Reviewed-on: https://chromium-review.googlesource.com/1202087Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55604}

include/v8.h

@@ -3279,10 +3279,17 @@ enum PropertyFilter {
  * Options for marking whether callbacks may trigger JS-observable side effects.
  * Side-effect-free callbacks are whitelisted during debug evaluation with
  * throwOnSideEffect. It applies when calling a Function, FunctionTemplate,
- * or an Accessor's getter callback. For Interceptors, please see
+ * or an Accessor callback. For Interceptors, please see
  * PropertyHandlerFlags's kHasNoSideEffect.
+ * Callbacks that only cause side effects to the receiver are whitelisted if
+ * invoked on receiver objects that are created within the same debug-evaluate
+ * call, as these objects are temporary and the side effect does not escape.
  */
-enum class SideEffectType { kHasSideEffect, kHasNoSideEffect };
+enum class SideEffectType {
+  kHasSideEffect,
+  kHasNoSideEffect,
+  kHasSideEffectToReceiver
+};
 
 /**
  * Keys/Properties filter enums:
@@ -3423,7 +3430,8 @@ class V8_EXPORT Object : public Value {
       AccessorNameGetterCallback getter, AccessorNameSetterCallback setter = 0,
       MaybeLocal<Value> data = MaybeLocal<Value>(),
       AccessControl settings = DEFAULT, PropertyAttribute attribute = None,
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
 
   void SetAccessorProperty(Local<Name> name, Local<Function> getter,
                            Local<Function> setter = Local<Function>(),
@@ -3439,7 +3447,8 @@ class V8_EXPORT Object : public Value {
       AccessorNameGetterCallback getter,
       AccessorNameSetterCallback setter = nullptr,
       Local<Value> data = Local<Value>(), PropertyAttribute attributes = None,
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
 
   /**
    * Attempts to create a property with the given name which behaves like a data
@@ -3453,7 +3462,8 @@ class V8_EXPORT Object : public Value {
       Local<Context> context, Local<Name> name,
       AccessorNameGetterCallback getter, Local<Value> data = Local<Value>(),
       PropertyAttribute attributes = None,
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
 
   /**
    * Functionality for private properties.
@@ -5388,7 +5398,8 @@ class V8_EXPORT Template : public Data {
       Local<Value> data = Local<Value>(), PropertyAttribute attribute = None,
       Local<AccessorSignature> signature = Local<AccessorSignature>(),
       AccessControl settings = DEFAULT,
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
   void SetNativeDataProperty(
       Local<Name> name, AccessorNameGetterCallback getter,
       AccessorNameSetterCallback setter = 0,
@@ -5396,7 +5407,8 @@ class V8_EXPORT Template : public Data {
       Local<Value> data = Local<Value>(), PropertyAttribute attribute = None,
       Local<AccessorSignature> signature = Local<AccessorSignature>(),
       AccessControl settings = DEFAULT,
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
 
   /**
    * Like SetNativeDataProperty, but V8 will replace the native data property
@@ -5405,7 +5417,8 @@ class V8_EXPORT Template : public Data {
   void SetLazyDataProperty(
       Local<Name> name, AccessorNameGetterCallback getter,
       Local<Value> data = Local<Value>(), PropertyAttribute attribute = None,
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
 
   /**
    * During template instantiation, sets the value with the intrinsic property
@@ -6120,13 +6133,15 @@ class V8_EXPORT ObjectTemplate : public Template {
       AccessorSetterCallback setter = 0, Local<Value> data = Local<Value>(),
       AccessControl settings = DEFAULT, PropertyAttribute attribute = None,
       Local<AccessorSignature> signature = Local<AccessorSignature>(),
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
   void SetAccessor(
       Local<Name> name, AccessorNameGetterCallback getter,
       AccessorNameSetterCallback setter = 0, Local<Value> data = Local<Value>(),
       AccessControl settings = DEFAULT, PropertyAttribute attribute = None,
       Local<AccessorSignature> signature = Local<AccessorSignature>(),
-      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect);
+      SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect,
+      SideEffectType setter_side_effect_type = SideEffectType::kHasSideEffect);
 
   /**
    * Sets a named property handler on the object template.

src/accessors.cc

@@ -31,7 +31,8 @@ Handle<AccessorInfo> Accessors::MakeAccessor(
   info->set_is_special_data_property(true);
   info->set_is_sloppy(false);
   info->set_replace_on_access(false);
-  info->set_has_no_side_effect(false);
+  info->set_getter_side_effect_type(SideEffectType::kHasSideEffect);
+  info->set_setter_side_effect_type(SideEffectType::kHasSideEffect);
   name = factory->InternalizeName(name);
   info->set_name(*name);
   Handle<Object> get = v8::FromCData(isolate, getter);

src/accessors.h

@@ -22,27 +22,27 @@ class JavaScriptFrame;
 
 // The list of accessor descriptors. This is a second-order macro
 // taking a macro to be applied to all accessor descriptor names.
-#define ACCESSOR_INFO_LIST(V)                                       \
-  V(arguments_iterator, ArgumentsIterator)                          \
-  V(array_length, ArrayLength)                                      \
-  V(bound_function_length, BoundFunctionLength)                     \
-  V(bound_function_name, BoundFunctionName)                         \
-  V(error_stack, ErrorStack)                                        \
-  V(function_arguments, FunctionArguments)                          \
-  V(function_caller, FunctionCaller)                                \
-  V(function_name, FunctionName)                                    \
-  V(function_length, FunctionLength)                                \
-  V(function_prototype, FunctionPrototype)                          \
-  V(string_length, StringLength)
-
-#define SIDE_EFFECT_FREE_ACCESSOR_INFO_LIST(V) \
-  V(ArrayLength)                               \
-  V(BoundFunctionLength)                       \
-  V(BoundFunctionName)                         \
-  V(FunctionName)                              \
-  V(FunctionLength)                            \
-  V(FunctionPrototype)                         \
-  V(StringLength)
+// V(accessor_name, AccessorName, GetterSideEffectType, SetterSideEffectType)
+#define ACCESSOR_INFO_LIST(V)                                                \
+  V(arguments_iterator, ArgumentsIterator, kHasNoSideEffect,                 \
+    kHasSideEffectToReceiver)                                                \
+  V(array_length, ArrayLength, kHasNoSideEffect, kHasSideEffectToReceiver)   \
+  V(bound_function_length, BoundFunctionLength, kHasNoSideEffect,            \
+    kHasSideEffectToReceiver)                                                \
+  V(bound_function_name, BoundFunctionName, kHasNoSideEffect,                \
+    kHasSideEffectToReceiver)                                                \
+  V(error_stack, ErrorStack, kHasSideEffectToReceiver,                       \
+    kHasSideEffectToReceiver)                                                \
+  V(function_arguments, FunctionArguments, kHasNoSideEffect,                 \
+    kHasSideEffectToReceiver)                                                \
+  V(function_caller, FunctionCaller, kHasNoSideEffect,                       \
+    kHasSideEffectToReceiver)                                                \
+  V(function_name, FunctionName, kHasNoSideEffect, kHasSideEffectToReceiver) \
+  V(function_length, FunctionLength, kHasNoSideEffect,                       \
+    kHasSideEffectToReceiver)                                                \
+  V(function_prototype, FunctionPrototype, kHasNoSideEffect,                 \
+    kHasSideEffectToReceiver)                                                \
+  V(string_length, StringLength, kHasNoSideEffect, kHasSideEffectToReceiver)
 
 #define ACCESSOR_SETTER_LIST(V) \
   V(ArrayLengthSetter)          \
@@ -55,9 +55,9 @@ class JavaScriptFrame;
 
 class Accessors : public AllStatic {
  public:
-#define ACCESSOR_GETTER_DECLARATION(accessor_name, AccessorName) \
-  static void AccessorName##Getter(                              \
-      v8::Local<v8::Name> name,                                  \
+#define ACCESSOR_GETTER_DECLARATION(accessor_name, AccessorName, ...) \
+  static void AccessorName##Getter(                                   \
+      v8::Local<v8::Name> name,                                       \
       const v8::PropertyCallbackInfo<v8::Value>& info);
   ACCESSOR_INFO_LIST(ACCESSOR_GETTER_DECLARATION)
 #undef ACCESSOR_GETTER_DECLARATION
@@ -118,7 +118,7 @@ class Accessors : public AllStatic {
       AccessorNameBooleanSetterCallback setter);
 
  private:
-#define ACCESSOR_INFO_DECLARATION(accessor_name, AccessorName) \
+#define ACCESSOR_INFO_DECLARATION(accessor_name, AccessorName, ...) \
   static Handle<AccessorInfo> Make##AccessorName##Info(Isolate* isolate);
   ACCESSOR_INFO_LIST(ACCESSOR_INFO_DECLARATION)
 #undef ACCESSOR_INFO_DECLARATION

src/api-arguments.h

@@ -133,9 +133,10 @@ class PropertyCallbackArguments
       IndexedPropertyGetterCallback f, uint32_t index, Handle<Object> info);
   inline Handle<Object> BasicCallNamedGetterCallback(
       GenericNamedPropertyGetterCallback f, Handle<Name> name,
-      Handle<Object> info);
+      Handle<Object> info, Handle<Object> receiver = Handle<Object>());
 
   inline JSObject* holder();
+  inline Object* receiver();
 
   // Don't copy PropertyCallbackArguments, because they would both have the
   // same prev_ pointer.

src/api.cc

@@ -1676,7 +1676,8 @@ static void TemplateSetAccessor(
     Template* template_obj, v8::Local<Name> name, Getter getter, Setter setter,
     Data data, AccessControl settings, PropertyAttribute attribute,
     v8::Local<AccessorSignature> signature, bool is_special_data_property,
-    bool replace_on_access, SideEffectType getter_side_effect_type) {
+    bool replace_on_access, SideEffectType getter_side_effect_type,
+    SideEffectType setter_side_effect_type) {
   auto info = Utils::OpenHandle(template_obj);
   auto isolate = info->GetIsolate();
   ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
@@ -1686,8 +1687,8 @@ static void TemplateSetAccessor(
                        is_special_data_property, replace_on_access);
   accessor_info->set_initial_property_attributes(
       static_cast<i::PropertyAttributes>(attribute));
-  accessor_info->set_has_no_side_effect(getter_side_effect_type ==
-                                        SideEffectType::kHasNoSideEffect);
+  accessor_info->set_getter_side_effect_type(getter_side_effect_type);
+  accessor_info->set_setter_side_effect_type(setter_side_effect_type);
   i::ApiNatives::AddNativeDataProperty(isolate, info, accessor_info);
 }
 
@@ -1695,29 +1696,34 @@ void Template::SetNativeDataProperty(
     v8::Local<String> name, AccessorGetterCallback getter,
     AccessorSetterCallback setter, v8::Local<Value> data,
     PropertyAttribute attribute, v8::Local<AccessorSignature> signature,
-    AccessControl settings, SideEffectType getter_side_effect_type) {
+    AccessControl settings, SideEffectType getter_side_effect_type,
+    SideEffectType setter_side_effect_type) {
   TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
-                      signature, true, false, getter_side_effect_type);
+                      signature, true, false, getter_side_effect_type,
+                      setter_side_effect_type);
 }
 
 void Template::SetNativeDataProperty(
     v8::Local<Name> name, AccessorNameGetterCallback getter,
     AccessorNameSetterCallback setter, v8::Local<Value> data,
     PropertyAttribute attribute, v8::Local<AccessorSignature> signature,
-    AccessControl settings, SideEffectType getter_side_effect_type) {
+    AccessControl settings, SideEffectType getter_side_effect_type,
+    SideEffectType setter_side_effect_type) {
   TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
-                      signature, true, false, getter_side_effect_type);
+                      signature, true, false, getter_side_effect_type,
+                      setter_side_effect_type);
 }
 
 void Template::SetLazyDataProperty(v8::Local<Name> name,
                                    AccessorNameGetterCallback getter,
                                    v8::Local<Value> data,
                                    PropertyAttribute attribute,
-                                   SideEffectType getter_side_effect_type) {
+                                   SideEffectType getter_side_effect_type,
+                                   SideEffectType setter_side_effect_type) {
   TemplateSetAccessor(this, name, getter,
                       static_cast<AccessorNameSetterCallback>(nullptr), data,
                       DEFAULT, attribute, Local<AccessorSignature>(), true,
-                      true, getter_side_effect_type);
+                      true, getter_side_effect_type, setter_side_effect_type);
 }
 
 void Template::SetIntrinsicDataProperty(Local<Name> name, Intrinsic intrinsic,
@@ -1737,10 +1743,11 @@ void ObjectTemplate::SetAccessor(v8::Local<String> name,
                                  v8::Local<Value> data, AccessControl settings,
                                  PropertyAttribute attribute,
                                  v8::Local<AccessorSignature> signature,
-                                 SideEffectType getter_side_effect_type) {
+                                 SideEffectType getter_side_effect_type,
+                                 SideEffectType setter_side_effect_type) {
   TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
                       signature, i::FLAG_disable_old_api_accessors, false,
-                      getter_side_effect_type);
+                      getter_side_effect_type, setter_side_effect_type);
 }
 
 void ObjectTemplate::SetAccessor(v8::Local<Name> name,
@@ -1749,10 +1756,11 @@ void ObjectTemplate::SetAccessor(v8::Local<Name> name,
                                  v8::Local<Value> data, AccessControl settings,
                                  PropertyAttribute attribute,
                                  v8::Local<AccessorSignature> signature,
-                                 SideEffectType getter_side_effect_type) {
+                                 SideEffectType getter_side_effect_type,
+                                 SideEffectType setter_side_effect_type) {
   TemplateSetAccessor(this, name, getter, setter, data, settings, attribute,
                       signature, i::FLAG_disable_old_api_accessors, false,
-                      getter_side_effect_type);
+                      getter_side_effect_type, setter_side_effect_type);
 }
 
 template <typename Getter, typename Setter, typename Query, typename Descriptor,
@@ -4569,8 +4577,8 @@ static Maybe<bool> ObjectSetAccessor(
     Local<Context> context, Object* self, Local<Name> name, Getter getter,
     Setter setter, Data data, AccessControl settings,
     PropertyAttribute attributes, bool is_special_data_property,
-    bool replace_on_access,
-    SideEffectType getter_side_effect_type = SideEffectType::kHasSideEffect) {
+    bool replace_on_access, SideEffectType getter_side_effect_type,
+    SideEffectType setter_side_effect_type) {
   auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
   ENTER_V8_NO_SCRIPT(isolate, context, Object, SetAccessor, Nothing<bool>(),
                      i::HandleScope);
@@ -4581,8 +4589,8 @@ static Maybe<bool> ObjectSetAccessor(
   i::Handle<i::AccessorInfo> info =
       MakeAccessorInfo(isolate, name, getter, setter, data, settings, signature,
                        is_special_data_property, replace_on_access);
-  info->set_has_no_side_effect(getter_side_effect_type ==
-                               SideEffectType::kHasNoSideEffect);
+  info->set_getter_side_effect_type(getter_side_effect_type);
+  info->set_setter_side_effect_type(setter_side_effect_type);
   if (info.is_null()) return Nothing<bool>();
   bool fast = obj->HasFastProperties();
   i::Handle<i::Object> result;
@@ -4605,11 +4613,12 @@ Maybe<bool> Object::SetAccessor(Local<Context> context, Local<Name> name,
                                 AccessorNameSetterCallback setter,
                                 MaybeLocal<Value> data, AccessControl settings,
                                 PropertyAttribute attribute,
-                                SideEffectType getter_side_effect_type) {
+                                SideEffectType getter_side_effect_type,
+                                SideEffectType setter_side_effect_type) {
   return ObjectSetAccessor(context, this, name, getter, setter,
                            data.FromMaybe(Local<Value>()), settings, attribute,
                            i::FLAG_disable_old_api_accessors, false,
-                           getter_side_effect_type);
+                           getter_side_effect_type, setter_side_effect_type);
 }
 
 
@@ -4636,19 +4645,22 @@ Maybe<bool> Object::SetNativeDataProperty(
     v8::Local<v8::Context> context, v8::Local<Name> name,
     AccessorNameGetterCallback getter, AccessorNameSetterCallback setter,
     v8::Local<Value> data, PropertyAttribute attributes,
-    SideEffectType getter_side_effect_type) {
+    SideEffectType getter_side_effect_type,
+    SideEffectType setter_side_effect_type) {
   return ObjectSetAccessor(context, this, name, getter, setter, data, DEFAULT,
-                           attributes, true, false, getter_side_effect_type);
+                           attributes, true, false, getter_side_effect_type,
+                           setter_side_effect_type);
 }
 
 Maybe<bool> Object::SetLazyDataProperty(
     v8::Local<v8::Context> context, v8::Local<Name> name,
     AccessorNameGetterCallback getter, v8::Local<Value> data,
-    PropertyAttribute attributes, SideEffectType getter_side_effect_type) {
+    PropertyAttribute attributes, SideEffectType getter_side_effect_type,
+    SideEffectType setter_side_effect_type) {
   return ObjectSetAccessor(context, this, name, getter,
                            static_cast<AccessorNameSetterCallback>(nullptr),
                            data, DEFAULT, attributes, true, true,
-                           getter_side_effect_type);
+                           getter_side_effect_type, setter_side_effect_type);
 }
 
 Maybe<bool> v8::Object::HasOwnProperty(Local<Context> context,
@@ -9684,7 +9696,7 @@ int debug::GetNativeAccessorDescriptor(v8::Local<v8::Context> context,
   }
   auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
   int result = 0;
-#define IS_BUILTIN_ACESSOR(name, _)                         \
+#define IS_BUILTIN_ACESSOR(name, ...)                       \
   if (*structure == *isolate->factory()->name##_accessor()) \
     result |= static_cast<int>(debug::NativeAccessorType::IsBuiltin);
   ACCESSOR_INFO_LIST(IS_BUILTIN_ACESSOR)

src/debug/debug-evaluate.cc

@@ -16,7 +16,6 @@
 #include "src/interpreter/bytecode-array-iterator.h"
 #include "src/interpreter/bytecodes.h"
 #include "src/isolate-inl.h"
-#include "src/objects/api-callbacks.h"
 #include "src/snapshot/snapshot.h"
 
 namespace v8 {
@@ -796,7 +795,10 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) {
     case Builtins::kMakeURIError:
     // RegExp builtins.
     case Builtins::kRegExpConstructor:
+    // Internal.
+    case Builtins::kStrictPoisonPillThrower:
       return DebugInfo::kHasNoSideEffect;
+
     // Set builtins.
     case Builtins::kSetIteratorPrototypeNext:
     case Builtins::kSetPrototypeAdd:
@@ -950,34 +952,6 @@ DebugInfo::SideEffectState DebugEvaluate::FunctionGetSideEffectState(
   return DebugInfo::kHasSideEffects;
 }
 
-// static
-bool DebugEvaluate::CallbackHasNoSideEffect(Object* callback_info) {
-  DisallowHeapAllocation no_gc;
-  if (callback_info->IsAccessorInfo()) {
-    // List of whitelisted internal accessors can be found in accessors.h.
-    AccessorInfo* info = AccessorInfo::cast(callback_info);
-    if (info->has_no_side_effect()) return true;
-    if (FLAG_trace_side_effect_free_debug_evaluate) {
-      PrintF("[debug-evaluate] API Callback '");
-      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");
-    }
-  } else if (callback_info->IsCallHandlerInfo()) {
-    CallHandlerInfo* info = CallHandlerInfo::cast(callback_info);
-    if (info->IsSideEffectFreeCallHandlerInfo()) return true;
-    if (FLAG_trace_side_effect_free_debug_evaluate) {
-      PrintF("[debug-evaluate] API CallHandlerInfo may cause side effect.\n");
-    }
-  }
-  return false;
-}
-
 // static
 void DebugEvaluate::ApplySideEffectChecks(
     Handle<BytecodeArray> bytecode_array) {

src/debug/debug-evaluate.h

@@ -41,7 +41,6 @@ class DebugEvaluate : public AllStatic {
 
   static DebugInfo::SideEffectState FunctionGetSideEffectState(
       Isolate* isolate, Handle<SharedFunctionInfo> info);
-  static bool CallbackHasNoSideEffect(Object* callback_info);
   static void ApplySideEffectChecks(Handle<BytecodeArray> bytecode_array);
 
  private:

src/debug/debug.cc

@@ -29,6 +29,7 @@
 #include "src/isolate-inl.h"
 #include "src/log.h"
 #include "src/messages.h"
+#include "src/objects/api-callbacks-inl.h"
 #include "src/objects/debug-objects-inl.h"
 #include "src/objects/js-generator-inl.h"
 #include "src/objects/js-promise-inl.h"
@@ -2180,16 +2181,55 @@ Handle<Object> Debug::return_value_handle() {
   return handle(thread_local_.return_value_, isolate_);
 }
 
-bool Debug::PerformSideEffectCheckForCallback(Handle<Object> callback_info) {
+bool Debug::PerformSideEffectCheckForCallback(
+    Handle<Object> callback_info, Handle<Object> receiver,
+    Debug::AccessorKind accessor_kind) {
+  DCHECK_EQ(!receiver.is_null(), callback_info->IsAccessorInfo());
   DCHECK_EQ(isolate_->debug_execution_mode(), DebugInfo::kSideEffects);
   if (!callback_info.is_null() && callback_info->IsCallHandlerInfo() &&
       i::CallHandlerInfo::cast(*callback_info)->NextCallHasNoSideEffect()) {
     return true;
   }
   // TODO(7515): always pass a valid callback info object.
-  if (!callback_info.is_null() &&
-      DebugEvaluate::CallbackHasNoSideEffect(*callback_info)) {
-    return true;
+  if (!callback_info.is_null()) {
+    if (callback_info->IsAccessorInfo()) {
+      // List of whitelisted internal accessors can be found in accessors.h.
+      AccessorInfo* info = AccessorInfo::cast(*callback_info);
+      DCHECK_NE(kNotAccessor, accessor_kind);
+      switch (accessor_kind == kSetter ? info->setter_side_effect_type()
+                                       : info->getter_side_effect_type()) {
+        case SideEffectType::kHasNoSideEffect:
+          // We do not support setter accessors with no side effects, since
+          // calling set accessors go through a store bytecode. Store bytecodes
+          // are considered to cause side effects (to non-temporary objects).
+          DCHECK_NE(kSetter, accessor_kind);
+          return true;
+        case SideEffectType::kHasSideEffectToReceiver:
+          DCHECK(!receiver.is_null());
+          if (PerformSideEffectCheckForObject(receiver)) return true;
+          isolate_->OptionalRescheduleException(false);
+          return false;
+        case SideEffectType::kHasSideEffect:
+          break;
+      }
+      if (FLAG_trace_side_effect_free_debug_evaluate) {
+        PrintF("[debug-evaluate] API Callback '");
+        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");
+      }
+    } else if (callback_info->IsCallHandlerInfo()) {
+      CallHandlerInfo* info = CallHandlerInfo::cast(*callback_info);
+      if (info->IsSideEffectFreeCallHandlerInfo()) return true;
+      if (FLAG_trace_side_effect_free_debug_evaluate) {
+        PrintF("[debug-evaluate] API CallHandlerInfo may cause side effect.\n");
+      }
+    }
   }
   side_effect_check_failed_ = true;
   // Throw an uncatchable termination exception.
@@ -2226,11 +2266,14 @@ bool Debug::PerformSideEffectCheckAtBytecode(InterpretedFrame* frame) {
 bool Debug::PerformSideEffectCheckForObject(Handle<Object> object) {
   DCHECK_EQ(isolate_->debug_execution_mode(), DebugInfo::kSideEffects);
 
-  if (object->IsHeapObject()) {
-    if (temporary_objects_->HasObject(Handle<HeapObject>::cast(object))) {
-      return true;
-    }
+  // We expect no side-effects for primitives.
+  if (object->IsNumber()) return true;
+  if (object->IsName()) return true;
+
+  if (temporary_objects_->HasObject(Handle<HeapObject>::cast(object))) {
+    return true;
   }
+
   if (FLAG_trace_side_effect_free_debug_evaluate) {
     PrintF("[debug-evaluate] failed runtime side effect check.\n");
   }

src/debug/debug.h

@@ -326,7 +326,11 @@ class Debug {
 
   bool PerformSideEffectCheck(Handle<JSFunction> function,
                               Handle<Object> receiver);
-  bool PerformSideEffectCheckForCallback(Handle<Object> callback_info);
+
+  enum AccessorKind { kNotAccessor, kGetter, kSetter };
+  bool PerformSideEffectCheckForCallback(Handle<Object> callback_info,
+                                         Handle<Object> receiver,
+                                         AccessorKind accessor_kind);
   bool PerformSideEffectCheckAtBytecode(InterpretedFrame* frame);
   bool PerformSideEffectCheckForObject(Handle<Object> object);
 

src/external-reference-table.cc

@@ -159,8 +159,8 @@ void ExternalReferenceTable::AddAccessors(int* index) {
   };
 
   static const AccessorRefTable getters[] = {
-#define ACCESSOR_INFO_DECLARATION(accessor_name, AccessorName) \
-  {FUNCTION_ADDR(&Accessors::AccessorName##Getter),            \
+#define ACCESSOR_INFO_DECLARATION(accessor_name, AccessorName, ...) \
+  {FUNCTION_ADDR(&Accessors::AccessorName##Getter),                 \
    "Accessors::" #AccessorName "Getter"}, /* NOLINT(whitespace/indent) */
       ACCESSOR_INFO_LIST(ACCESSOR_INFO_DECLARATION)
 #undef ACCESSOR_INFO_DECLARATION

src/heap/factory-inl.h

@@ -73,7 +73,7 @@ PUBLIC_SYMBOL_LIST(SYMBOL_ACCESSOR)
 WELL_KNOWN_SYMBOL_LIST(SYMBOL_ACCESSOR)
 #undef SYMBOL_ACCESSOR
 
-#define ACCESSOR_INFO_ACCESSOR(accessor_name, AccessorName)        \
+#define ACCESSOR_INFO_ACCESSOR(accessor_name, AccessorName, ...)   \
   Handle<AccessorInfo> Factory::accessor_name##_accessor() {       \
     return Handle<AccessorInfo>(bit_cast<AccessorInfo**>(          \
         &isolate()                                                 \

src/heap/factory.h

@@ -861,7 +861,7 @@ class V8_EXPORT_PRIVATE Factory {
   WELL_KNOWN_SYMBOL_LIST(SYMBOL_ACCESSOR)
 #undef SYMBOL_ACCESSOR
 
-#define ACCESSOR_INFO_ACCESSOR(accessor_name, AccessorName) \
+#define ACCESSOR_INFO_ACCESSOR(accessor_name, ...) \
   inline Handle<AccessorInfo> accessor_name##_accessor();
   ACCESSOR_INFO_LIST(ACCESSOR_INFO_ACCESSOR)
 #undef ACCESSOR_INFO_ACCESSOR

src/heap/heap-inl.h

@@ -64,7 +64,7 @@ MUTABLE_ROOT_LIST(ROOT_ACCESSOR)
 DATA_HANDLER_LIST(DATA_HANDLER_MAP_ACCESSOR)
 #undef DATA_HANDLER_MAP_ACCESSOR
 
-#define ACCESSOR_INFO_ACCESSOR(accessor_name, AccessorName)                \
+#define ACCESSOR_INFO_ACCESSOR(accessor_name, AccessorName, ...)           \
   AccessorInfo* Heap::accessor_name##_accessor() {                         \
     return AccessorInfo::cast(roots_[k##AccessorName##AccessorRootIndex]); \
   }

src/heap/heap.h

@@ -325,7 +325,8 @@ class Heap {
     WELL_KNOWN_SYMBOL_LIST(DECL)
 #undef DECL
 
-#define DECL(accessor_name, AccessorName) k##AccessorName##AccessorRootIndex,
+#define DECL(accessor_name, AccessorName, ...) \
+    k##AccessorName##AccessorRootIndex,
     ACCESSOR_INFO_LIST(DECL)
 #undef DECL
 
@@ -821,7 +822,7 @@ class Heap {
   DATA_HANDLER_LIST(DATA_HANDLER_MAP_ACCESSOR)
 #undef DATA_HANDLER_MAP_ACCESSOR
 
-#define ACCESSOR_INFO_ACCESSOR(accessor_name, AccessorName) \
+#define ACCESSOR_INFO_ACCESSOR(accessor_name, ...) \
   inline AccessorInfo* accessor_name##_accessor();
   ACCESSOR_INFO_LIST(ACCESSOR_INFO_ACCESSOR)
 #undef ACCESSOR_INFO_ACCESSOR

src/heap/setup-heap-internal.cc

@@ -901,16 +901,19 @@ void Heap::CreateInternalAccessorInfoObjects() {
   HandleScope scope(isolate);
   Handle<AccessorInfo> acessor_info;
 
-#define INIT_ACCESSOR_INFO(accessor_name, AccessorName)        \
+#define INIT_ACCESSOR_INFO(accessor_name, AccessorName, ...)   \
   acessor_info = Accessors::Make##AccessorName##Info(isolate); \
   roots_[k##AccessorName##AccessorRootIndex] = *acessor_info;
   ACCESSOR_INFO_LIST(INIT_ACCESSOR_INFO)
 #undef INIT_ACCESSOR_INFO
 
-#define INIT_SIDE_EFFECT_FLAG(AccessorName)                      \
-  AccessorInfo::cast(roots_[k##AccessorName##AccessorRootIndex]) \
-      ->set_has_no_side_effect(true);
-  SIDE_EFFECT_FREE_ACCESSOR_INFO_LIST(INIT_SIDE_EFFECT_FLAG)
+#define INIT_SIDE_EFFECT_FLAG(accessor_name, AccessorName, GetterType, \
+                              SetterType)                              \
+  AccessorInfo::cast(roots_[k##AccessorName##AccessorRootIndex])       \
+      ->set_getter_side_effect_type(SideEffectType::GetterType);       \
+  AccessorInfo::cast(roots_[k##AccessorName##AccessorRootIndex])       \
+      ->set_setter_side_effect_type(SideEffectType::SetterType);
+  ACCESSOR_INFO_LIST(INIT_SIDE_EFFECT_FLAG)
 #undef INIT_SIDE_EFFECT_FLAG
 }
 

src/objects/api-callbacks-inl.h

@@ -59,8 +59,22 @@ BIT_FIELD_ACCESSORS(AccessorInfo, flags, is_special_data_property,
 BIT_FIELD_ACCESSORS(AccessorInfo, flags, replace_on_access,
                     AccessorInfo::ReplaceOnAccessBit)
 BIT_FIELD_ACCESSORS(AccessorInfo, flags, is_sloppy, AccessorInfo::IsSloppyBit)
-BIT_FIELD_ACCESSORS(AccessorInfo, flags, has_no_side_effect,
-                    AccessorInfo::HasNoSideEffectBit)
+BIT_FIELD_ACCESSORS(AccessorInfo, flags, getter_side_effect_type,
+                    AccessorInfo::GetterSideEffectTypeBits)
+
+SideEffectType AccessorInfo::setter_side_effect_type() const {
+  return SetterSideEffectTypeBits::decode(flags());
+}
+
+void AccessorInfo::set_setter_side_effect_type(SideEffectType value) {
+  // We do not support describing setters as having no side effect, since
+  // calling set accessors must go through a store bytecode. Store bytecodes
+  // support checking receivers for temporary objects, but still expect
+  // the receiver to be written to.
+  CHECK_NE(value, SideEffectType::kHasNoSideEffect);
+  set_flags(SetterSideEffectTypeBits::update(flags(), value));
+}
+
 BIT_FIELD_ACCESSORS(AccessorInfo, flags, initial_property_attributes,
                     AccessorInfo::InitialAttributesBits)
 

src/objects/api-callbacks.h

@@ -48,7 +48,12 @@ class AccessorInfo : public Struct {
   DECL_BOOLEAN_ACCESSORS(is_special_data_property)
   DECL_BOOLEAN_ACCESSORS(replace_on_access)
   DECL_BOOLEAN_ACCESSORS(is_sloppy)
-  DECL_BOOLEAN_ACCESSORS(has_no_side_effect)
+
+  inline SideEffectType getter_side_effect_type() const;
+  inline void set_getter_side_effect_type(SideEffectType type);
+
+  inline SideEffectType setter_side_effect_type() const;
+  inline void set_setter_side_effect_type(SideEffectType type);
 
   // The property attributes used when an API object template is instantiated
   // for the first time. Changing of this value afterwards does not affect
@@ -89,13 +94,15 @@ class AccessorInfo : public Struct {
   inline bool HasExpectedReceiverType();
 
 // Bit positions in |flags|.
-#define ACCESSOR_INFO_FLAGS_BIT_FIELDS(V, _) \
-  V(AllCanReadBit, bool, 1, _)               \
-  V(AllCanWriteBit, bool, 1, _)              \
-  V(IsSpecialDataPropertyBit, bool, 1, _)    \
-  V(IsSloppyBit, bool, 1, _)                 \
-  V(ReplaceOnAccessBit, bool, 1, _)          \
-  V(HasNoSideEffectBit, bool, 1, _)          \
+#define ACCESSOR_INFO_FLAGS_BIT_FIELDS(V, _)                           \
+  V(AllCanReadBit, bool, 1, _)                                         \
+  V(AllCanWriteBit, bool, 1, _)                                        \
+  V(IsSpecialDataPropertyBit, bool, 1, _)                              \
+  V(IsSloppyBit, bool, 1, _)                                           \
+  V(ReplaceOnAccessBit, bool, 1, _)                                    \
+  V(GetterSideEffectTypeBits, SideEffectType, 2, _)                    \
+  /* We could save a bit from setter side-effect type, if necessary */ \
+  V(SetterSideEffectTypeBits, SideEffectType, 2, _)                    \
   V(InitialAttributesBits, PropertyAttributes, 3, _)
 
   DEFINE_BIT_FIELDS(ACCESSOR_INFO_FLAGS_BIT_FIELDS)

src/profiler/heap-snapshot-generator.cc

@@ -1935,8 +1935,7 @@ const char* V8HeapExplorer::GetStrongGcSubrootName(Object* object) {
     PUBLIC_SYMBOL_LIST(SYMBOL_NAME)
     WELL_KNOWN_SYMBOL_LIST(SYMBOL_NAME)
 #undef SYMBOL_NAME
-#define ACCESSOR_NAME(accessor_name, AccessorName) \
-  NAME_ENTRY(accessor_name##_accessor)
+#define ACCESSOR_NAME(accessor_name, ...) NAME_ENTRY(accessor_name##_accessor)
     ACCESSOR_INFO_LIST(ACCESSOR_NAME)
 #undef ACCESSOR_NAME
 #undef NAME_ENTRY

test/cctest/test-api-accessors.cc

@@ -240,8 +240,12 @@ static void Getter(v8::Local<v8::Name> name,
 static void StringGetter(v8::Local<v8::String> name,
                          const v8::PropertyCallbackInfo<v8::Value>& info) {}
 
-static void Setter(v8::Local<v8::String> name, v8::Local<v8::Value> value,
-                   const v8::PropertyCallbackInfo<void>& info) {}
+static int set_accessor_call_count = 0;
+
+static void Setter(v8::Local<v8::Name> name, v8::Local<v8::Value> value,
+                   const v8::PropertyCallbackInfo<void>& info) {
+  set_accessor_call_count++;
+}
 }  // namespace
 
 // Re-declaration of non-configurable accessors should throw.
@@ -297,6 +301,64 @@ TEST(AccessorSetHasNoSideEffect) {
                   .ToLocalChecked()
                   ->Int32Value(env.local())
                   .FromJust());
+  CHECK_EQ(0, set_accessor_call_count);
+}
+
+// Set accessors can be whitelisted as side-effect-free via SetAccessor.
+TEST(SetAccessorSetSideEffectReceiverCheck1) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+
+  v8::Local<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(isolate);
+  v8::Local<v8::Object> obj = templ->NewInstance(env.local()).ToLocalChecked();
+  CHECK(env->Global()->Set(env.local(), v8_str("obj"), obj).FromJust());
+  obj->SetAccessor(env.local(), v8_str("foo"), Getter, Setter,
+                   v8::MaybeLocal<v8::Value>(), v8::AccessControl::DEFAULT,
+                   v8::PropertyAttribute::None,
+                   v8::SideEffectType::kHasNoSideEffect,
+                   v8::SideEffectType::kHasSideEffectToReceiver)
+      .ToChecked();
+  CHECK(v8::debug::EvaluateGlobal(isolate, v8_str("obj.foo"), true)
+            .ToLocalChecked()
+            ->Equals(env.local(), v8_str("return value"))
+            .FromJust());
+  v8::TryCatch try_catch(isolate);
+  CHECK(v8::debug::EvaluateGlobal(isolate, v8_str("obj.foo = 1"), true)
+            .IsEmpty());
+  CHECK(try_catch.HasCaught());
+  CHECK_EQ(0, set_accessor_call_count);
+}
+
+static void ConstructCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
+}
+
+TEST(SetAccessorSetSideEffectReceiverCheck2) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+
+  v8::Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(
+      isolate, ConstructCallback, v8::Local<v8::Value>(),
+      v8::Local<v8::Signature>(), 0, v8::ConstructorBehavior::kAllow,
+      v8::SideEffectType::kHasNoSideEffect);
+  templ->InstanceTemplate()->SetAccessor(
+      v8_str("bar"), Getter, Setter, v8::Local<v8::Value>(),
+      v8::AccessControl::DEFAULT, v8::PropertyAttribute::None,
+      v8::Local<v8::AccessorSignature>(),
+      v8::SideEffectType::kHasSideEffectToReceiver,
+      v8::SideEffectType::kHasSideEffectToReceiver);
+  CHECK(env->Global()
+            ->Set(env.local(), v8_str("f"),
+                  templ->GetFunction(env.local()).ToLocalChecked())
+            .FromJust());
+  CHECK(v8::debug::EvaluateGlobal(isolate, v8_str("new f().bar"), true)
+            .ToLocalChecked()
+            ->Equals(env.local(), v8_str("return value"))
+            .FromJust());
+  v8::debug::EvaluateGlobal(isolate, v8_str("new f().bar = 1"), true)
+      .ToLocalChecked();
+  CHECK_EQ(1, set_accessor_call_count);
 }
 
 // Accessors can be whitelisted as side-effect-free via SetNativeDataProperty.

test/cctest/test-roots.cc

@@ -124,7 +124,7 @@ TEST(TestAccessorInfosNotReadOnly) {
   Factory* factory = CcTest::i_isolate()->factory();
   Heap* heap = CcTest::i_isolate()->heap();
 
-#define TEST_ROOT(name, AccessorName) CHECK_NOT_IN_RO_SPACE(name##_accessor)
+#define TEST_ROOT(name, ...) CHECK_NOT_IN_RO_SPACE(name##_accessor)
   ACCESSOR_INFO_LIST(TEST_ROOT)
 #undef TEST_ROOT
 }

test/debugger/debug/side-effect/debug-evaluate-no-side-effect-runtime-check.js

@@ -50,8 +50,7 @@ success([1], `return_array_use_spread([1])`);
 // CallAccessorSetter
 var array = [1,2,3];
 fail(`array.length = 2`);
-// TODO(7515): this one should be side effect free
-fail(`[1,2,3].length = 2`);
+success(2, `[1,2,3].length = 2`);
 
 // StaDataPropertyInLiteral
 function return_literal_with_data_property(a) {

test/debugger/debug/side-effect/debug-evaluate-no-side-effect.js

@@ -9,6 +9,7 @@ let a = 1;
 var object = { property : 2,
                get getter() { return 3; }
              };
+var string0 = new String("string");
 var string1 = { toString() { return "x"; } };
 var string2 = { toString() { print("x"); return "x"; } };
 var array = [4, 5];
@@ -19,6 +20,9 @@ function set_a() { a = 2; }
 function get_a() { return a; }
 var bound = get_a.bind(0);
 
+function return_arg0() { return return_arg0.arguments[0]; }
+function return_caller_name() { return return_caller_name.caller.name; }
+
 var global_eval = eval;
 
 function listener(event, exec_state, event_data, data) {
@@ -32,6 +36,7 @@ function listener(event, exec_state, event_data, data) {
       assertThrows(() => exec_state.frame(0).evaluate(source, true),
                    EvalError);
     }
+
     // Simple test.
     success(3, "1 + 2");
     // Dymanic load.
@@ -62,8 +67,9 @@ function listener(event, exec_state, event_data, data) {
     success("set_a", "set_a.name");
     success(0, "bound.length");
     success("bound get_a", "bound.name");
+    success(1, "return_arg0(1)");
+    success("f", "(function f() { return return_caller_name() })()");
     // Non-evaluated call.
-    success("abc", "['abc'].join('foo')");
     // Constructed literals.
     success([1], "[1]");
     success({x: 1}, "({x: 1})");
@@ -82,13 +88,25 @@ function listener(event, exec_state, event_data, data) {
     fail("try { set_a() } catch (e) {}");
     // Test that call to set accessor fails.
     fail("array.length = 4");
-    fail("'x'.length = 1");
     fail("set_a.name = 'set_b'");
     fail("set_a.length = 1");
     fail("bound.name = 'bound'");
     fail("bound.length = 1");
+    fail("set_a.prototype = null");
     // Test that call to non-whitelisted get accessor fails.
     fail("error.stack");
+    // Call to set accessors with receiver check.
+    success(1, "[].length = 1");
+    success(1, "'x'.length = 1");
+    fail("string0.length = 1");
+    success(1, "(new String('abc')).length = 1");
+    success("g", "(function(){}).name = 'g'");
+    success(1, "(function(){}).length = 1");
+    success("g", "get_a.bind(0).name = 'g'");
+    success(1, "get_a.bind(0).length = 1");
+    success(null, "(function(){}).prototype = null");
+    success(true, "(new Error()).stack.length > 1");
+    success("a", "(new Error()).stack = 'a'");
     // Eval is not allowed.
     fail("eval('Math.sin(1)')");
     fail("eval('exception = 1')");