[torque] disallow tail calls from macros

Bug: v8:7793
Change-Id: I36daa0ef26cc7c274c64cfdba7e3a196677a7bc4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1718156Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62916}

src/builtins/array-shift.tq

@@ -3,11 +3,10 @@
 // found in the LICENSE file.
 
 namespace array_shift {
-  extern builtin ArrayShift(Context, JSFunction, Object, int32);
+  extern builtin ArrayShift(Context, JSFunction, Object, int32): Object;
 
-  macro TryFastArrayShift(implicit context: Context)(
-      receiver: Object, arguments: Arguments): Object
-      labels Slow {
+  macro TryFastArrayShift(implicit context: Context)(receiver: Object): Object
+      labels Slow, Runtime {
     const array: FastJSArray = Cast<FastJSArray>(receiver) otherwise Slow;
     let witness = NewFastJSArrayWitness(array);
 
@@ -17,31 +16,24 @@ namespace array_shift {
       return Undefined;
     }
 
-    try {
-      const newLength = array.length - 1;
+    const newLength = array.length - 1;
 
-      // Check that we're not supposed to right-trim the backing store, as
-      // implemented in elements.cc:ElementsAccessorBase::SetLengthImpl.
-      if ((newLength + newLength + kMinAddedElementsCapacity) <
-          array.elements.length) {
-        goto Runtime;
-      }
+    // Check that we're not supposed to right-trim the backing store, as
+    // implemented in elements.cc:ElementsAccessorBase::SetLengthImpl.
+    if ((newLength + newLength + kMinAddedElementsCapacity) <
+        array.elements.length) {
+      goto Runtime;
+    }
 
-      // Check that we're not supposed to left-trim the backing store, as
-      // implemented in elements.cc:FastElementsAccessor::MoveElements.
-      if (newLength > kMaxCopyElements) goto Runtime;
+    // Check that we're not supposed to left-trim the backing store, as
+    // implemented in elements.cc:FastElementsAccessor::MoveElements.
+    if (newLength > kMaxCopyElements) goto Runtime;
 
-      const result = witness.LoadElementOrUndefined(0);
-      witness.ChangeLength(newLength);
-      witness.MoveElements(0, 1, Convert<intptr>(newLength));
-      witness.StoreHole(newLength);
-      return result;
-    }
-    label Runtime {
-      tail ArrayShift(
-          context, LoadTargetFromFrame(), Undefined,
-          Convert<int32>(arguments.length));
-    }
+    const result = witness.LoadElementOrUndefined(0);
+    witness.ChangeLength(newLength);
+    witness.MoveElements(0, 1, Convert<intptr>(newLength));
+    witness.StoreHole(newLength);
+    return result;
   }
 
   transitioning macro GenericArrayShift(implicit context:
@@ -105,10 +97,15 @@ namespace array_shift {
   transitioning javascript builtin ArrayPrototypeShift(
       js-implicit context: Context, receiver: Object)(...arguments): Object {
     try {
-      return TryFastArrayShift(receiver, arguments) otherwise Slow;
+      return TryFastArrayShift(receiver) otherwise Slow, Runtime;
     }
     label Slow {
       return GenericArrayShift(receiver);
     }
+    label Runtime {
+      tail ArrayShift(
+          context, LoadTargetFromFrame(), Undefined,
+          Convert<int32>(arguments.length));
+    }
   }
 }

src/builtins/array-unshift.tq

@@ -3,22 +3,7 @@
 // found in the LICENSE file.
 
 namespace array_unshift {
-  extern builtin ArrayUnshift(Context, JSFunction, Object, int32);
-
-  macro TryFastArrayUnshift(
-      context: Context, receiver: Object, arguments: Arguments): never
-      labels Slow {
-    const array: FastJSArray = Cast<FastJSArray>(receiver) otherwise Slow;
-    array::EnsureWriteableFastElements(array);
-
-    const map: Map = array.map;
-    if (!IsExtensibleMap(map)) goto Slow;
-    EnsureArrayLengthWritable(map) otherwise Slow;
-
-    tail ArrayUnshift(
-        context, LoadTargetFromFrame(), Undefined,
-        Convert<int32>(arguments.length));
-  }
+  extern builtin ArrayUnshift(Context, JSFunction, Object, int32): Object;
 
   transitioning macro GenericArrayUnshift(
       context: Context, receiver: Object, arguments: Arguments): Number {
@@ -95,9 +80,18 @@ namespace array_unshift {
   transitioning javascript builtin ArrayPrototypeUnshift(
       js-implicit context: Context, receiver: Object)(...arguments): Object {
     try {
-      TryFastArrayUnshift(context, receiver, arguments) otherwise Baseline;
+      const array: FastJSArray = Cast<FastJSArray>(receiver) otherwise Slow;
+      array::EnsureWriteableFastElements(array);
+
+      const map: Map = array.map;
+      if (!IsExtensibleMap(map)) goto Slow;
+      EnsureArrayLengthWritable(map) otherwise Slow;
+
+      tail ArrayUnshift(
+          context, LoadTargetFromFrame(), Undefined,
+          Convert<int32>(arguments.length));
     }
-    label Baseline {
+    label Slow {
       return GenericArrayUnshift(context, receiver, arguments);
     }
   }

src/torque/implementation-visitor.cc

@@ -2118,6 +2118,18 @@ VisitResult ImplementationVisitor::GenerateCall(
 
   const Type* return_type = callable->signature().return_type;
 
+  if (is_tailcall) {
+    if (Builtin* builtin = Builtin::DynamicCast(CurrentCallable::Get())) {
+      const Type* outer_return_type = builtin->signature().return_type;
+      if (!return_type->IsSubtypeOf(outer_return_type)) {
+        Error("Cannot tailcall, type of result is ", *return_type,
+              " but should be a subtype of ", *outer_return_type, ".");
+      }
+    } else {
+      Error("Tail calls are only allowed from builtins");
+    }
+  }
+
   std::vector<VisitResult> converted_arguments;
   StackRange argument_range = assembler().TopRange(0);
   std::vector<std::string> constexpr_arguments;