[vm] Bail out inlining on wrong optional arguments

TEST=runtime/tests/vm/dart/regress_flutter89482_test.dart

Before: assert (debug mode)
After:
  Closure Calls (1)
  => other (deopt count 0)
     Bailout: optional arg mismatch

Bug: https://github.com/flutter/flutter/issues/89482

Change-Id: I5b0a9b738465d12c47304e9309bfa8f701c5aae6
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64c-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212570
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

runtime/tests/vm/dart/regress_flutter89482_2_test.dart

@@ -0,0 +1,16 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+void other([String something = "ok"]) {
+  print(something);
+}
+
+void main(List<String> args) {
+  Function x = other;
+  if (x is void Function({List<String> somethingElse})) {
+    x(somethingElse: args);
+  } else {
+    x();
+  }
+}

runtime/tests/vm/dart/regress_flutter89482_test.dart

@@ -0,0 +1,19 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// This should not hit an ASSERT in the inliner that the call site has a
+// different number of named arguments.
+
+void other({String something = "ok"}) {
+  print(something);
+}
+
+void main(List<String> args) {
+  Function x = other;
+  if (x is void Function(List<String>)) {
+    x(args);
+  } else {
+    x();
+  }
+}

runtime/tests/vm/dart_2/regress_flutter89482_2_test.dart

@@ -0,0 +1,16 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+void other([String something = "ok"]) {
+  print(something);
+}
+
+void main(List<String> args) {
+  Function x = other;
+  if (x is void Function({List<String> somethingElse})) {
+    x(somethingElse: args);
+  } else {
+    x();
+  }
+}

runtime/tests/vm/dart_2/regress_flutter89482_test.dart

@@ -0,0 +1,19 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// This should not hit an ASSERT in the inliner that the call site has a
+// different number of named arguments.
+
+void other({String something = "ok"}) {
+  print(something);
+}
+
+void main(List<String> args) {
+  Function x = other;
+  if (x is void Function(List<String>)) {
+    x(args);
+  } else {
+    x();
+  }
+}

runtime/vm/compiler/backend/inliner.cc

@@ -996,6 +996,17 @@ class CallSiteInliner : public ValueObject {
       return false;
     }
 
+    if ((function.HasOptionalPositionalParameters() ||
+         function.HasOptionalNamedParameters()) &&
+        !function.AreValidArguments(function.NumTypeParameters(),
+                                    arguments->length(), argument_names,
+                                    nullptr)) {
+      TRACE_INLINING(THR_Print("     Bailout: optional arg mismatch\n"));
+      PRINT_INLINING_TREE("Optional arg mismatch", &call_data->caller,
+                          &function, call_data->call);
+      return false;
+    }
+
     // Abort if this is a recursive occurrence.
     Definition* call = call_data->call;
     // Added 'volatile' works around a possible GCC 4.9 compiler bug.
@@ -1077,17 +1088,17 @@ class CallSiteInliner : public ValueObject {
           COMPILER_TIMINGS_TIMER_SCOPE(thread(), PopulateWithICData);
 
 #if defined(DART_PRECOMPILER) && !defined(TARGET_ARCH_IA32)
-        if (CompilerState::Current().is_aot()) {
-          callee_graph->PopulateWithICData(parsed_function->function());
-        }
+          if (CompilerState::Current().is_aot()) {
+            callee_graph->PopulateWithICData(parsed_function->function());
+          }
 #endif
 
-        // If we inline a function which is intrinsified without a fall-through
-        // to IR code, we will not have any ICData attached, so we do it
-        // manually here.
-        if (!CompilerState::Current().is_aot() && function.is_intrinsic()) {
-          callee_graph->PopulateWithICData(parsed_function->function());
-        }
+          // If we inline a function which is intrinsified without a
+          // fall-through to IR code, we will not have any ICData attached, so
+          // we do it manually here.
+          if (!CompilerState::Current().is_aot() && function.is_intrinsic()) {
+            callee_graph->PopulateWithICData(parsed_function->function());
+          }
         }
 
         // The parameter stubs are a copy of the actual arguments providing
@@ -1584,10 +1595,14 @@ class CallSiteInliner : public ValueObject {
     intptr_t arg_count = arguments->length();
     intptr_t param_count = function.NumParameters();
     intptr_t fixed_param_count = function.num_fixed_parameters();
+    intptr_t argument_names_count =
+        (argument_names.IsNull()) ? 0 : argument_names.Length();
     ASSERT(fixed_param_count <= arg_count - first_arg_index);
     ASSERT(arg_count - first_arg_index <= param_count);
 
     if (function.HasOptionalPositionalParameters()) {
+      // Arguments mismatch: Caller supplied unsupported named argument.
+      ASSERT(argument_names_count == 0);
       // Create a stub for each optional positional parameters with an actual.
       for (intptr_t i = first_arg_index + fixed_param_count; i < arg_count;
            ++i) {
@@ -1609,12 +1624,10 @@ class CallSiteInliner : public ValueObject {
 
     ASSERT(function.HasOptionalNamedParameters());
 
-    // Passed arguments (not counting optional type args) must match fixed
-    // parameters plus named arguments.
-    intptr_t argument_names_count =
-        (argument_names.IsNull()) ? 0 : argument_names.Length();
-    ASSERT((arg_count - first_arg_index) ==
-           (fixed_param_count + argument_names_count));
+    const intptr_t positional_args =
+        arg_count - first_arg_index - argument_names_count;
+    // Arguments mismatch: Caller supplied unsupported positional argument.
+    ASSERT(positional_args == fixed_param_count);
 
     // Fast path when no optional named parameters are given.
     if (argument_names_count == 0) {