Defer inferring language mode for PropertyCallbackInfo

This cl: https://chromium-review.googlesource.com/c/v8/v8/+/1421077
changed the implementation of SetProperty to infer the language mode.
Language mode is only required when there is an error to decide if we
have to throw an error or not. However we used to compute language mode
eagerly for PropertyCallbackInfo. This causes regressions in some
benchmarks. This cl changes it by deferring it further by computing
it only when it is actually required.

BUG: v8:8580, chromium:925289
Change-Id: Iba70ec5f9bb3deec16414a1ec418b3963f2144f9
Reviewed-on: https://chromium-review.googlesource.com/c/1454608
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59450}

include/v8-internal.h

@@ -182,6 +182,12 @@ class Internals {
   static const int kUndefinedOddballKind = 5;
   static const int kNullOddballKind = 3;
 
+  // Constants used by PropertyCallbackInfo to check if we should throw when an
+  // error occurs.
+  static const int kThrowOnError = 0;
+  static const int kDontThrow = 1;
+  static const int kInferShouldThrowMode = 2;
+
   // Soft limit for AdjustAmountofExternalAllocatedMemory. Trigger an
   // incremental GC once the external memory reaches this limit.
   static constexpr int kExternalAllocationSoftLimit = 64 * 1024 * 1024;
@@ -367,6 +373,11 @@ V8_INLINE void PerformCastCheck(T* data) {
 // that's guaranteed to never be in ReadOnlySpace.
 V8_EXPORT internal::Isolate* IsolateFromNeverReadOnlySpaceObject(Address obj);
 
+// Returns if we need to throw when an error occurs. This infers the language
+// mode based on the current context and the closure. This returns true if the
+// language mode is strict.
+V8_EXPORT bool ShouldThrowOnError(v8::internal::Isolate* isolate);
+
 }  // namespace internal
 }  // namespace v8
 

include/v8.h

@@ -10723,10 +10723,14 @@ ReturnValue<T> PropertyCallbackInfo<T>::GetReturnValue() const {
 template <typename T>
 bool PropertyCallbackInfo<T>::ShouldThrowOnError() const {
   typedef internal::Internals I;
-  return args_[kShouldThrowOnErrorIndex] != I::IntToSmi(0);
+  if (args_[kShouldThrowOnErrorIndex] !=
+      I::IntToSmi(I::kInferShouldThrowMode)) {
+    return args_[kShouldThrowOnErrorIndex] != I::IntToSmi(I::kDontThrow);
+  }
+  return v8::internal::ShouldThrowOnError(
+      reinterpret_cast<v8::internal::Isolate*>(GetIsolate()));
 }
 
-
 Local<Primitive> Undefined(Isolate* isolate) {
   typedef internal::Address S;
   typedef internal::Internals I;

src/api-arguments.cc

@@ -9,17 +9,19 @@
 namespace v8 {
 namespace internal {
 
-PropertyCallbackArguments::PropertyCallbackArguments(Isolate* isolate,
+PropertyCallbackArguments::PropertyCallbackArguments(
-                                                     Object data, Object self,
+    Isolate* isolate, Object data, Object self, JSObject holder,
-                                                     JSObject holder,
+    Maybe<ShouldThrow> should_throw)
-                                                     ShouldThrow should_throw)
     : Super(isolate) {
   slot_at(T::kThisIndex).store(self);
   slot_at(T::kHolderIndex).store(holder);
   slot_at(T::kDataIndex).store(data);
   slot_at(T::kIsolateIndex).store(Object(reinterpret_cast<Address>(isolate)));
-  slot_at(T::kShouldThrowOnErrorIndex)
+  int value = Internals::kInferShouldThrowMode;
-      .store(Smi::FromInt(should_throw == kThrowOnError ? 1 : 0));
+  if (should_throw.IsJust()) {
+    value = should_throw.FromJust();
+  }
+  slot_at(T::kShouldThrowOnErrorIndex).store(Smi::FromInt(value));
 
   // Here the hole is set as default value.
   // It cannot escape into js as it's removed in Call below.

src/api-arguments.h

@@ -72,7 +72,7 @@ class PropertyCallbackArguments
   static const int kShouldThrowOnErrorIndex = T::kShouldThrowOnErrorIndex;
 
   PropertyCallbackArguments(Isolate* isolate, Object data, Object self,
-                            JSObject holder, ShouldThrow should_throw);
+                            JSObject holder, Maybe<ShouldThrow> should_throw);
 
   // -------------------------------------------------------------------------
   // Accessor Callbacks

src/api.cc

@@ -3660,6 +3660,11 @@ i::Isolate* i::IsolateFromNeverReadOnlySpaceObject(i::Address obj) {
       i::HeapObject::cast(i::Object(obj)));
 }
 
+bool i::ShouldThrowOnError(i::Isolate* isolate) {
+  return i::GetShouldThrow(isolate, Nothing<i::ShouldThrow>()) ==
+         i::ShouldThrow::kThrowOnError;
+}
+
 void i::Internals::CheckInitializedImpl(v8::Isolate* external_isolate) {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(external_isolate);
   Utils::ApiCheck(isolate != nullptr && !isolate->IsDead(),

src/globals.h

@@ -800,7 +800,10 @@ enum WhereToStart { kStartAtReceiver, kStartAtPrototype };
 
 enum ResultSentinel { kNotFound = -1, kUnsupported = -2 };
 
-enum ShouldThrow { kThrowOnError, kDontThrow };
+enum ShouldThrow {
+  kThrowOnError = Internals::kThrowOnError,
+  kDontThrow = Internals::kDontThrow
+};
 
 // The Store Buffer (GC).
 typedef enum {

src/ic/ic.cc

@@ -2710,9 +2710,8 @@ RUNTIME_FUNCTION(Runtime_StoreCallbackProperty) {
 
   DCHECK(info->IsCompatibleReceiver(*receiver));
 
-  ShouldThrow should_throw = GetShouldThrow(isolate, Nothing<ShouldThrow>());
   PropertyCallbackArguments arguments(isolate, info->data(), *receiver, *holder,
-                                      should_throw);
+                                      Nothing<ShouldThrow>());
   arguments.CallAccessorSetter(info, name, value);
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
   return *value;
@@ -2728,7 +2727,7 @@ RUNTIME_FUNCTION(Runtime_LoadCallbackProperty) {
   DCHECK(info->IsCompatibleReceiver(*receiver));
 
   PropertyCallbackArguments custom_args(isolate, info->data(), *receiver,
-                                        *holder, kThrowOnError);
+                                        *holder, Just(kThrowOnError));
   Handle<Object> result = custom_args.CallAccessorGetter(info, name);
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
   if (result.is_null()) return ReadOnlyRoots(isolate).undefined_value();
@@ -2776,7 +2775,7 @@ RUNTIME_FUNCTION(Runtime_LoadPropertyWithInterceptor) {
 
   Handle<InterceptorInfo> interceptor(holder->GetNamedInterceptor(), isolate);
   PropertyCallbackArguments arguments(isolate, interceptor->data(), *receiver,
-                                      *holder, kDontThrow);
+                                      *holder, Just(kDontThrow));
   Handle<Object> result = arguments.CallNamedGetter(interceptor, name);
 
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
@@ -2838,7 +2837,7 @@ RUNTIME_FUNCTION(Runtime_StorePropertyWithInterceptor) {
 
   DCHECK(!interceptor->non_masking());
   PropertyCallbackArguments arguments(isolate, interceptor->data(), *receiver,
-                                      *receiver, kDontThrow);
+                                      *receiver, Just(kDontThrow));
 
   Handle<Object> result = arguments.CallNamedSetter(interceptor, name, value);
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
@@ -2870,7 +2869,7 @@ RUNTIME_FUNCTION(Runtime_LoadElementWithInterceptor) {
   Handle<InterceptorInfo> interceptor(receiver->GetIndexedInterceptor(),
                                       isolate);
   PropertyCallbackArguments arguments(isolate, interceptor->data(), *receiver,
-                                      *receiver, kDontThrow);
+                                      *receiver, Just(kDontThrow));
   Handle<Object> result = arguments.CallIndexedGetter(interceptor, index);
 
   RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);

src/keys.cc

@@ -489,7 +489,7 @@ void FilterForEnumerableProperties(Handle<JSReceiver> receiver,
 
     // args are invalid after args.Call(), create a new one in every iteration.
     PropertyCallbackArguments args(accumulator->isolate(), interceptor->data(),
-                                   *receiver, *object, kDontThrow);
+                                   *receiver, *object, Just(kDontThrow));
 
     Handle<Object> element = accessor->Get(result, i);
     Handle<Object> attributes;
@@ -521,7 +521,7 @@ Maybe<bool> CollectInterceptorKeysInternal(Handle<JSReceiver> receiver,
                                            IndexedOrNamed type) {
   Isolate* isolate = accumulator->isolate();
   PropertyCallbackArguments enum_args(isolate, interceptor->data(), *receiver,
-                                      *object, kDontThrow);
+                                      *object, Just(kDontThrow));
 
   Handle<JSObject> result;
   if (!interceptor->enumerator()->IsUndefined(isolate)) {

src/objects.cc

@@ -1379,7 +1379,7 @@ MaybeHandle<Object> Object::GetPropertyWithAccessor(LookupIterator* it) {
     }
 
     PropertyCallbackArguments args(isolate, info->data(), *receiver, *holder,
-                                   kDontThrow);
+                                   Just(kDontThrow));
     Handle<Object> result = args.CallAccessorGetter(info, name);
     RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
     if (result.is_null()) return isolate->factory()->undefined_value();
@@ -1489,9 +1489,8 @@ Maybe<bool> Object::SetPropertyWithAccessor(
     // AccessorInfo was created by the API or internally (see accessors.cc).
     // Here we handle both cases using GenericNamedPropertySetterCallback and
     // its Call method.
-    ShouldThrow should_throw = GetShouldThrow(isolate, maybe_should_throw);
     PropertyCallbackArguments args(isolate, info->data(), *receiver, *holder,
-                                   should_throw);
+                                   maybe_should_throw);
     Handle<Object> result = args.CallAccessorSetter(info, name, value);
     // In the case of AccessorNameSetterCallback, we know that the result value
     // cannot have been set, so the result of Call will be null.  In the case of
@@ -1499,7 +1498,8 @@ Maybe<bool> Object::SetPropertyWithAccessor(
     // (signalling an exception) or a boolean Oddball.
     RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate, Nothing<bool>());
     if (result.is_null()) return Just(true);
-    DCHECK(result->BooleanValue(isolate) || should_throw == kDontThrow);
+    DCHECK(result->BooleanValue(isolate) ||
+           GetShouldThrow(isolate, maybe_should_throw) == kDontThrow);
     return Just(result->BooleanValue(isolate));
   }
 

src/objects/js-objects.cc

@@ -973,7 +973,7 @@ MaybeHandle<Object> GetPropertyWithInterceptorInternal(
         isolate, receiver, Object::ConvertReceiver(isolate, receiver), Object);
   }
   PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
-                                 *holder, kDontThrow);
+                                 *holder, Just(kDontThrow));
 
   if (it->IsElement()) {
     result = args.CallIndexedGetter(interceptor, it->index());
@@ -1006,7 +1006,7 @@ Maybe<PropertyAttributes> GetPropertyAttributesWithInterceptorInternal(
                                      Nothing<PropertyAttributes>());
   }
   PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
-                                 *holder, kDontThrow);
+                                 *holder, Just(kDontThrow));
   if (!interceptor->query()->IsUndefined(isolate)) {
     Handle<Object> result;
     if (it->IsElement()) {
@@ -1053,8 +1053,7 @@ Maybe<bool> SetPropertyWithInterceptorInternal(
                                      Nothing<bool>());
   }
   PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
-                                 *holder,
+                                 *holder, should_throw);
-                                 GetShouldThrow(isolate, should_throw));
 
   if (it->IsElement()) {
     // TODO(neis): In the future, we may want to actually return the
@@ -1087,8 +1086,7 @@ Maybe<bool> DefinePropertyWithInterceptorInternal(
                                      Nothing<bool>());
   }
   PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
-                                 *holder,
+                                 *holder, should_throw);
-                                 GetShouldThrow(isolate, should_throw));
 
   std::unique_ptr<v8::PropertyDescriptor> descriptor(
       new v8::PropertyDescriptor());
@@ -1512,7 +1510,7 @@ Maybe<bool> GetPropertyDescriptorWithInterceptor(LookupIterator* it,
   }
 
   PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
-                                 *holder, kDontThrow);
+                                 *holder, Just(kDontThrow));
   if (it->IsElement()) {
     result = args.CallIndexedDescriptor(interceptor, it->index());
   } else {
@@ -3538,7 +3536,7 @@ Maybe<bool> JSObject::DeletePropertyWithInterceptor(LookupIterator* it,
   }
 
   PropertyCallbackArguments args(isolate, interceptor->data(), *receiver,
-                                 *holder, should_throw);
+                                 *holder, Just(should_throw));
   Handle<Object> result;
   if (it->IsElement()) {
     result = args.CallIndexedDeleter(interceptor, it->index());