[ic] Fix handling of API properties with side effects

DebugEvaluate can evaluate expressions in side-effect-free mode, where
any operation that would cause observable side effects throws an
exception. Currently, when accessors are backed by callbacks, it's
possible that ICs call those accessors directly, bypassing the
side-effect checks. This CL introduces a bailouts to runtime in those
cases.

Fixed: chromium:1201781
Also-By: ishell@chromium.org, pfaffe@chromium.org
Change-Id: Ie53bfb2bff7b3420f2b27091e8df6723382cf53c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2857634
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74507}

src/codegen/code-stub-assembler.cc

@@ -8930,9 +8930,9 @@ void CodeStubAssembler::ForEachEnumerableOwnProperty(
           {
             Label slow_load(this, Label::kDeferred);
 
-            var_value = CallGetterIfAccessor(var_value.value(), object,
-                                             var_details.value(), context,
-                                             object, &slow_load, kCallJSGetter);
+            var_value = CallGetterIfAccessor(
+                var_value.value(), object, var_details.value(), context, object,
+                next_key, &slow_load, kCallJSGetter);
             Goto(&callback);
 
             BIND(&slow_load);
@@ -9384,8 +9384,8 @@ template void CodeStubAssembler::LoadPropertyFromDictionary(
 // result of the getter call.
 TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
     TNode<Object> value, TNode<HeapObject> holder, TNode<Uint32T> details,
-    TNode<Context> context, TNode<Object> receiver, Label* if_bailout,
-    GetOwnPropertyMode mode) {
+    TNode<Context> context, TNode<Object> receiver, TNode<Object> name,
+    Label* if_bailout, GetOwnPropertyMode mode) {
   TVARIABLE(Object, var_value, value);
   Label done(this), if_accessor_info(this, Label::kDeferred);
 
@@ -9413,13 +9413,16 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
 
       BIND(&if_callable);
       {
-        // Call the accessor.
+        // Call the accessor. No need to check side-effect mode here, since it
+        // will be checked later in DebugOnFunctionCall.
         var_value = Call(context, getter, receiver);
         Goto(&done);
       }
 
       BIND(&if_function_template_info);
       {
+        Label runtime(this, Label::kDeferred);
+        GotoIf(IsSideEffectFreeDebuggingActive(), &runtime);
         TNode<HeapObject> cached_property_name = LoadObjectField<HeapObject>(
             getter, FunctionTemplateInfo::kCachedPropertyNameOffset);
         GotoIfNot(IsTheHole(cached_property_name), if_bailout);
@@ -9430,6 +9433,13 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
             Builtins::kCallFunctionTemplate_CheckAccessAndCompatibleReceiver,
             creation_context, getter, IntPtrConstant(0), receiver);
         Goto(&done);
+
+        BIND(&runtime);
+        {
+          var_value = CallRuntime(Runtime::kGetProperty, context, holder, name,
+                                  receiver);
+          Goto(&done);
+        }
       }
     } else {
       Goto(&done);
@@ -9564,7 +9574,7 @@ void CodeStubAssembler::TryGetOwnProperty(
     }
     TNode<Object> value =
         CallGetterIfAccessor(var_value->value(), object, var_details->value(),
-                             context, receiver, if_bailout, mode);
+                             context, receiver, unique_name, if_bailout, mode);
     *var_value = value;
     Goto(if_found_value);
   }
@@ -14016,6 +14026,20 @@ TNode<BoolT> CodeStubAssembler::IsDebugActive() {
   return Word32NotEqual(is_debug_active, Int32Constant(0));
 }
 
+TNode<BoolT> CodeStubAssembler::IsSideEffectFreeDebuggingActive() {
+  TNode<Uint8T> debug_execution_mode = Load<Uint8T>(ExternalConstant(
+      ExternalReference::debug_execution_mode_address(isolate())));
+
+  TNode<BoolT> is_active =
+      Word32Equal(debug_execution_mode,
+                  Int32Constant(DebugInfo::ExecutionMode::kSideEffects));
+#ifdef DEBUG
+  CSA_ASSERT(this, Word32Or(IsDebugActive(), Word32BinaryNot(is_active)));
+#endif
+
+  return is_active;
+}
+
 TNode<BoolT> CodeStubAssembler::HasAsyncEventDelegate() {
   const TNode<RawPtrT> async_event_delegate = Load<RawPtrT>(ExternalConstant(
       ExternalReference::async_event_delegate_address(isolate())));

src/codegen/code-stub-assembler.h

@@ -3481,6 +3481,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
 
   // Debug helpers
   TNode<BoolT> IsDebugActive();
+  TNode<BoolT> IsSideEffectFreeDebuggingActive();
 
   // JSArrayBuffer helpers
   TNode<RawPtrT> LoadJSArrayBufferBackingStorePtr(
@@ -3755,12 +3756,10 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
                                     const ForEachKeyValueFunction& body,
                                     Label* bailout);
 
-  TNode<Object> CallGetterIfAccessor(TNode<Object> value,
-                                     TNode<HeapObject> holder,
-                                     TNode<Uint32T> details,
-                                     TNode<Context> context,
-                                     TNode<Object> receiver, Label* if_bailout,
-                                     GetOwnPropertyMode mode = kCallJSGetter);
+  TNode<Object> CallGetterIfAccessor(
+      TNode<Object> value, TNode<HeapObject> holder, TNode<Uint32T> details,
+      TNode<Context> context, TNode<Object> receiver, TNode<Object> name,
+      Label* if_bailout, GetOwnPropertyMode mode = kCallJSGetter);
 
   TNode<IntPtrT> TryToIntptr(TNode<Object> key, Label* if_not_intptr,
                              TVariable<Int32T>* var_instance_type = nullptr);

src/debug/debug-evaluate.cc

@@ -43,6 +43,22 @@ MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
                                           Handle<String> source,
                                           debug::EvaluateGlobalMode mode,
                                           REPLMode repl_mode) {
+  Handle<SharedFunctionInfo> shared_info;
+  if (!GetFunctionInfo(isolate, source, repl_mode).ToHandle(&shared_info)) {
+    return MaybeHandle<Object>();
+  }
+
+  Handle<NativeContext> context = isolate->native_context();
+  Handle<JSFunction> fun =
+      Factory::JSFunctionBuilder{isolate, shared_info, context}.Build();
+
+  return Global(isolate, fun, mode, repl_mode);
+}
+
+MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
+                                          Handle<JSFunction> function,
+                                          debug::EvaluateGlobalMode mode,
+                                          REPLMode repl_mode) {
   // Disable breaks in side-effect free mode.
   DisableBreak disable_break_scope(
       isolate->debug(),
@@ -50,19 +66,14 @@ MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
           mode ==
               debug::EvaluateGlobalMode::kDisableBreaksAndThrowOnSideEffect);
 
-  Handle<SharedFunctionInfo> shared_info;
-  if (!GetFunctionInfo(isolate, source, repl_mode).ToHandle(&shared_info)) {
-    return MaybeHandle<Object>();
-  }
+  Handle<NativeContext> context = isolate->native_context();
+  CHECK_EQ(function->native_context(), *context);
 
-  Handle<Context> context = isolate->native_context();
-  Handle<JSFunction> fun =
-      Factory::JSFunctionBuilder{isolate, shared_info, context}.Build();
   if (mode == debug::EvaluateGlobalMode::kDisableBreaksAndThrowOnSideEffect) {
     isolate->debug()->StartSideEffectCheckMode();
   }
   MaybeHandle<Object> result = Execution::Call(
-      isolate, fun, Handle<JSObject>(context->global_proxy(), isolate), 0,
+      isolate, function, Handle<JSObject>(context->global_proxy(), isolate), 0,
       nullptr);
   if (mode == debug::EvaluateGlobalMode::kDisableBreaksAndThrowOnSideEffect) {
     isolate->debug()->StopSideEffectCheckMode();

src/debug/debug-evaluate.h

@@ -27,6 +27,10 @@ class DebugEvaluate : public AllStatic {
                                     debug::EvaluateGlobalMode mode,
                                     REPLMode repl_mode = REPLMode::kNo);
 
+  static V8_EXPORT_PRIVATE MaybeHandle<Object> Global(
+      Isolate* isolate, Handle<JSFunction> function,
+      debug::EvaluateGlobalMode mode, REPLMode repl_mode = REPLMode::kNo);
+
   // Evaluate a piece of JavaScript in the context of a stack frame for
   // debugging.  Things that need special attention are:
   // - Parameters and stack-allocated locals need to be materialized.  Altered

src/diagnostics/objects-printer.cc

@@ -777,6 +777,34 @@ void FixedArray::FixedArrayPrint(std::ostream& os) {
   PrintFixedArrayWithHeader(os, *this, "FixedArray");
 }
 
+namespace {
+const char* SideEffectType2String(SideEffectType type) {
+  switch (type) {
+    case SideEffectType::kHasSideEffect:
+      return "kHasSideEffect";
+    case SideEffectType::kHasNoSideEffect:
+      return "kHasNoSideEffect";
+    case SideEffectType::kHasSideEffectToReceiver:
+      return "kHasSideEffectToReceiver";
+  }
+}
+}  // namespace
+
+void AccessorInfo::AccessorInfoPrint(std::ostream& os) {
+  TorqueGeneratedAccessorInfo<AccessorInfo, Struct>::AccessorInfoPrint(os);
+  os << " - all_can_read: " << all_can_read();
+  os << "\n - all_can_write: " << all_can_write();
+  os << "\n - is_special_data_property: " << is_special_data_property();
+  os << "\n - is_sloppy: " << is_sloppy();
+  os << "\n - replace_on_access: " << replace_on_access();
+  os << "\n - getter_side_effect_type: "
+     << SideEffectType2String(getter_side_effect_type());
+  os << "\n - setter_side_effect_type: "
+     << SideEffectType2String(setter_side_effect_type());
+  os << "\n - initial_attributes: " << initial_property_attributes();
+  os << '\n';
+}
+
 namespace {
 void PrintContextWithHeader(std::ostream& os, Context context,
                             const char* type) {
@@ -2777,6 +2805,22 @@ V8_EXPORT_PRIVATE extern void _v8_internal_Print_Object(void* object) {
   GetObjectFromRaw(object).Print();
 }
 
+V8_EXPORT_PRIVATE extern void _v8_internal_Print_LoadHandler(void* object) {
+#ifdef OBJECT_PRINT
+  i::StdoutStream os;
+  i::LoadHandler::PrintHandler(GetObjectFromRaw(object), os);
+  os << std::flush;
+#endif
+}
+
+V8_EXPORT_PRIVATE extern void _v8_internal_Print_StoreHandler(void* object) {
+#ifdef OBJECT_PRINT
+  i::StdoutStream os;
+  i::StoreHandler::PrintHandler(GetObjectFromRaw(object), os);
+  os << std::flush;
+#endif
+}
+
 V8_EXPORT_PRIVATE extern void _v8_internal_Print_Code(void* object) {
   i::Address address = reinterpret_cast<i::Address>(object);
   i::Isolate* isolate = i::Isolate::Current();

src/ic/accessor-assembler.cc

@@ -594,7 +594,7 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
           properties, var_name_index.value(), &var_details, &var_value);
       TNode<Object> value = CallGetterIfAccessor(
           var_value.value(), CAST(holder), var_details.value(), p->context(),
-          p->receiver(), miss);
+          p->receiver(), p->name(), miss);
       exit_point->Return(value);
     }
   }
@@ -614,11 +614,17 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
   }
 
   BIND(&native_data_property);
+  {
+    GotoIf(IsSideEffectFreeDebuggingActive(), &slow);
     HandleLoadCallbackProperty(p, CAST(holder), handler_word, exit_point);
+  }
 
   BIND(&api_getter);
-  HandleLoadAccessor(p, CAST(holder), handler_word, CAST(handler), handler_kind,
-                     exit_point);
+  {
+    GotoIf(IsSideEffectFreeDebuggingActive(), &slow);
+    HandleLoadAccessor(p, CAST(holder), handler_word, CAST(handler),
+                       handler_kind, exit_point);
+  }
 
   BIND(&proxy);
   {
@@ -678,7 +684,8 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
     GotoIf(IsTheHole(value), miss);
 
     exit_point->Return(CallGetterIfAccessor(value, CAST(holder), details,
-                                            p->context(), p->receiver(), miss));
+                                            p->context(), p->receiver(),
+                                            p->name(), miss));
   }
 
   BIND(&interceptor);
@@ -964,7 +971,7 @@ void AccessorAssembler::HandleLoadICProtoHandler(
               properties, name_index, &var_details, &var_value);
           TNode<Object> value = CallGetterIfAccessor(
               var_value.value(), CAST(var_holder->value()), var_details.value(),
-              p->context(), p->receiver(), miss);
+              p->context(), p->receiver(), p->name(), miss);
           exit_point->Return(value);
         }
       },
@@ -1737,9 +1744,12 @@ void AccessorAssembler::HandleStoreICProtoHandler(
       Goto(&store);
 
       BIND(&store);
+      {
+        GotoIf(IsSideEffectFreeDebuggingActive(), &if_slow);
         TNode<IntPtrT> argc = IntPtrConstant(1);
-      Return(CallApiCallback(context, callback, argc, data, api_holder.value(),
-                             p->receiver(), p->value()));
+        Return(CallApiCallback(context, callback, argc, data,
+                               api_holder.value(), p->receiver(), p->value()));
+      }
     }
 
     BIND(&if_store_global_proxy);
@@ -2561,7 +2571,7 @@ void AccessorAssembler::GenericPropertyLoad(
   {
     TNode<Object> value = CallGetterIfAccessor(
         var_value.value(), lookup_start_object, var_details.value(),
-        p->context(), p->receiver(), slow);
+        p->context(), p->receiver(), p->name(), slow);
     IncrementCounter(isolate()->counters()->ic_keyed_load_generic_symbol(), 1);
     Return(value);
   }

src/objects/api-callbacks.h

@@ -64,6 +64,8 @@ class AccessorInfo : public TorqueGeneratedAccessorInfo<AccessorInfo, Struct> {
   static int AppendUnique(Isolate* isolate, Handle<Object> descriptors,
                           Handle<FixedArray> array, int valid_descriptors);
 
+  DECL_PRINTER(AccessorInfo)
+
  private:
   inline bool HasExpectedReceiverType();