Reland of Port Proxy SetProperty trap builtin to Torque

Reverted CL is in https://chromium-review.googlesource.com/c/v8/v8/+/1585269
This includes fix for ThrowTypeErrorIfStrict and add regression test.

Spec: https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-set-p-v-receiver
Bug: v8:6664, v8:9234
Change-Id: I785df3f12f619e2e0fe7b011b72043758e4083e3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1604071Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#61497}

BUILD.gn

@@ -948,6 +948,7 @@ torque_files = [
   "src/builtins/object-fromentries.tq",
   "src/builtins/proxy-constructor.tq",
   "src/builtins/proxy-get-property.tq",
+  "src/builtins/proxy-set-property.tq",
   "src/builtins/proxy-revocable.tq",
   "src/builtins/proxy-revoke.tq",
   "src/builtins/proxy.tq",

src/builtins/base.tq

@@ -1276,6 +1276,9 @@ extern macro ThrowTypeError(implicit context: Context)(
     constexpr MessageTemplate, Object, Object): never;
 extern macro ThrowTypeError(implicit context: Context)(
     constexpr MessageTemplate, Object, Object, Object): never;
+extern transitioning runtime ThrowTypeErrorIfStrict(implicit context: Context)(
+    Smi, Object, Object): void;
+
 extern macro ArraySpeciesCreate(Context, Object, Number): JSReceiver;
 extern macro ArrayCreate(implicit context: Context)(Number): JSArray;
 extern macro BuildAppendJSArray(
@@ -1894,6 +1897,7 @@ extern macro Float64Constant(constexpr int31): float64;
 extern macro Float64Constant(constexpr float64): float64;
 extern macro SmiConstant(constexpr int31): Smi;
 extern macro SmiConstant(constexpr Smi): Smi;
+extern macro SmiConstant(constexpr MessageTemplate): Smi;
 extern macro BoolConstant(constexpr bool): bool;
 extern macro StringConstant(constexpr string): String;
 extern macro LanguageModeConstant(constexpr LanguageMode): LanguageMode;

src/builtins/builtins-definitions.h

@@ -826,7 +826,6 @@ namespace internal {
                                                                                \
   /* Proxy */                                                                  \
   TFS(ProxyHasProperty, kProxy, kName)                                         \
-  TFS(ProxySetProperty, kProxy, kName, kValue, kReceiverValue)                 \
                                                                                \
   /* Reflect */                                                                \
   ASM(ReflectApply, Dummy)                                                     \

src/builtins/builtins-proxy-gen.cc

@@ -338,93 +338,6 @@ TF_BUILTIN(ProxyHasProperty, ProxiesCodeStubAssembler) {
                  StringConstant("has"), proxy);
 }
 
-TF_BUILTIN(ProxySetProperty, ProxiesCodeStubAssembler) {
-  Node* context = Parameter(Descriptor::kContext);
-  Node* proxy = Parameter(Descriptor::kProxy);
-  Node* name = Parameter(Descriptor::kName);
-  Node* value = Parameter(Descriptor::kValue);
-  Node* receiver = Parameter(Descriptor::kReceiverValue);
-
-  CSA_ASSERT(this, IsJSProxy(proxy));
-
-  // 1. Assert: IsPropertyKey(P) is true.
-  CSA_ASSERT(this, TaggedIsNotSmi(name));
-  CSA_ASSERT(this, IsName(name));
-
-  Label throw_proxy_handler_revoked(this, Label::kDeferred),
-      trap_undefined(this), failure(this, Label::kDeferred),
-      continue_checks(this), success(this),
-      private_symbol(this, Label::kDeferred);
-
-  GotoIf(IsPrivateSymbol(name), &private_symbol);
-
-  // 2. Let handler be O.[[ProxyHandler]].
-  Node* handler = LoadObjectField(proxy, JSProxy::kHandlerOffset);
-
-  // 3. If handler is null, throw a TypeError exception.
-  GotoIfNot(IsJSReceiver(handler), &throw_proxy_handler_revoked);
-
-  // 4. Assert: Type(handler) is Object.
-  CSA_ASSERT(this, IsJSReceiver(handler));
-
-  // 5. Let target be O.[[ProxyTarget]].
-  Node* target = LoadObjectField(proxy, JSProxy::kTargetOffset);
-
-  // 6. Let trap be ? GetMethod(handler, "set").
-  // 7. If trap is undefined, then (see 7.a below).
-  Handle<Name> set_string = factory()->set_string();
-  Node* trap = GetMethod(context, handler, set_string, &trap_undefined);
-
-  // 8. Let booleanTrapResult be ToBoolean(? Call(trap, handler,
-  // « target, P, V, Receiver »)).
-  // 9. If booleanTrapResult is false, return false.
-  BranchIfToBooleanIsTrue(
-      CallJS(CodeFactory::Call(isolate(),
-                               ConvertReceiverMode::kNotNullOrUndefined),
-             context, trap, handler, target, name, value, receiver),
-      &continue_checks, &failure);
-
-  BIND(&continue_checks);
-  {
-    // 9. Let targetDesc be ? target.[[GetOwnProperty]](P).
-    Label return_result(this);
-    Return(CheckGetSetTrapResult(context, target, proxy, name, value,
-                                 JSProxy::kSet));
-  }
-
-  BIND(&failure);
-  {
-    CallRuntime(Runtime::kThrowTypeErrorIfStrict, context,
-                SmiConstant(MessageTemplate::kProxyTrapReturnedFalsishFor),
-                HeapConstant(set_string), name);
-    Goto(&success);
-  }
-
-  // 12. Return true.
-  BIND(&success);
-  Return(value);
-
-  BIND(&private_symbol);
-  {
-    Label failure(this);
-
-    CallRuntime(Runtime::kThrowTypeErrorIfStrict, context,
-                SmiConstant(MessageTemplate::kProxyPrivate));
-    Return(UndefinedConstant());
-  }
-
-  BIND(&trap_undefined);
-  {
-    // 7.a. Return ? target.[[Set]](P, V, Receiver).
-    CallRuntime(Runtime::kSetPropertyWithReceiver, context, target, name, value,
-                receiver);
-    Return(value);
-  }
-
-  BIND(&throw_proxy_handler_revoked);
-  ThrowTypeError(context, MessageTemplate::kProxyRevoked, "set");
-}
-
 Node* ProxiesCodeStubAssembler::CheckGetSetTrapResult(
     Node* context, Node* target, Node* proxy, Node* name, Node* trap_result,
     JSProxy::AccessKind access_kind) {

src/builtins/builtins-proxy-gen.h

@@ -18,16 +18,6 @@ class ProxiesCodeStubAssembler : public CodeStubAssembler {
   explicit ProxiesCodeStubAssembler(compiler::CodeAssemblerState* state)
       : CodeStubAssembler(state) {}
 
-  // ES6 section 9.5.8 [[Get]] ( P, Receiver )
-  // name should not be an index.
-  Node* ProxyGetProperty(Node* context, Node* proxy, Node* name,
-                         Node* receiver);
-
-  // ES6 section 9.5.9 [[Set]] ( P, V, Receiver )
-  // name should not be an index.
-  Node* ProxySetProperty(Node* context, Node* proxy, Node* name, Node* value,
-                         Node* receiver);
-
   Node* AllocateProxy(Node* target, Node* handler, Node* context);
   Node* AllocateProxyRevokeFunction(Node* proxy, Node* context);
 

src/builtins/proxy-get-property.tq

@@ -10,10 +10,6 @@ namespace proxy {
   GetPropertyWithReceiver(implicit context: Context)(Object, Name, Object, Smi):
       Object;
 
-  extern transitioning macro ProxiesCodeStubAssembler::CheckGetSetTrapResult(
-      implicit context:
-          Context)(Object, JSProxy, Name, Object, constexpr int31): Object;
-
   // ES #sec-proxy-object-internal-methods-and-internal-slots-get-p-receiver
   // https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-get-p-receiver
   transitioning builtin

src/builtins/proxy-set-property.tq

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include 'src/builtins/builtins-proxy-gen.h'
+
+namespace proxy {
+
+  extern transitioning runtime
+  SetPropertyWithReceiver(implicit context:
+                              Context)(Object, Name, Object, Object): void;
+
+  transitioning macro CallThrowTypeErrorIfStrict(implicit context: Context)(
+      message: constexpr MessageTemplate) {
+    ThrowTypeErrorIfStrict(SmiConstant(message), Null, Null);
+  }
+
+  // ES #sec-proxy-object-internal-methods-and-internal-slots-set-p-v-receiver
+  // https://tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-set-p-v-receiver
+  transitioning builtin
+  ProxySetProperty(implicit context: Context)(
+      proxy: JSProxy, name: Name, value: Object,
+      receiverValue: Object): Object {
+    // 1. Assert: IsPropertyKey(P) is true.
+    assert(TaggedIsNotSmi(name));
+    assert(IsName(name));
+
+    if (IsPrivateSymbol(name)) {
+      CallThrowTypeErrorIfStrict(kProxyPrivate);
+      return Undefined;
+    }
+
+    // 2. Let handler be O.[[ProxyHandler]].
+    const handler: Object = proxy.handler;
+
+    try {
+      // 3. If handler is null, throw a TypeError exception.
+      // 4. Assert: Type(handler) is Object.
+      const handlerJSReceiver =
+          Cast<JSReceiver>(handler) otherwise ThrowProxyHandlerRevoked;
+
+      // 5. Let target be O.[[ProxyTarget]].
+      const target = proxy.target;
+
+      // 6. Let trap be ? GetMethod(handler, "set").
+      // 7. If trap is undefined, then (see 7.a below).
+      const trap: Callable = GetMethod(handlerJSReceiver, 'set')
+          otherwise goto TrapUndefined(target);
+
+      // 8. Let booleanTrapResult be ToBoolean(? Call(trap, handler,
+      // « target, P, V, Receiver »)).
+      // 9. If booleanTrapResult is false, return false.
+      // 10. Let targetDesc be ? target.[[GetOwnProperty]](P).
+      // 11. If targetDesc is not undefined and targetDesc.[[Configurable]] is
+      // false, then
+      //    a. If IsDataDescriptor(targetDesc) is true and
+      //    targetDesc.[[Writable]] is false, then
+      //      i. If SameValue(V, targetDesc.[[Value]]) is false, throw a
+      //      TypeError exception.
+      //    b. If IsAccessorDescriptor(targetDesc) is true, then
+      //      i. If targetDesc.[[Set]] is undefined, throw a TypeError
+      //      exception.
+      // 12. Return true.
+      const trapResult = Call(
+          context, trap, handlerJSReceiver, target, name, value, receiverValue);
+      if (BranchIfToBooleanIsTrue(trapResult)) {
+        return CheckGetSetTrapResult(
+            target, proxy, name, trapResult, kProxySet);
+      }
+      ThrowTypeErrorIfStrict(
+          SmiConstant(kProxyTrapReturnedFalsishFor), 'set', name);
+      return value;
+    }
+    label TrapUndefined(target: Object) {
+      // 7.a. Return ? target.[[Set]](P, V, Receiver).
+      SetPropertyWithReceiver(target, name, value, receiverValue);
+      return value;
+    }
+    label ThrowProxyHandlerRevoked deferred {
+      assert(handler == Null);
+      ThrowTypeError(kProxyRevoked, 'set');
+    }
+  }
+}

src/builtins/proxy.tq

@@ -20,12 +20,20 @@ namespace proxy {
     return false;
   }
 
+  extern transitioning macro ProxiesCodeStubAssembler::CheckGetSetTrapResult(
+      implicit context:
+          Context)(Object, JSProxy, Name, Object, constexpr int31): Object;
+
   const kProxyNonObject: constexpr MessageTemplate
   generates 'MessageTemplate::kProxyNonObject';
   const kProxyHandlerOrTargetRevoked: constexpr MessageTemplate
   generates 'MessageTemplate::kProxyHandlerOrTargetRevoked';
   const kProxyRevoked: constexpr MessageTemplate
   generates 'MessageTemplate::kProxyRevoked';
+  const kProxyTrapReturnedFalsishFor: constexpr MessageTemplate
+  generates 'MessageTemplate::kProxyTrapReturnedFalsishFor';
+  const kProxyPrivate: constexpr MessageTemplate
+  generates 'MessageTemplate::kProxyPrivate';
 
   const kProxyGet: constexpr int31
   generates 'JSProxy::AccessKind::kGet';

test/mjsunit/es6/regress/regress-9234.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+(function returnFalsishStrict() {
+  "use strict";
+
+  function trySet(o) {
+    o["bla"] = 0;
+  }
+
+  var proxy = new Proxy({}, {});
+  var proxy2 = new Proxy({}, { set() { return ""; } });
+
+  trySet(proxy);
+  trySet(proxy);
+  assertThrows(() => trySet(proxy2), TypeError);
+})();
+
+(function privateSymbolStrict() {
+  "use strict";
+  var proxy = new Proxy({}, {});
+  var proxy2 = new Proxy({a: 1}, { set() { return true; } });
+
+  function trySet(o) {
+    var symbol = o == proxy2 ? %CreatePrivateSymbol("private"): 1;
+    o[symbol] = 0;
+  }
+
+  trySet(proxy);
+  trySet(proxy);
+  assertThrows(() => trySet(proxy2), TypeError);
+})();