Reland "[ic] Fix handling of API properties with side effects"

This is a reland of 0ce36e7d

The reland includes two fixes:
- Move the EvaluateGlobalForTesting into libv8 to avoid linkage issues
  and to avoid having to export ThreadLocalTop symbols.
- Give the ExecutionMode enum a uint8_t backing type to avoid endianess
  issues.

Original change's description:
> [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}

Fixes: v8:11761
Change-Id: I58cde8bd11ba0fc9d83adc19fa87733628ab6c13
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2891829Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74602}

src/codegen/code-stub-assembler.cc

@@ -8938,9 +8938,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);
@@ -9392,8 +9392,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);
 
@@ -9421,13 +9421,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);
@@ -9438,6 +9441,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);
@@ -9572,7 +9582,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);
   }
@@ -14027,6 +14037,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(
@@ -3748,12 +3749,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/debug/debug-interface.cc

@@ -951,6 +951,21 @@ MaybeLocal<v8::Value> EvaluateGlobal(v8::Isolate* isolate,
   RETURN_ESCAPED(result);
 }
 
+v8::MaybeLocal<v8::Value> EvaluateGlobalForTesting(
+    v8::Isolate* isolate, v8::Local<v8::Script> function,
+    v8::debug::EvaluateGlobalMode mode, bool repl) {
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  PREPARE_FOR_DEBUG_INTERFACE_EXECUTION_WITH_ISOLATE(internal_isolate, Value);
+  i::REPLMode repl_mode = repl ? i::REPLMode::kYes : i::REPLMode::kNo;
+  Local<Value> result;
+  has_pending_exception = !ToLocal<Value>(
+      i::DebugEvaluate::Global(internal_isolate, Utils::OpenHandle(*function),
+                               mode, repl_mode),
+      &result);
+  RETURN_ON_FAILED_EXECUTION(Value);
+  RETURN_ESCAPED(result);
+}
+
 void QueryObjects(v8::Local<v8::Context> v8_context,
                   QueryObjectPredicate* predicate,
                   PersistentValueVector<v8::Object>* objects) {

src/debug/debug-interface.h

@@ -527,6 +527,10 @@ V8_EXPORT_PRIVATE v8::MaybeLocal<v8::Value> EvaluateGlobal(
     v8::Isolate* isolate, v8::Local<v8::String> source, EvaluateGlobalMode mode,
     bool repl_mode = false);
 
+V8_EXPORT_PRIVATE v8::MaybeLocal<v8::Value> EvaluateGlobalForTesting(
+    v8::Isolate* isolate, v8::Local<v8::Script> function,
+    v8::debug::EvaluateGlobalMode mode, bool repl);
+
 int GetDebuggingId(v8::Local<v8::Function> function);
 
 bool SetFunctionBreakpoint(v8::Local<v8::Function> function,

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);
-  HandleLoadCallbackProperty(p, CAST(holder), handler_word, exit_point);
+  {
+    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);
-      TNode<IntPtrT> argc = IntPtrConstant(1);
-      Return(CallApiCallback(context, callback, argc, data, api_holder.value(),
-                             p->receiver(), p->value()));
+      {
+        GotoIf(IsSideEffectFreeDebuggingActive(), &if_slow);
+        TNode<IntPtrT> argc = IntPtrConstant(1);
+        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();
 

src/objects/debug-objects.h

@@ -37,7 +37,10 @@ class DebugInfo : public TorqueGeneratedDebugInfo<DebugInfo, Struct> {
   // --- Debug execution ---
   // -----------------------
 
-  enum ExecutionMode { kBreakpoints = 0, kSideEffects = kDebugExecutionMode };
+  enum ExecutionMode : uint8_t {
+    kBreakpoints = 0,
+    kSideEffects = kDebugExecutionMode
+  };
 
   // Returns current debug execution mode. Debug execution mode defines by
   // applied to bytecode patching. False for breakpoints, true for side effect