[stable] [dart2wasm] Fix partial instantiation constants

Partial instantiation constants are closures. Those closures have vtables with all entries needed for the closure representation corresponding to the instantiated closure. The entries of those vtables have to either call the corresponding method of the generic closure, or are unreachable dummy entries. Dummy entries can be required due to clustering callees/callers together where a particular target doesn't support the name combination. The case that was incorrect is if the particular name combination did not get clustered with anything for the generic closure representation. Issue #56372 TEST=web/wasm/regress_56372_test Bug: #56372 Change-Id: I7a219237519c39d982b89ce272f33fb4d90cd173 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/380100 Cherry-pick-request: #56440 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380120 Reviewed-by: Ömer Ağacan <[email protected]> Commit-Queue: Martin Kustermann <[email protected]>
dart-lang · Aug 13, 2024 · c12bc56 · c12bc56
1 parent 475c337
commit c12bc56
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,8 +11,14 @@
   implicit setter for a field of generic type will store `null` instead of the
   field value. [#152029][]
 
+- Fixes a bug in the dart2wasm compiler that can trigger in certain situations
+  when using partial instantiations of generic tear-offs (constructors or static
+  methods) in constant expressions. [#56372][]
+
 [#152029]: https://github.com/flutter/flutter/issues/152029
 
+[#56372]: https://github.com/dart-lang/sdk/issues/56372
+
 ## 3.5.0
 
 ### Language

diff --git a/pkg/dart2wasm/lib/closures.dart b/pkg/dart2wasm/lib/closures.dart
@@ -125,7 +125,7 @@ class ClosureRepresentation {
 /// lexicographically according to their lists of names, corresponding to the
 /// order in which entry points taking named arguments will appear in vtables.
 class NameCombination implements Comparable<NameCombination> {
-  List<String> names;
+  final List<String> names;
 
   NameCombination(this.names);
 

diff --git a/pkg/dart2wasm/lib/constants.dart b/pkg/dart2wasm/lib/constants.dart
@@ -843,12 +843,12 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
     int positionalCount = tearOffConstant.function.positionalParameters.length;
     List<String> names =
         tearOffConstant.function.namedParameters.map((p) => p.name!).toList();
-    ClosureRepresentation representation = translator.closureLayouter
-        .getClosureRepresentation(0, positionalCount, names)!;
-    ClosureRepresentation instantiationRepresentation = translator
+    ClosureRepresentation instantiationOfTearOffRepresentation = translator
         .closureLayouter
+        .getClosureRepresentation(0, positionalCount, names)!;
+    ClosureRepresentation tearOffRepresentation = translator.closureLayouter
         .getClosureRepresentation(types.length, positionalCount, names)!;
-    w.StructType struct = representation.closureStruct;
+    w.StructType struct = instantiationOfTearOffRepresentation.closureStruct;
     w.RefType type = w.RefType.def(struct, nullable: false);
 
     final tearOffConstantInfo = ensureConstant(tearOffConstant)!;
@@ -904,37 +904,50 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
         return function;
       }
 
-      void fillVtableEntry(int posArgCount, List<String> argNames) {
-        int fieldIndex =
-            representation.fieldIndexForSignature(posArgCount, argNames);
-        int tearOffFieldIndex = tearOffClosure.representation
-            .fieldIndexForSignature(posArgCount, argNames);
-
-        w.FunctionType signature =
-            representation.getVtableFieldType(fieldIndex);
-        w.BaseFunction tearOffFunction = tearOffClosure.functions[
-            tearOffFieldIndex - tearOffClosure.representation.vtableBaseIndex];
-        w.BaseFunction function =
-            translator.globals.isDummyFunction(tearOffFunction)
-                ? translator.globals.getDummyFunction(signature)
-                : makeTrampoline(signature, tearOffFunction);
+      void fillVtableEntry(int posArgCount, NameCombination nameCombination) {
+        final fieldIndex = instantiationOfTearOffRepresentation
+            .fieldIndexForSignature(posArgCount, nameCombination.names);
+        final signature =
+            instantiationOfTearOffRepresentation.getVtableFieldType(fieldIndex);
+
+        w.BaseFunction function;
+        if (nameCombination.names.isNotEmpty &&
+            !tearOffRepresentation.nameCombinations.contains(nameCombination)) {
+          // This name combination only has
+          //   - non-generic closure / non-generic tear-off definitions
+          //   - non-generic callers
+          // => We make a dummy entry which is unreachable.
+          function = translator.globals.getDummyFunction(signature);
+        } else {
+          final int tearOffFieldIndex = tearOffRepresentation
+              .fieldIndexForSignature(posArgCount, nameCombination.names);
+          w.BaseFunction tearOffFunction = tearOffClosure.functions[
+              tearOffFieldIndex - tearOffRepresentation.vtableBaseIndex];
+          if (translator.globals.isDummyFunction(tearOffFunction)) {
+            // This name combination may not exist for the target, but got
+            // clustered together with other name combinations that do exist.
+            // => We make a dummy entry which is unreachable.
+            function = translator.globals.getDummyFunction(signature);
+          } else {
+            function = makeTrampoline(signature, tearOffFunction);
+          }
+        }
         b.ref_func(function);
       }
 
       void makeVtable() {
         b.ref_func(dynamicCallEntry);
-        if (representation.isGeneric) {
-          b.ref_func(representation.instantiationFunction);
-        }
+        assert(!instantiationOfTearOffRepresentation.isGeneric);
         for (int posArgCount = 0;
             posArgCount <= positionalCount;
             posArgCount++) {
-          fillVtableEntry(posArgCount, const []);
+          fillVtableEntry(posArgCount, NameCombination(const []));
         }
-        for (NameCombination combination in representation.nameCombinations) {
-          fillVtableEntry(positionalCount, combination.names);
+        for (NameCombination combination
+            in instantiationOfTearOffRepresentation.nameCombinations) {
+          fillVtableEntry(positionalCount, combination);
         }
-        b.struct_new(representation.vtableStruct);
+        b.struct_new(instantiationOfTearOffRepresentation.vtableStruct);
       }
 
       b.i32_const(info.classId);
@@ -946,7 +959,7 @@ class ConstantCreator extends ConstantVisitor<ConstantInfo?>
       for (final ty in types) {
         b.global_get(ty.global);
       }
-      b.struct_new(instantiationRepresentation.instantiationContextStruct!);
+      b.struct_new(tearOffRepresentation.instantiationContextStruct!);
 
       makeVtable();
       constants.instantiateConstant(

diff --git a/tests/web/wasm/regress_56372_test.dart b/tests/web/wasm/regress_56372_test.dart
@@ -0,0 +1,22 @@
+// Copyright (c) 2024, 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 main() {
+  final funs = <String Function({Object? shared})>[
+    foo,
+    bar,
+  ];
+
+  final one = int.parse('1');
+  final barS = funs[one] as String Function({Object? barSpecific});
+  if (barS(barSpecific: 1) != 'bar(null, 1)') {
+    throw 'failed: ${barS(barSpecific: 1)}';
+  }
+}
+
+String foo<T>({Object? shared, Object? fooSpecific}) =>
+    'foo<$T>($shared, $fooSpecific)';
+
+String bar({Object? shared, Object? barSpecific}) =>
+    'bar($shared, $barSpecific)';