Skip to content

Commit c942b23

Browse files
mkustermannCommit Queue
authored andcommitted
[dart2wasm] Fix access of constants under deferred load guard
When accessing a constant under a load guard we can push the constant to the deferred module (as we only access it under the guard that the module has been loaded). The access has to therefore also use the constant initializer function from the deferred module. We also remove `--extra-compiler-option` prefix for `--enable-deferred-loading` as the prefix isn't recognized by dartdev. Change-Id: I58489303dc10bb265ef730c05a4fc78a8f598edb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464980 Reviewed-by: Ömer Ağacan <[email protected]> Commit-Queue: Martin Kustermann <[email protected]>
1 parent cb718f8 commit c942b23

8 files changed

+77
-10
lines changed

pkg/dart2wasm/lib/constants.dart

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,7 +1557,7 @@ class _ConstantAccessor {
15571557

15581558
if (existingDefinition != null) {
15591559
// We already have a defined constant. Possibly import it and then use it.
1560-
return _readDefinedConstant(b, info, existingDefinition);
1560+
return _readDefinedConstant(b, b.moduleBuilder, info, existingDefinition);
15611561
}
15621562

15631563
// We have to guarantee that using the constant synchronously works. If the
@@ -1573,7 +1573,7 @@ class _ConstantAccessor {
15731573
usingModule == translator.mainModule) {
15741574
final definition =
15751575
_defineConstantInModuleRecursive(translator.mainModule, info);
1576-
return _readDefinedConstant(b, info, definition);
1576+
return _readDefinedConstant(b, usingModule, info, definition);
15771577
}
15781578

15791579
// Remember for the transitive DAG of [constant] that we use it in this
@@ -1602,18 +1602,30 @@ class _ConstantAccessor {
16021602
if (patchInstructions != null) {
16031603
translator.linkingActions.add(() {
16041604
// All constant uses have been discovered during codegen phase so we can
1605-
// know decide into which module to place the constant and patch the
1605+
// now decide into which module to place the constant and patch the
16061606
// constant access to load it from there.
16071607
final definition =
16081608
info._definition ?? _defineConstantInModuleRecursive(null, info);
1609-
_readDefinedConstant(patchInstructions, info, definition);
1609+
_readDefinedConstant(patchInstructions, usingModule, info, definition);
16101610
});
16111611
}
16121612

16131613
return info.type;
16141614
}
16151615

1616-
w.ValueType _readDefinedConstant(w.InstructionsBuilder b, ConstantInfo info,
1616+
/// Reads the given constant.
1617+
///
1618+
/// Normally `b.moduleBuilder == usingModule`, except for the situation where
1619+
/// the read happens under a load guard.
1620+
///
1621+
/// In that case the `b.moduleBuilder` may not have an initializer function
1622+
/// (to reduce its size) and instead it can rely on the load guarded deferred
1623+
/// module to be loaded by the time we read the constant, so it can use that
1624+
/// deferred module's constant initializer.
1625+
w.ValueType _readDefinedConstant(
1626+
w.InstructionsBuilder b,
1627+
w.ModuleBuilder usingModule,
1628+
ConstantInfo info,
16171629
ConstantDefinition definition) {
16181630
// Eagerly initialized constant.
16191631
if (definition is GlobalBasedConstantDefinition && !definition.isLazy) {
@@ -1630,7 +1642,7 @@ class _ConstantAccessor {
16301642
w.Label done = b.block(const [], [info.type]);
16311643
translator.globals.readGlobal(b, definition.global);
16321644
b.br_on_non_null(done);
1633-
translator.callFunction(definition.initializer(b.moduleBuilder), b);
1645+
translator.callFunction(definition.initializer(usingModule), b);
16341646
b.end();
16351647
break;
16361648
case TableBasedConstantDefinition():
@@ -1639,7 +1651,7 @@ class _ConstantAccessor {
16391651
b.i32_const(definition.tableIndex);
16401652
b.table_get(tableImporter.get(definition.table, b.moduleBuilder));
16411653
b.br_on_non_null(done);
1642-
translator.callFunction(definition.initializer(b.moduleBuilder), b);
1654+
translator.callFunction(definition.initializer(usingModule), b);
16431655
b.end();
16441656
break;
16451657
}

pkg/dart2wasm/tool/compile_benchmark

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ while [ $# -gt 0 ]; do
146146
shift
147147
;;
148148

149-
--extra-compiler-option=--enable-deferred-loading)
149+
--enable-deferred-loading | --extra-compiler-option=--enable-deferred-loading)
150150
MULTI_MODULE=1
151151
DART2WASM_ARGS+=("--enable-deferred-loading")
152152
shift

tests/language/deferred/regression_22995_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
// Test that closurizing a function implies a dependency on its type.
66

7-
// dart2wasmOptions=--extra-compiler-option=--enable-deferred-loading -O0
7+
// dart2wasmOptions=--enable-deferred-loading -O0
88

99
import "package:expect/expect.dart";
1010

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'deferred_constant_use_under_load_guard_def1.dart' deferred as D1;
6+
import 'deferred_constant_use_under_load_guard_def2.dart' deferred as D2;
7+
8+
Future runTest() async {
9+
await D1.loadLibrary();
10+
await D2.loadLibrary();
11+
12+
// Access of `foo` under load guard D1.
13+
D1.printValue(foo);
14+
15+
// Access of `foo` under load guard D2.
16+
D2.printValue(foo);
17+
18+
// To prevent signature shaking from removing parameter and pushing `foo` to
19+
// callee.
20+
D1.printValue('a');
21+
D2.printValue('b');
22+
}
23+
24+
class FooConst {
25+
final String value;
26+
const FooConst(this.value);
27+
}
28+
29+
const foo = FooConst('foo');
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
void printValue(value) {
6+
print(value);
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
void printValue(value) {
6+
print(value);
7+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
//
5+
// dart2wasmOptions=--enable-deferred-loading
6+
7+
import 'deferred_constant_use_under_load_guard_def.dart' deferred as D;
8+
9+
main() async {
10+
await D.loadLibrary();
11+
await D.runTest();
12+
}

tests/web/wasm/source_map_simple_optimized_deferred_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
// dart2wasmOptions=-O4 --no-strip-wasm --extra-compiler-option=--enable-deferred-loading --extra-compiler-option=-DTEST_COMPILATION_DIR=$TEST_COMPILATION_DIR
5+
// dart2wasmOptions=-O4 --no-strip-wasm --enable-deferred-loading --extra-compiler-option=-DTEST_COMPILATION_DIR=$TEST_COMPILATION_DIR
66

77
import 'source_map_simple_lib.dart' as Lib;
88

0 commit comments

Comments
 (0)