From 28654b90e8ef4022bc80496c7198e96b61760fc7 Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Thu, 21 Apr 2022 11:06:39 +0000 Subject: [PATCH] [vm] Avoid local labels in assembly snapshots. Labels starting with L are treated as local by the assembler and cause problems down the line. When targeting ARM64 Mach-O having local labels cause linker to break when trying to generate compact unwinding information with a cryptic error ld: too many compact unwind infos in function <...> This happens because local labels are not seen as function boundaries and multiple .cfi_startproc/.cfi_endproc are mashed into a single function. Fixes https://github.com/flutter/flutter/issues/102281 TEST=runtime/tests/vm/dart{,_2}/no_local_labels_test.dart Change-Id: I0171dc08f49c71ccb1ca02b398e01ac241efd9a8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241962 Reviewed-by: Daco Harkes Commit-Queue: Slava Egorov --- .../tests/vm/dart/no_local_labels_test.dart | 78 ++++++++++++++++++ .../tests/vm/dart_2/no_local_labels_test.dart | 80 +++++++++++++++++++ runtime/vm/image_snapshot.cc | 10 +++ 3 files changed, 168 insertions(+) create mode 100644 runtime/tests/vm/dart/no_local_labels_test.dart create mode 100644 runtime/tests/vm/dart_2/no_local_labels_test.dart diff --git a/runtime/tests/vm/dart/no_local_labels_test.dart b/runtime/tests/vm/dart/no_local_labels_test.dart new file mode 100644 index 000000000000..25fe76f8da57 --- /dev/null +++ b/runtime/tests/vm/dart/no_local_labels_test.dart @@ -0,0 +1,78 @@ +// Copyright (c) 2022, 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. + +// This test verifies that assembly snapshot does not contain local labels. +// (labels starting with 'L'). + +import 'dart:io'; + +import 'package:expect/expect.dart'; +import 'package:path/path.dart' as path; + +import 'use_flag_test_helper.dart'; + +main(List args) async { + if (!isAOTRuntime) { + return; // Running in JIT: AOT binaries not available. + } + + if (Platform.isAndroid) { + return; // SDK tree not available on the test device. + } + + // These are the tools we need to be available to run on a given platform: + if (!File(platformDill).existsSync()) { + throw "Cannot run test as $platformDill does not exist"; + } + if (!await testExecutable(genSnapshot)) { + throw "Cannot run test as $genSnapshot not available"; + } + + await withTempDir('no-local-labels-test', (String tempDir) async { + final script = path.join(tempDir, 'program.dart'); + final scriptDill = path.join(tempDir, 'program.dill'); + + await File(script).writeAsString(''' +class Local { + @pragma('vm:never-inline') + void foo() { + } + + @pragma('vm:never-inline') + void bar() { + } +} + +void main(List args) { + Local()..foo()..bar(); +} +'''); + + // Compile script to Kernel IR. + await run(genKernel, [ + '--aot', + '--platform=$platformDill', + '-o', + scriptDill, + script, + ]); + + if (Platform.isWindows) { + return; // No assembly generation on Windows. + } + + final assembly = path.join(tempDir, 'program.S'); + await run(genSnapshot, [ + '--snapshot-kind=app-aot-assembly', + '--assembly=$assembly', + scriptDill, + ]); + + final localLabelRe = RegExp(r'^L[a-zA-Z0-9_\.$]*:$', multiLine: true); + final match = localLabelRe.firstMatch(await File(assembly).readAsString()); + if (match != null) { + Expect.isTrue(false, 'unexpected local label found ${match[0]!}'); + } + }); +} diff --git a/runtime/tests/vm/dart_2/no_local_labels_test.dart b/runtime/tests/vm/dart_2/no_local_labels_test.dart new file mode 100644 index 000000000000..7c9c217fdbaf --- /dev/null +++ b/runtime/tests/vm/dart_2/no_local_labels_test.dart @@ -0,0 +1,80 @@ +// Copyright (c) 2022, 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. + +// This test verifies that assembly snapshot does not contain local labels. +// (labels starting with 'L'). + +// @dart=2.9 + +import 'dart:io'; + +import 'package:expect/expect.dart'; +import 'package:path/path.dart' as path; + +import 'use_flag_test_helper.dart'; + +main(List args) async { + if (!isAOTRuntime) { + return; // Running in JIT: AOT binaries not available. + } + + if (Platform.isAndroid) { + return; // SDK tree not available on the test device. + } + + // These are the tools we need to be available to run on a given platform: + if (!File(platformDill).existsSync()) { + throw "Cannot run test as $platformDill does not exist"; + } + if (!await testExecutable(genSnapshot)) { + throw "Cannot run test as $genSnapshot not available"; + } + + await withTempDir('no-local-labels-test', (String tempDir) async { + final script = path.join(tempDir, 'program.dart'); + final scriptDill = path.join(tempDir, 'program.dill'); + + await File(script).writeAsString(''' +class Local { + @pragma('vm:never-inline') + void foo() { + } + + @pragma('vm:never-inline') + void bar() { + } +} + +void main(List args) { + Local()..foo()..bar(); +} +'''); + + // Compile script to Kernel IR. + await run(genKernel, [ + '--aot', + '--platform=$platformDill', + '-o', + scriptDill, + script, + ]); + + if (Platform.isWindows) { + return; // No assembly generation on Windows. + } + + final assembly = path.join(tempDir, 'program.S'); + await run(genSnapshot, [ + '--snapshot-kind=app-aot-assembly', + '--assembly=$assembly', + scriptDill, + ]); + + final localLabelRe = RegExp(r'^L[a-zA-Z0-9_\.$]*:$', multiLine: true); + final match = localLabelRe.firstMatch(await File(assembly).readAsString()); + if (match != null) { + Expect.isTrue(false, 'unexpected local label found ${match[0]}'); + } + }); +} diff --git a/runtime/vm/image_snapshot.cc b/runtime/vm/image_snapshot.cc index b4e85145752f..811593104d9e 100644 --- a/runtime/vm/image_snapshot.cc +++ b/runtime/vm/image_snapshot.cc @@ -1108,6 +1108,16 @@ void AssemblyImageWriter::Finalize() { static void AddAssemblerIdentifier(ZoneTextBuffer* printer, const char* label) { ASSERT(label[0] != '.'); + if (label[0] == 'L' && printer->length() == 0) { + // Assembler treats labels starting with `L` as local which can cause + // some issues down the line e.g. on Mac the linker might fail to encode + // compact unwind information because multiple functions end up being + // treated as a single function. See https://github.com/flutter/flutter/issues/102281. + // + // Avoid this by prepending an underscore. + printer->AddString("_"); + } + for (char c = *label; c != '\0'; c = *++label) { #define OP(dart_name, asm_name) \ if (strncmp(label, dart_name, strlen(dart_name)) == 0) { \