[dart2wasm] Fix dynamic modules bugs.

- For direct calls in selector branch, ensure 'if' branches have correct inputs.

- For overrideable selectors, the receiver type cannot be known in the main module so use 'top' type. Technically we could do better if selectors tracked interfaces they targeted. Then we could take the LUB of all those classes. But this would be a significant refactor for a small benefit.

- Type checks on classes defined in the main module should use the class ID ranges from the main module rather than those from the dynamic submodule. This only applies to non-dynamic module extendable types (otherwise we'd use the RTT checks). So we know the class can only exist in one of the range sets, not both.

I discovered (1) from running Flutter which led me to create this test which uncovered (2) and (3).

Change-Id: I80f39835f66aa7cf0cff527341e3f4a948a7a0cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430360
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This commit is contained in:
Nate Biggs
2025-05-22 16:15:58 -07:00
committed by Commit Queue
parent 09cdebdd59
commit 63ee73a2ed
8 changed files with 116 additions and 10 deletions
+12 -4
View File
@@ -678,10 +678,18 @@ class ClassIdNumbering {
_concreteSubclassIdRangeForDynamicSubmodule);
}
List<Range> getConcreteClassIdRangeForCurrentModule(Class klass) {
return translator.isDynamicSubmodule
? getConcreteClassIdRangeForDynamicSubmodule(klass)
: getConcreteClassIdRangeForMainModule(klass);
/// In case the [klass] is from a dynamic module the returned class id
/// ranges may be relative. The caller has to ensure to use them
/// appropriately.
List<Range> getConcreteClassIdRangeForClass(Class klass) {
// We cannot return class id ranges for [klass] if there can be more
// classes in future dynamic module compilations.
assert(!klass.isDynamicSubmoduleExtendable(translator.coreTypes));
return !translator.isDynamicSubmodule ||
klass.enclosingLibrary.isFromMainModule(translator.coreTypes)
? getConcreteClassIdRangeForMainModule(klass)
: getConcreteClassIdRangeForDynamicSubmodule(klass);
}
List<Range> _getConcreteClassIdRange(Class klass,
+3 -1
View File
@@ -202,7 +202,9 @@ class SelectorInfo {
}
}
assert(returns.length <= outputSets.length);
inputSets[0].add(translator.translateType(receiver));
inputSets[0].add(isDynamicSubmoduleOverridable
? translator.topInfo.nonNullableType
: translator.translateType(receiver));
for (int i = 0; i < positional.length; i++) {
DartType type = positional[i];
inputSets[1 + i].add(translator.translateType(type));
+1 -1
View File
@@ -835,7 +835,7 @@ class DynamicModuleInfo {
// Check if the invocation is checked or unchecked and use the
// appropriate offset.
ib.local_get(ib.locals[function.type.inputs.length - 1]);
ib.if_(const [], localSignature.outputs);
ib.if_(localSignature.inputs, localSignature.outputs);
ib.invoke(translator.directCallTarget(uncheckedTarget));
ib.else_();
ib.invoke(translator.directCallTarget(checkedTarget));
+33 -4
View File
@@ -767,7 +767,7 @@ class IsCheckerCallTarget extends CallTarget {
// Always inline single class-id range checks (no branching, simply loads,
// arithmetic and unsigned compare).
final ranges = translator.classIdNumbering
.getConcreteClassIdRangeForCurrentModule(interfaceClass);
.getConcreteClassIdRangeForClass(interfaceClass);
return ranges.length <= 1;
}
@@ -883,10 +883,37 @@ class IsCheckerCodeGenerator implements CodeGenerator {
b.ref_test(translator.closureInfo.nonNullableType);
} else {
final ranges = translator.classIdNumbering
.getConcreteClassIdRangeForCurrentModule(interfaceClass);
.getConcreteClassIdRangeForClass(interfaceClass);
b.local_get(operand);
b.struct_get(translator.topInfo.struct, FieldIndex.classId);
b.emitClassIdRangeCheck(ranges);
if (translator.isDynamicSubmodule) {
// Only types that are not dynamic module extendable can get here.
final classIdLocal = b.addLocal(w.NumType.i32);
b.local_tee(classIdLocal);
translator.callReference(translator.classIdToModuleId.reference, b);
b.i32_wrap_i64();
// Check if the class ID belongs to this module or the main module.
b.global_get(translator.dynamicModuleInfo!.moduleIdGlobal);
b.i32_wrap_i64();
b.i32_eq();
b.local_get(classIdLocal);
b.i32_const(translator.classIdNumbering.firstDynamicSubmoduleClassId);
b.i32_lt_u();
b.i32_or();
b.if_(const [], const [w.NumType.i32]);
// If it is in a known range, then localize the class ID to this
// module. If the class is from the main module this will do nothing.
b.local_get(classIdLocal);
translator.callReference(translator.localizeClassId.reference, b);
b.emitClassIdRangeCheck(ranges);
b.else_();
// If it's not in the main module or this submodule then the type is
// unknown to this module so the test fails.
b.i32_const(0);
b.end();
} else {
b.emitClassIdRangeCheck(ranges);
}
}
b.br(resultLabel);
}
@@ -922,7 +949,9 @@ class AsCheckerCallTarget extends CallTarget {
this.testedAgainstType,
this.operandIsNullable,
this.checkArguments,
this.argumentCount);
this.argumentCount)
: assert(!testedAgainstType.classNode
.isDynamicSubmoduleExtendable(translator.coreTypes));
@override
String get name {
@@ -0,0 +1,17 @@
# Copyright (c) 2025, 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.
callable:
- library: 'shared/shared.dart'
class: 'Base'
member: ''
- library: 'shared/shared.dart'
class: 'Tester'
extendable:
- library: 'shared/shared.dart'
class: 'Base'
can-be-overridden:
- library: 'shared/shared.dart'
class: 'Base'
member: 'foo'
@@ -0,0 +1,23 @@
// Copyright (c) 2025, 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.
import '../../common/testing.dart' as helper;
import 'package:expect/expect.dart';
import 'shared/shared.dart';
class MyChild extends Base {
@override
Tester foo(covariant Tester x) {
return Tester(x.val + 1);
}
}
/// A dynamic module is allowed to extend a class in the dynamic interface and
/// override its members.
void main() async {
final c = (await helper.load('entry1.dart')) as Base;
Expect.equals(3, c.foo(Tester(3)).val);
helper.done();
}
@@ -0,0 +1,15 @@
// Copyright (c) 2025, 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.
import '../shared/shared.dart';
class Child extends Base {
@override
Tester foo(covariant Tester x) {
return x;
}
}
@pragma('dyn-module:entry-point')
Object? dynamicModuleEntrypoint() => Child();
@@ -0,0 +1,12 @@
// Copyright (c) 2025, 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.
abstract class Base {
Tester foo(Tester t);
}
class Tester {
final int val;
Tester(this.val);
}