Skip to content

[native_toolchain_c] Use 16 kb pages for Android by default #1740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkgs/native_toolchain_c/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## 0.6.1-wip

- For Android, produce dylibs with page-size set to 16kb by default.
https://github.com/dart-lang/native/issues/1611

## 0.6.0

- Address analyzer info diagnostic about multi-line if requiring a block body.
Expand Down
3 changes: 3 additions & 0 deletions pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ class RunCBuilder {
cppLinkStdLib ?? defaultCppLinkStdLib[config.targetOS]!
],
...linkerOptions?.preSourcesFlags(toolInstance.tool, sourceFiles) ?? [],
// Support Android 15 page size by default, can be overridden by
// passing [flags].
if (config.targetOS == OS.android) '-Wl,-z,max-page-size=16384',
...flags,
for (final MapEntry(key: name, :value) in defines.entries)
if (value == null) '-D$name' else '-D$name=$value',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ void main() {
.firstWhere((e) => e.contains('file format'));
expect(machine, contains(objdumpFileFormat[target]));
}
if (linkMode == DynamicLoadingBundled()) {
await expectPageSize(libUri, 16 * 1024);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have evidence of users running into this issue?

Not all users will use package:native_toolchain_c, some may ship binaries, etc. So we could add build output validation for this that will apply to all code assets.

if (targetOS == OS.android && androidVersion >= ...) {
  ensurePageSizeIs16Kb(...);
}

somewhere in

Future<ValidationErrors> _validateCodeAssetBuildOrLinkOutput(

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have evidence of users running into this issue?

So we could add build output validation for this that will apply to all code assets.

We're not mandating readelf or objdump currently. Flutter provides access to a compiler, linker, and archiver via cCompiler. But we don't have something similar for commandline utilities at the moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

androidVersion

We don't have an android version. Only a minAndroidSdkVersion. But the minAndroidSdk doesn't matter, something compiled with a lower min SDK trying to run on Android 15 will still fail to load the dylib if the page-size is smaller than 16kb.

}
});
}
}
Expand Down Expand Up @@ -95,14 +98,37 @@ void main() {
// Identical API levels should lead to an identical binary.
expect(bytes2, bytes3);
});

test('page size override', () async {
const target = Architecture.arm64;
final linkMode = DynamicLoadingBundled();
const apiLevel1 = flutterAndroidNdkVersionLowestSupported;
final tempUri = await tempDirForTest();
final outUri = tempUri.resolve('out1/');
await Directory.fromUri(outUri).create();
const pageSize = 4 * 1024;
final libUri = await buildLib(
outUri,
target,
apiLevel1,
linkMode,
flags: ['-Wl,-z,max-page-size=$pageSize'],
);
if (Platform.isMacOS || Platform.isLinux) {
final address = await textSectionAddress(libUri);
expect(address, greaterThanOrEqualTo(pageSize));
expect(address, isNot(greaterThanOrEqualTo(pageSize * 4)));
}
});
}

Future<Uri> buildLib(
Uri tempUri,
Architecture targetArchitecture,
int androidNdkApi,
LinkMode linkMode,
) async {
LinkMode linkMode, {
List<String> flags = const [],
}) async {
final addCUri = packageUri.resolve('test/cbuilder/testfiles/add/src/add.c');
const name = 'add';

Expand Down Expand Up @@ -140,6 +166,7 @@ Future<Uri> buildLib(
name: name,
assetName: name,
sources: [addCUri.toFilePath()],
flags: flags,
);
await cbuilder.run(
config: buildConfig,
Expand Down
45 changes: 45 additions & 0 deletions pkgs/native_toolchain_c/test/helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,48 @@ Future<void> expectSymbols({
throw UnimplementedError();
}
}

Future<int> textSectionAddress(Uri dylib) async {
if (Platform.isMacOS) {
// Find the line in the objdump output that looks like:
// 11 .text 00000046 00000000000045a0 TEXT
final result = await runProcess(
executable: Uri.file('objdump'),
arguments: ['--headers', dylib.toFilePath()],
logger: logger,
);
expect(result.exitCode, 0);
final textSection =
result.stdout.split('\n').firstWhere((e) => e.contains('.text'));
final parsed = textSection.split(' ').where((e) => e.isNotEmpty).toList();
expect(parsed[1], '.text');
expect(parsed[4], 'TEXT');
final vma = int.parse(parsed[3], radix: 16);
return vma;
}
if (Platform.isLinux) {
// Find the line in the readelf output that looks like:
// [11] .text PROGBITS 00004328 000328 000064 00 AX 0 0 4
final result = await readelf(dylib.toFilePath(), 'S');
final textSection =
result.split('\n').firstWhere((e) => e.contains('.text'));
final parsed = textSection.split(' ').where((e) => e.isNotEmpty).toList();
expect(parsed[1], '.text');
expect(parsed[2], 'PROGBITS');
final addr = int.parse(parsed[3], radix: 16);
return addr;
}
throw UnimplementedError();
}

Future<void> expectPageSize(
Uri dylib,
int pageSize,
) async {
if (Platform.isMacOS || Platform.isLinux) {
// If page size is 16kb, the `.text` section address should be
// above 0x4000. With smaller page sizes it's above 0x1000.
final vma = await textSectionAddress(dylib);
expect(vma, greaterThanOrEqualTo(pageSize));
}
}