[turbofan] Fix iterator-generator issue with --turbo-optimize-apply

Fuzzing found a problem with --turbo-optimize-apply when the
Array.prototype iterator is replaced with a generator function.
We can the issue by installing a protector on the array iterator.

This CL also defines the --turbo-optimize-apply as 'future' to get
more test coverage.

Bug: v8:9974
Change-Id: Id5bc68fde98ea5d1f6a951c4381ca6283b892632
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2966058
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75197}

src/compiler/js-call-reducer.cc

@@ -4152,6 +4152,13 @@ Reduction JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread(
     return NoChange();
   }
 
+  // For call/construct with spread, we need to also install a code
+  // dependency on the array iterator lookup protector cell to ensure
+  // that no one messed with the %ArrayIteratorPrototype%.next method.
+  if (IsCallOrConstructWithSpread(node)) {
+    if (!dependencies()->DependOnArrayIteratorProtector()) return NoChange();
+  }
+
   int new_argument_count;
   if (arguments_list->opcode() == IrOpcode::kJSCreateLiteralArray) {
     // Find array length and elements' kind from the feedback's allocation

src/flags/flag-definitions.h

@@ -865,6 +865,7 @@ DEFINE_BOOL(turbo_compress_translation_arrays, false,
 DEFINE_BOOL(turbo_inline_js_wasm_calls, false, "inline JS->Wasm calls")
 
 DEFINE_BOOL(turbo_optimize_apply, false, "optimize Function.prototype.apply")
+DEFINE_WEAK_IMPLICATION(future, turbo_optimize_apply)
 
 DEFINE_BOOL(turbo_collect_feedback_in_generic_lowering, true,
             "enable experimental feedback collection in generic lowering.")

test/mjsunit/compiler/call-with-arraylike-or-spread-5.js

@@ -0,0 +1,55 @@
+// Copyright 2021 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 --turbo-optimize-apply --opt
+
+// These tests do not work well if this script is run more than once (e.g.
+// --stress-opt); after a few runs the whole function is immediately compiled
+// and assertions would fail. We prevent re-runs.
+// Flags: --nostress-opt --no-always-opt
+
+// Some of the tests rely on optimizing/deoptimizing at predictable moments, so
+// this is not suitable for deoptimization fuzzing.
+// Flags: --deopt-every-n-times=0
+
+// Test for optimization of CallWithSpread when the array iterator is replaced
+// with a generator function.
+//
+// Note: this test must be in a separate file because the test invalidates a
+// protector, which then remains invalidated.
+(function () {
+  "use strict";
+
+  // This invalidates the DependOnArrayIteratorProtector.
+  Object.defineProperty(Array.prototype, Symbol.iterator, {
+    value: function* () {
+      yield 42;
+    },
+  });
+
+  var log_got_interpreted = true;
+
+  function log(a) {
+    assertEquals(1, arguments.length);
+    log_got_interpreted = %IsBeingInterpreted();
+    return a;
+  }
+  function foo() {
+    return log(...[1]);
+  }
+
+  %PrepareFunctionForOptimization(log);
+  %PrepareFunctionForOptimization(foo);
+  assertEquals(42, foo());
+  assertTrue(log_got_interpreted);
+
+  // Compile foo.
+  %OptimizeFunctionOnNextCall(log);
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(42, foo());
+  // The call with spread should not have been inlined, because of the
+  // generator/iterator.
+  assertFalse(log_got_interpreted);
+  assertOptimized(foo);
+})();

test/mjsunit/compiler/call-with-arraylike-or-spread-6.js

@@ -0,0 +1,55 @@
+// Copyright 2021 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 --turbo-optimize-apply --opt
+
+// These tests do not work well if this script is run more than once (e.g.
+// --stress-opt); after a few runs the whole function is immediately compiled
+// and assertions would fail. We prevent re-runs.
+// Flags: --nostress-opt --no-always-opt
+
+// Some of the tests rely on optimizing/deoptimizing at predictable moments, so
+// this is not suitable for deoptimization fuzzing.
+// Flags: --deopt-every-n-times=0
+
+// Test for optimization of CallWithSpread when the array iterator is replaced
+// with a generator function and the array in empty.
+//
+// Note: this test must be in a separate file because the test invalidates a
+// protector, which then remains invalidated.
+(function () {
+  "use strict";
+
+  // This invalidates the DependOnArrayIteratorProtector.
+  Object.defineProperty(Array.prototype, Symbol.iterator, {
+    value: function* () {
+      yield 42;
+    },
+  });
+
+  var log_got_interpreted = true;
+
+  function log(a) {
+    assertEquals(1, arguments.length);
+    log_got_interpreted = %IsBeingInterpreted();
+    return a;
+  }
+  function foo() {
+    return log(...[]);
+  }
+
+  %PrepareFunctionForOptimization(log);
+  %PrepareFunctionForOptimization(foo);
+  assertEquals(42, foo());
+  assertTrue(log_got_interpreted);
+
+  // Compile foo.
+  %OptimizeFunctionOnNextCall(log);
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(42, foo());
+  // The call with spread should not have been inlined, because of the
+  // generator/iterator.
+  assertFalse(log_got_interpreted);
+  assertOptimized(foo);
+})();

test/mjsunit/compiler/call-with-arraylike-or-spread-7.js

@@ -0,0 +1,67 @@
+// Copyright 2021 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 --turbo-optimize-apply --opt
+
+// These tests do not work well if this script is run more than once (e.g.
+// --stress-opt); after a few runs the whole function is immediately compiled
+// and assertions would fail. We prevent re-runs.
+// Flags: --nostress-opt --no-always-opt
+
+// Some of the tests rely on optimizing/deoptimizing at predictable moments, so
+// this is not suitable for deoptimization fuzzing.
+// Flags: --deopt-every-n-times=0
+
+// Test for optimization of CallWithSpread when the array iterator is replaced
+// with a generator function after a function is compiled.
+//
+// Note: this test must be in a separate file because the test invalidates a
+// protector, which then remains invalidated.
+(function () {
+  "use strict";
+  var log_got_interpreted = true;
+
+  function log(a) {
+    assertEquals(1, arguments.length);
+    log_got_interpreted = %IsBeingInterpreted();
+    return a;
+  }
+  function foo() {
+    return log(...[1]);
+  }
+
+  %PrepareFunctionForOptimization(log);
+  %PrepareFunctionForOptimization(foo);
+  assertEquals(1, foo());
+  assertTrue(log_got_interpreted);
+
+  // Compile foo.
+  %OptimizeFunctionOnNextCall(log);
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(1, foo());
+  // The call with spread should have been inlined.
+  assertFalse(log_got_interpreted);
+  assertOptimized(foo);
+
+  // This invalidates the DependOnArrayIteratorProtector and causes deopt.
+  Object.defineProperty(Array.prototype, Symbol.iterator, {
+    value: function* () {
+      yield 42;
+    },
+  });
+
+  // Now we expect the value yielded by the generator.
+  assertEquals(42, foo());
+  assertFalse(log_got_interpreted);
+  assertUnoptimized(foo);
+
+  // Recompile 'foo'.
+  %PrepareFunctionForOptimization(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(42, foo());
+  // The call with spread will not be inlined because we have redefined the
+  // array iterator.
+  assertFalse(log_got_interpreted);
+  assertOptimized(foo);
+})();