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

This reverts commit 0ce36e7d.

Reason for revert: Speculative revert for a Chromium build breakage causing a blocked roll - https://bugs.chromium.org/p/v8/issues/detail?id=11761

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}

Change-Id: Ifb5c24682af29572591d436ab92b0304058e99af
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2891650
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#74515}

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,
-                next_key, &slow_load, kCallJSGetter);
+            var_value = CallGetterIfAccessor(var_value.value(), object,
+                                             var_details.value(), context,
+                                             object, &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, TNode<Object> name,
-    Label* if_bailout, GetOwnPropertyMode mode) {
+    TNode<Context> context, TNode<Object> receiver, Label* if_bailout,
+    GetOwnPropertyMode mode) {
   TVARIABLE(Object, var_value, value);
   Label done(this), if_accessor_info(this, Label::kDeferred);
 
@@ -9413,16 +9413,13 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
 
       BIND(&if_callable);
       {
-        // Call the accessor. No need to check side-effect mode here, since it
-        // will be checked later in DebugOnFunctionCall.
+        // Call the accessor.
         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);
@@ -9433,13 +9430,6 @@ 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);
@@ -9574,7 +9564,7 @@ void CodeStubAssembler::TryGetOwnProperty(
     }
     TNode<Object> value =
         CallGetterIfAccessor(var_value->value(), object, var_details->value(),
-                             context, receiver, unique_name, if_bailout, mode);
+                             context, receiver, if_bailout, mode);
     *var_value = value;
     Goto(if_found_value);
   }
@@ -14026,20 +14016,6 @@ 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,7 +3481,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
 
   // Debug helpers
   TNode<BoolT> IsDebugActive();
-  TNode<BoolT> IsSideEffectFreeDebuggingActive();
 
   // JSArrayBuffer helpers
   TNode<RawPtrT> LoadJSArrayBufferBackingStorePtr(
@@ -3756,10 +3755,12 @@ 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, TNode<Object> name,
-      Label* if_bailout, GetOwnPropertyMode mode = kCallJSGetter);
+  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<IntPtrT> TryToIntptr(TNode<Object> key, Label* if_not_intptr,
                              TVariable<Int32T>* var_instance_type = nullptr);

src/debug/debug-evaluate.cc

@@ -43,22 +43,6 @@ 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(),
@@ -66,14 +50,19 @@ MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
           mode ==
               debug::EvaluateGlobalMode::kDisableBreaksAndThrowOnSideEffect);
 
-  Handle<NativeContext> context = isolate->native_context();
-  CHECK_EQ(function->native_context(), *context);
+  Handle<SharedFunctionInfo> shared_info;
+  if (!GetFunctionInfo(isolate, source, repl_mode).ToHandle(&shared_info)) {
+    return MaybeHandle<Object>();
+  }
 
+  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, function, Handle<JSObject>(context->global_proxy(), isolate), 0,
+      isolate, fun, Handle<JSObject>(context->global_proxy(), isolate), 0,
       nullptr);
   if (mode == debug::EvaluateGlobalMode::kDisableBreaksAndThrowOnSideEffect) {
     isolate->debug()->StopSideEffectCheckMode();

src/debug/debug-evaluate.h

@@ -27,10 +27,6 @@ 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,34 +777,6 @@ 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) {
@@ -2805,22 +2777,6 @@ 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(), p->name(), miss);
+          p->receiver(), miss);
       exit_point->Return(value);
     }
   }
@@ -614,17 +614,11 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
   }
 
   BIND(&native_data_property);
-  {
-    GotoIf(IsSideEffectFreeDebuggingActive(), &slow);
-    HandleLoadCallbackProperty(p, CAST(holder), handler_word, exit_point);
-  }
+  HandleLoadCallbackProperty(p, CAST(holder), handler_word, exit_point);
 
   BIND(&api_getter);
-  {
-    GotoIf(IsSideEffectFreeDebuggingActive(), &slow);
-    HandleLoadAccessor(p, CAST(holder), handler_word, CAST(handler),
-                       handler_kind, exit_point);
-  }
+  HandleLoadAccessor(p, CAST(holder), handler_word, CAST(handler), handler_kind,
+                     exit_point);
 
   BIND(&proxy);
   {
@@ -684,8 +678,7 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
     GotoIf(IsTheHole(value), miss);
 
     exit_point->Return(CallGetterIfAccessor(value, CAST(holder), details,
-                                            p->context(), p->receiver(),
-                                            p->name(), miss));
+                                            p->context(), p->receiver(), miss));
   }
 
   BIND(&interceptor);
@@ -971,7 +964,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(), p->name(), miss);
+              p->context(), p->receiver(), miss);
           exit_point->Return(value);
         }
       },
@@ -1744,12 +1737,9 @@ 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()));
-      }
+      TNode<IntPtrT> argc = IntPtrConstant(1);
+      Return(CallApiCallback(context, callback, argc, data, api_holder.value(),
+                             p->receiver(), p->value()));
     }
 
     BIND(&if_store_global_proxy);
@@ -2571,7 +2561,7 @@ void AccessorAssembler::GenericPropertyLoad(
   {
     TNode<Object> value = CallGetterIfAccessor(
         var_value.value(), lookup_start_object, var_details.value(),
-        p->context(), p->receiver(), p->name(), slow);
+        p->context(), p->receiver(), slow);
     IncrementCounter(isolate()->counters()->ic_keyed_load_generic_symbol(), 1);
     Return(value);
   }

src/objects/api-callbacks.h

@@ -64,8 +64,6 @@ 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();