Skip to content

Commit ccbd88a

Browse files
authored
[native_toolchain_c] Fix treeshaking on mac (#2399)
By creating a list of symbols to be exported. Also needs to be tested with ICU4X, but manually setting this seems to do the trick to solve dart-lang/i18n#989 --- <details> <summary>Contribution guidelines:</summary><br> - See our [contributor guide](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md) for general expectations for PRs. - Larger or significant changes should be discussed in an issue before creating a PR. - Contributions to our repos should follow the [Dart style guide](https://dart.dev/guides/language/effective-dart) and use `dart format`. - Most changes should add an entry to the changelog and may need to [rev the pubspec package version](https://github.com/dart-lang/sdk/blob/main/docs/External-Package-Maintenance.md#making-a-change). - Changes to packages require [corresponding tests](https://github.com/dart-lang/.github/blob/main/CONTRIBUTING.md#Testing). Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback. </details>
1 parent b08519c commit ccbd88a

File tree

12 files changed

+229
-56
lines changed

12 files changed

+229
-56
lines changed

.github/workflows/native_toolchain_c.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ jobs:
4747
- name: Install native toolchains
4848
run: sudo apt-get update && sudo apt-get install gcc-i686-linux-gnu gcc-aarch64-linux-gnu gcc-arm-linux-gnueabihf gcc-riscv64-linux-gnu
4949

50+
- name: Install rust for pkgs/native_toolchain_c/test/clinker/rust_test.dart
51+
uses: actions-rust-lang/setup-rust-toolchain@fb51252c7ba57d633bc668f941da052e410add48
52+
5053
- run: git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git
5154
- run: echo "$PWD/depot_tools" >> $GITHUB_PATH
5255
- run: mkdir dart-sdk

.github/workflows/publish.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,4 @@ jobs:
2525
with:
2626
write-comments: false
2727
sdk: dev # use beta/stable after 3.3.0
28+
ignore-packages: pkgs/objective_c,pkgs/ffigen,pkgs/jni,pkgs/jnigen

pkgs/hooks_runner/test_data/treeshaking_native_libs/hook/link.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ void main(List<String> arguments) async {
1212
final linker = CLinker.library(
1313
name: input.packageName,
1414
assetName: input.assets.code.single.id.split('/').skip(1).join('/'),
15-
linkerOptions: LinkerOptions.treeshake(symbols: ['add']),
15+
linkerOptions: LinkerOptions.treeshake(symbolsToKeep: ['add']),
1616
sources: [input.assets.code.single.file!.toFilePath()],
1717
);
1818
await linker.run(

pkgs/native_toolchain_c/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.17.0
2+
3+
* Fix treeshaking on mac.
4+
15
## 0.16.8
26

37
* Support building assets for packages which are not the input package.

pkgs/native_toolchain_c/lib/src/cbuilder/linker_options.dart

Lines changed: 68 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -25,45 +25,48 @@ class LinkerOptions {
2525
/// See also the `ld` man page at https://linux.die.net/man/1/ld.
2626
final bool gcSections;
2727

28-
/// The linker script to be passed via `--version-script`.
29-
///
30-
/// See also the `ld` man page at https://linux.die.net/man/1/ld.
31-
final Uri? linkerScript;
28+
final LinkerScriptMode? _linkerScriptMode;
3229

3330
/// Whether to strip debugging symbols from the binary.
3431
final bool stripDebug;
3532

3633
/// The symbols to keep in the resulting binaries.
37-
///
38-
/// If null all symbols will be kept.
39-
final List<String>? _symbolsToKeep;
34+
final List<String> _symbols;
4035

41-
final bool _generateLinkerScript;
36+
final bool _keepAllSymbols;
4237

4338
/// Create linking options manually for fine-grained control.
39+
///
40+
/// If [symbolsToKeep] is null, all symbols will be kept.
4441
LinkerOptions.manual({
4542
List<String>? flags,
4643
bool? gcSections,
47-
this.linkerScript,
44+
Uri? linkerScript,
4845
this.stripDebug = true,
4946
Iterable<String>? symbolsToKeep,
5047
}) : _linkerFlags = flags ?? [],
5148
gcSections = gcSections ?? true,
52-
_symbolsToKeep = symbolsToKeep?.toList(growable: false),
53-
_generateLinkerScript = false;
49+
_symbols = symbolsToKeep?.toList(growable: false) ?? const [],
50+
_keepAllSymbols = symbolsToKeep == null,
51+
_linkerScriptMode = linkerScript != null
52+
? ManualLinkerScript(script: linkerScript)
53+
: null;
5454

5555
/// Create linking options to tree-shake symbols from the input files.
5656
///
57-
/// The [symbols] specify the symbols which should be kept.
57+
/// The [symbolsToKeep] specify the symbols which should be kept. Passing
58+
/// `null` implies that all symbols should be kept.
5859
LinkerOptions.treeshake({
5960
Iterable<String>? flags,
60-
required Iterable<String>? symbols,
61+
required Iterable<String>? symbolsToKeep,
6162
this.stripDebug = true,
6263
}) : _linkerFlags = flags?.toList(growable: false) ?? [],
63-
_symbolsToKeep = symbols?.toList(growable: false),
64+
_symbols = symbolsToKeep?.toList(growable: false) ?? const [],
65+
_keepAllSymbols = symbolsToKeep == null,
6466
gcSections = true,
65-
linkerScript = null,
66-
_generateLinkerScript = symbols != null;
67+
_linkerScriptMode = symbolsToKeep != null
68+
? GenerateLinkerScript()
69+
: null;
6770

6871
Iterable<String> _toLinkerSyntax(Tool linker, Iterable<String> flagList) {
6972
if (linker.isClangLike) {
@@ -76,6 +79,19 @@ class LinkerOptions {
7679
}
7780
}
7881

82+
sealed class LinkerScriptMode {}
83+
84+
final class GenerateLinkerScript extends LinkerScriptMode {}
85+
86+
final class ManualLinkerScript extends LinkerScriptMode {
87+
/// The linker script to be passed via `--version-script`.
88+
///
89+
/// See also the `ld` man page at https://linux.die.net/man/1/ld.
90+
final Uri script;
91+
92+
ManualLinkerScript({required this.script});
93+
}
94+
7995
extension LinkerOptionsExt on LinkerOptions {
8096
/// Takes [sourceFiles] and turns it into flags for the compiler driver while
8197
/// considering the current [LinkerOptions].
@@ -99,8 +115,6 @@ extension LinkerOptionsExt on LinkerOptions {
99115
}
100116
}
101117

102-
bool get _includeAllSymbols => _symbolsToKeep == null;
103-
104118
Iterable<String> _sourceFilesToFlagsForClangLike(
105119
Tool tool,
106120
Iterable<String> sourceFiles,
@@ -109,33 +123,37 @@ extension LinkerOptionsExt on LinkerOptions {
109123
switch (targetOS) {
110124
case OS.macOS || OS.iOS:
111125
return [
112-
if (!_includeAllSymbols) ...sourceFiles,
126+
if (!_keepAllSymbols) ...sourceFiles,
113127
..._toLinkerSyntax(tool, [
114-
if (_includeAllSymbols) ...sourceFiles.map((e) => '-force_load,$e'),
128+
if (_keepAllSymbols) ...sourceFiles.map((e) => '-force_load,$e'),
115129
..._linkerFlags,
116-
..._symbolsToKeep?.map((symbol) => '-u,_$symbol') ?? [],
130+
..._symbols.map((symbol) => '-u,_$symbol'),
117131
if (stripDebug) '-S',
118132
if (gcSections) '-dead_strip',
133+
if (_linkerScriptMode is ManualLinkerScript)
134+
'-exported_symbols_list,${_linkerScriptMode.script.toFilePath()}'
135+
else if (_linkerScriptMode is GenerateLinkerScript)
136+
'-exported_symbols_list,${_createMacSymbolList(_symbols)}',
119137
]),
120138
];
121139

122140
case OS.android || OS.linux:
123141
final wholeArchiveSandwich =
124142
sourceFiles.any((source) => source.endsWith('.a')) ||
125-
_includeAllSymbols;
143+
_keepAllSymbols;
126144
return [
127145
if (wholeArchiveSandwich)
128146
..._toLinkerSyntax(tool, ['--whole-archive']),
129147
...sourceFiles,
130148
..._toLinkerSyntax(tool, [
131149
..._linkerFlags,
132-
..._symbolsToKeep?.map((symbol) => '-u,$symbol') ?? [],
150+
..._symbols.map((symbol) => '-u,$symbol'),
133151
if (stripDebug) '--strip-debug',
134152
if (gcSections) '--gc-sections',
135-
if (linkerScript != null)
136-
'--version-script=${linkerScript!.toFilePath()}'
137-
else if (_generateLinkerScript && _symbolsToKeep != null)
138-
'--version-script=${_createClangLikeLinkScript(_symbolsToKeep)}',
153+
if (_linkerScriptMode is ManualLinkerScript)
154+
'--version-script=${_linkerScriptMode.script.toFilePath()}'
155+
else if (_linkerScriptMode is GenerateLinkerScript)
156+
'--version-script=${_createClangLikeLinkScript(_symbols)}',
139157
if (wholeArchiveSandwich) '--no-whole-archive',
140158
]),
141159
];
@@ -152,21 +170,34 @@ extension LinkerOptionsExt on LinkerOptions {
152170
) => [
153171
...sourceFiles,
154172
'/link',
155-
if (_includeAllSymbols) ...sourceFiles.map((e) => '/WHOLEARCHIVE:$e'),
173+
if (_keepAllSymbols) ...sourceFiles.map((e) => '/WHOLEARCHIVE:$e'),
156174
..._linkerFlags,
157-
..._symbolsToKeep?.map(
158-
(symbol) =>
159-
'/INCLUDE:${targetArch == Architecture.ia32 ? '_' : ''}$symbol',
160-
) ??
161-
[],
162-
if (linkerScript != null)
163-
'/DEF:${linkerScript!.toFilePath()}'
164-
else if (_generateLinkerScript && _symbolsToKeep != null)
165-
'/DEF:${_createClLinkScript(_symbolsToKeep)}',
175+
..._symbols.map(
176+
(symbol) =>
177+
'/INCLUDE:${targetArch == Architecture.ia32 ? '_' : ''}$symbol',
178+
),
179+
if (_linkerScriptMode is ManualLinkerScript)
180+
'/DEF:${_linkerScriptMode.script.toFilePath()}'
181+
else if (_linkerScriptMode is GenerateLinkerScript)
182+
'/DEF:${_createClLinkScript(_symbols)}',
166183
if (stripDebug) '/PDBSTRIPPED',
167184
if (gcSections) '/OPT:REF',
168185
];
169186

187+
/// This creates a list of exported symbols.
188+
///
189+
/// If this is not set, some symbols might be kept. This can be inspected
190+
/// using `ld -why_live`, see https://www.unix.com/man_page/osx/1/ld/, where
191+
/// the reason will show up as `global-dont-strip`.
192+
/// This might possibly be a Rust only feature.
193+
static String _createMacSymbolList(Iterable<String> symbols) {
194+
final tempDir = Directory.systemTemp.createTempSync();
195+
final symbolsFileUri = tempDir.uri.resolve('exported_symbols_list.txt');
196+
final symbolsFile = File.fromUri(symbolsFileUri)..createSync();
197+
symbolsFile.writeAsStringSync(symbols.map((e) => '_$e').join('\n'));
198+
return symbolsFileUri.toFilePath();
199+
}
200+
170201
static String _createClangLikeLinkScript(Iterable<String> symbols) {
171202
final tempDir = Directory.systemTemp.createTempSync();
172203
final symbolsFileUri = tempDir.uri.resolve('symbols.lds');

pkgs/native_toolchain_c/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: native_toolchain_c
22
description: >-
33
A library to invoke the native C compiler installed on the host machine.
4-
version: 0.16.8
4+
version: 0.17.0
55
repository: https://github.com/dart-lang/native/tree/main/pkgs/native_toolchain_c
66

77
topics:
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
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 'dart:io';
6+
7+
import 'package:code_assets/code_assets.dart';
8+
import 'package:hooks/hooks.dart';
9+
import 'package:native_toolchain_c/native_toolchain_c.dart';
10+
import 'package:test/test.dart';
11+
12+
import '../helpers.dart';
13+
14+
Future<void> main() async {
15+
late final LinkInput linkInput;
16+
late final Uri staticLib;
17+
final linkOutputBuilder = LinkOutputBuilder();
18+
final targetArchitecture = Architecture.current;
19+
final targetOS = OS.current;
20+
late final bool rustToolchainInstalled;
21+
setUpAll(() async {
22+
final tempUri = await tempDirForTest();
23+
final tempUri2 = await tempDirForTest();
24+
staticLib = tempUri.resolve(targetOS.staticlibFileName('libtest'));
25+
final processResult = await Process.run('rustc', [
26+
'--crate-type=staticlib',
27+
'test/clinker/testfiles/linker/test.rs',
28+
'-o',
29+
staticLib.toFilePath(),
30+
]);
31+
rustToolchainInstalled = processResult.exitCode == 0;
32+
if (rustToolchainInstalled) {
33+
await File.fromUri(
34+
staticLib,
35+
).copy(tempUri.resolve('libtest.a').toFilePath());
36+
}
37+
final linkInputBuilder = LinkInputBuilder()
38+
..setupShared(
39+
packageName: 'testpackage',
40+
packageRoot: tempUri,
41+
outputFile: tempUri.resolve('output.json'),
42+
outputDirectoryShared: tempUri2,
43+
)
44+
..setupLink(assets: [], recordedUsesFile: null)
45+
..addExtension(
46+
CodeAssetExtension(
47+
targetOS: targetOS,
48+
targetArchitecture: targetArchitecture,
49+
linkModePreference: LinkModePreference.dynamic,
50+
),
51+
);
52+
53+
linkInput = linkInputBuilder.build();
54+
});
55+
test('link rust binary with script treeshakes', () async {
56+
if (!rustToolchainInstalled) {
57+
return;
58+
}
59+
final treeshakeOption = LinkerOptions.treeshake(
60+
symbolsToKeep: ['my_other_func'],
61+
);
62+
final symbols = await _link(
63+
staticLib,
64+
treeshakeOption,
65+
linkInput,
66+
linkOutputBuilder,
67+
targetArchitecture,
68+
targetOS,
69+
);
70+
final skipReason = symbols == null
71+
? 'tool to extract symbols unavailable'
72+
: false;
73+
expect(symbols, contains('my_other_func'), skip: skipReason);
74+
expect(symbols, isNot(contains('my_func')), skip: skipReason);
75+
});
76+
77+
test('link rust binary without script keeps symbols', () async {
78+
if (!rustToolchainInstalled) {
79+
return;
80+
}
81+
final manualOption = LinkerOptions.manual(
82+
symbolsToKeep: ['my_other_func'],
83+
stripDebug: true,
84+
gcSections: true,
85+
);
86+
final symbols = await _link(
87+
staticLib,
88+
manualOption,
89+
linkInput,
90+
linkOutputBuilder,
91+
targetArchitecture,
92+
targetOS,
93+
);
94+
final skipReason = symbols == null
95+
? 'tool to extract symbols unavailable'
96+
: false;
97+
expect(symbols, contains('my_other_func'), skip: skipReason);
98+
expect(symbols, contains('my_func'), skip: skipReason);
99+
});
100+
}
101+
102+
Future<String?> _link(
103+
Uri staticLib,
104+
LinkerOptions manualOption,
105+
LinkInput linkInput,
106+
LinkOutputBuilder linkOutputBuilder,
107+
Architecture targetArchitecture,
108+
OS targetOS,
109+
) async {
110+
await CLinker.library(
111+
name: 'mylibname',
112+
assetName: '',
113+
sources: [staticLib.toFilePath()],
114+
linkerOptions: manualOption,
115+
).run(input: linkInput, output: linkOutputBuilder, logger: logger);
116+
117+
final linkOutput = linkOutputBuilder.build();
118+
final asset = linkOutput.assets.code.first;
119+
120+
await expectMachineArchitecture(asset.file!, targetArchitecture, targetOS);
121+
122+
return await readSymbols(asset, targetOS);
123+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
_my_other_func
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#[no_mangle]
2+
pub extern "C" fn my_func() -> u64 {
3+
return 41;
4+
}
5+
6+
#[no_mangle]
7+
pub extern "C" fn my_other_func() -> u64 {
8+
return 42;
9+
}

0 commit comments

Comments
 (0)