[turbofan] Fix a lazy deopt bug in Array.prototype.map

The bug was that the allocation of the result array (before the loop)
was using the outer frame state, thus returning the allocation's result
(an array full of holes) as the return value of the map operation in
case the allocation triggers a lazy deopt.

Bug: chromium:1104514
Change-Id: I9a6db8a5860472e1b438b6b54414938d61e166c1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2324249
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69129}

src/builtins/array-map.tq

@@ -3,18 +3,28 @@
 // found in the LICENSE file.
 
 namespace array {
+
+// Continuation for lazy deopt triggered by allocation of the result array.
+transitioning javascript builtin
+ArrayMapPreLoopLazyDeoptContinuation(
+    js-implicit context: NativeContext, receiver: JSAny)(
+    callback: JSAny, thisArg: JSAny, length: JSAny, result: JSAny): JSAny {
+  const jsreceiver = Cast<JSReceiver>(receiver) otherwise unreachable;
+  const outputArray = Cast<JSReceiver>(result) otherwise unreachable;
+  const numberLength = Cast<Number>(length) otherwise unreachable;
+
+  const callbackfn = Cast<Callable>(callback)
+      otherwise ThrowTypeError(MessageTemplate::kCalledNonCallable, callback);
+  return ArrayMapLoopContinuation(
+      jsreceiver, callbackfn, thisArg, outputArray, jsreceiver, kZero,
+      numberLength);
+}
+
 transitioning javascript builtin
 ArrayMapLoopEagerDeoptContinuation(
     js-implicit context: NativeContext, receiver: JSAny)(
     callback: JSAny, thisArg: JSAny, array: JSAny, initialK: JSAny,
     length: JSAny): JSAny {
-  // All continuation points in the optimized filter implementation are
-  // after the ToObject(O) call that ensures we are dealing with a
-  // JSReceiver.
-  //
-  // Also, this great mass of casts is necessary because the signature
-  // of Torque javascript builtins requires JSAny type for all parameters
-  // other than {context}.
   const jsreceiver = Cast<JSReceiver>(receiver) otherwise unreachable;
   const callbackfn = Cast<Callable>(callback) otherwise unreachable;
   const outputArray = Cast<JSReceiver>(array) otherwise unreachable;
@@ -31,17 +41,14 @@ ArrayMapLoopLazyDeoptContinuation(
     js-implicit context: NativeContext, receiver: JSAny)(
     callback: JSAny, thisArg: JSAny, array: JSAny, initialK: JSAny,
     length: JSAny, result: JSAny): JSAny {
-  // All continuation points in the optimized filter implementation are
-  // after the ToObject(O) call that ensures we are dealing with a
-  // JSReceiver.
   const jsreceiver = Cast<JSReceiver>(receiver) otherwise unreachable;
   const callbackfn = Cast<Callable>(callback) otherwise unreachable;
   const outputArray = Cast<JSReceiver>(array) otherwise unreachable;
   let numberK = Cast<Number>(initialK) otherwise unreachable;
   const numberLength = Cast<Number>(length) otherwise unreachable;
 
-  // This custom lazy deopt point is right after the callback. map() needs
-  // to pick up at the next step, which is setting the callback result in
+  // This custom lazy deopt point is right after the callback. The continuation
+  // needs to pick up at the next step, which is setting the callback result in
   // the output array. After incrementing k, we can glide into the loop
   // continuation builtin.
 

src/codegen/tnode.h

@@ -355,7 +355,7 @@ class TNode {
     return *this;
   }
 
-  bool is_null() { return node_ == nullptr; }
+  bool is_null() const { return node_ == nullptr; }
 
   operator compiler::Node*() const { return node_; }
 

src/compiler/js-call-reducer.cc

@@ -263,7 +263,8 @@ class JSCallReducerAssembler : public JSGraphAssembler {
                                TNode<Object> arg0, TNode<Object> arg1,
                                FrameState frame_state);
   // Used in special cases in which we are certain CreateArray does not throw.
-  TNode<JSArray> CreateArrayNoThrow(TNode<Object> ctor, TNode<Number> size);
+  TNode<JSArray> CreateArrayNoThrow(TNode<Object> ctor, TNode<Number> size,
+                                    FrameState frame_state);
 
   TNode<JSArray> AllocateEmptyJSArray(ElementsKind kind,
                                       const NativeContextRef& native_context);
@@ -1089,13 +1090,12 @@ TNode<Object> JSCallReducerAssembler::JSCallRuntime2(
   });
 }
 
-TNode<JSArray> JSCallReducerAssembler::CreateArrayNoThrow(TNode<Object> ctor,
-                                                          TNode<Number> size) {
+TNode<JSArray> JSCallReducerAssembler::CreateArrayNoThrow(
+    TNode<Object> ctor, TNode<Number> size, FrameState frame_state) {
   return AddNode<JSArray>(graph()->NewNode(
       javascript()->CreateArray(1, MaybeHandle<AllocationSite>()), ctor, ctor,
-      size, ContextInput(), FrameStateInput(), effect(), control()));
+      size, ContextInput(), frame_state, effect(), control()));
 }
-
 TNode<JSArray> JSCallReducerAssembler::AllocateEmptyJSArray(
     ElementsKind kind, const NativeContextRef& native_context) {
   // TODO(jgruber): Port AllocationBuilder to JSGraphAssembler.
@@ -1477,6 +1477,17 @@ struct MapFrameStateParams {
   TNode<Object> original_length;
 };
 
+FrameState MapPreLoopLazyFrameState(const MapFrameStateParams& params) {
+  DCHECK(params.a.is_null());
+  Node* checkpoint_params[] = {params.receiver, params.callback,
+                               params.this_arg, params.original_length};
+  return CreateJavaScriptBuiltinContinuationFrameState(
+      params.jsgraph, params.shared,
+      Builtins::kArrayMapPreLoopLazyDeoptContinuation, params.target,
+      params.context, checkpoint_params, arraysize(checkpoint_params),
+      params.outer_frame_state, ContinuationFrameStateMode::LAZY);
+}
+
 FrameState MapLoopLazyFrameState(const MapFrameStateParams& params,
                                  TNode<Number> k) {
   Node* checkpoint_params[] = {
@@ -1527,11 +1538,15 @@ TNode<JSArray> IteratingArrayBuiltinReducerAssembler::ReduceArrayPrototypeMap(
   // parameters.
   TNode<Object> array_ctor =
       Constant(native_context.GetInitialJSArrayMap(kind).GetConstructor());
-  TNode<JSArray> a = CreateArrayNoThrow(array_ctor, original_length);
 
   MapFrameStateParams frame_state_params{
-      jsgraph(), shared,     context,  target, outer_frame_state,
-      receiver,  fncallback, this_arg, a,      original_length};
+      jsgraph(), shared,     context,  target,       outer_frame_state,
+      receiver,  fncallback, this_arg, {} /* TBD */, original_length};
+
+  TNode<JSArray> a =
+      CreateArrayNoThrow(array_ctor, original_length,
+                         MapPreLoopLazyFrameState(frame_state_params));
+  frame_state_params.a = a;
 
   ThrowIfNotCallable(fncallback,
                      MapLoopLazyFrameState(frame_state_params, ZeroConstant()));

test/mjsunit/compiler/regress-1104514.js

@@ -0,0 +1,20 @@
+// Copyright 2020 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
+
+Array(32760);  // > JSArray::kInitialMaxFastElementArray
+
+function main() {
+  const a = [1, 2];
+  a.x = 666;
+  a.toString();
+  const aa = Array.prototype.map.call(a, v => v);
+  if (aa[0] != 1 || aa[1] != 2) { %SystemBreak(); }
+  a.z = 667;
+}
+
+for (var i = 0; i < 20000; ++i) {
+  main();
+}