From 4ab313cf090ea9edbdbe2b092915b1aa912ef6f3 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Tue, 1 Apr 2025 09:50:12 -0700 Subject: [PATCH] [vm, test] Enable some AOT testing for Fuchsia. Fix the standalone embedder's ELF loader to open files with the executable permission. TEST=ci Bug: b/399714829 Bug: https://github.com/dart-lang/sdk/issues/60442 Cq-Include-Trybots: luci.dart.try:vm-fuchsia-release-arm64-try,vm-fuchsia-release-x64-try Change-Id: I2652a5849462ce63585550e6c581cc578a228955 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419341 Reviewed-by: Alexander Aprelev Commit-Queue: Ryan Macnak Reviewed-by: Zijie He --- BUILD.gn | 19 ++++++-- .../lib/src/compiler_configuration.dart | 5 ++ pkg/test_runner/lib/src/fuchsia.dart | 24 ++++++++-- .../lib/src/runtime_configuration.dart | 20 ++++++-- .../lib/src/test_configurations.dart | 2 +- runtime/bin/BUILD.gn | 1 + runtime/bin/elf_loader.cc | 5 +- runtime/bin/elf_loader.h | 2 - runtime/bin/file.h | 10 +++- runtime/bin/file_fuchsia.cc | 47 +++++++++++++------ runtime/bin/file_linux.cc | 12 +++-- runtime/bin/file_macos.cc | 12 +++-- runtime/bin/file_win.cc | 12 +++-- runtime/bin/snapshot_utils.cc | 4 -- tools/bots/test_matrix.json | 18 ++++++- 15 files changed, 145 insertions(+), 48 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index a5572b60a6c..fb3ca5fc537 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -225,12 +225,13 @@ group("compressed_observatory_archive") { if (is_fuchsia) { import("third_party/fuchsia/gn-sdk/src/component.gni") import("third_party/fuchsia/gn-sdk/src/package.gni") + import("utils/aot_snapshot.gni") import("utils/application_snapshot.gni") + # TODO(b/399714829): Test packages should be created at test-time, not build-time. + # tests/ffi/**_test.dart except those with compile-time errors test_sources = [ - "tests/ffi/abi_specific_int_incomplete_aot_test.dart", - "tests/ffi/abi_specific_int_incomplete_jit_test.dart", "tests/ffi/abi_specific_int_test.dart", "tests/ffi/abi_test.dart", "tests/ffi/address_of_array_generated_test.dart", @@ -393,6 +394,7 @@ if (is_fuchsia) { test_resources = [] foreach(test_source, test_sources) { label = string_replace(test_source, "/", "_") + aot_label = label + "_aot" application_snapshot(label) { dart_snapshot_kind = "kernel" @@ -400,13 +402,24 @@ if (is_fuchsia) { training_args = [] # Not used output = "$target_gen_dir/$test_source.dill" } + aot_snapshot(aot_label) { + main_dart = test_source + output = "$target_gen_dir/$test_source.elf" + } - test_deps += [ ":$label" ] + test_deps += [ + ":$aot_label", + ":$label", + ] test_resources += [ { path = rebase_path("$target_gen_dir/$test_source.dill") dest = "data/$test_source" }, + { + path = rebase_path("$target_gen_dir/$test_source.elf") + dest = "data/$test_source.elf" + }, ] } diff --git a/pkg/test_runner/lib/src/compiler_configuration.dart b/pkg/test_runner/lib/src/compiler_configuration.dart index af3554885e1..308b4c60d04 100644 --- a/pkg/test_runner/lib/src/compiler_configuration.dart +++ b/pkg/test_runner/lib/src/compiler_configuration.dart @@ -105,6 +105,11 @@ abstract class CompilerConfiguration { return NoneCompilerConfiguration(configuration); case Compiler.dartkp: + // TODO(b/399714829): Test packages should be created at test-time, not + // build time. + if (configuration.system == System.fuchsia) { + return NoneCompilerConfiguration(configuration); + } return PrecompilerCompilerConfiguration(configuration); case Compiler.specParser: diff --git a/pkg/test_runner/lib/src/fuchsia.dart b/pkg/test_runner/lib/src/fuchsia.dart index 8a4800bfc35..e7e003dc8af 100644 --- a/pkg/test_runner/lib/src/fuchsia.dart +++ b/pkg/test_runner/lib/src/fuchsia.dart @@ -49,17 +49,22 @@ class FuchsiaEmulator { // Returns a command to execute a set of tests against the running Fuchsia // environment. - VMCommand getTestCommand(String buildDir, String mode, String arch, - List arguments, Map environmentOverrides) { + VMCommand getTestCommand( + String buildDir, + String mode, + String arch, + String component, + List arguments, + Map environmentOverrides) { environmentOverrides.addAll(envs); return VMCommand( withEnv, [ "./third_party/fuchsia/test_scripts/test/run_executable_test.py", - "--test-name=fuchsia-pkg://fuchsia.com/dart_test_$mode#meta/dart_test_component.cm", + "--test-name=fuchsia-pkg://fuchsia.com/dart_test_$mode#meta/$component", // VmexResource not available in default hermetic realm // TODO(38752): Setup a Dart test realm. - "--test-realm=/core/testing:system-tests", + "--test-realm=/core/testing/system-tests", "--out-dir=${_outDir(buildDir, mode)}", "--package-deps=dart_test_$mode.far", ...arguments @@ -68,7 +73,16 @@ class FuchsiaEmulator { } // Tears down the Fuchsia environment. - Future stop() async { + Future stop(bool verbose) async { + if (verbose) { + // Get the logs before they're deleted. + var logDump = await Process.start(withEnv, + ["third_party/fuchsia/sdk/linux/tools/x64/ffx", "log", "dump"], + environment: {"FFX_ISOLATE_DIR": daemonIsolateDir!.path}, + mode: ProcessStartMode.inheritStdio); + await logDump.exitCode; + } + publisher!.kill(); await publisher!.exitCode; publisher = null; diff --git a/pkg/test_runner/lib/src/runtime_configuration.dart b/pkg/test_runner/lib/src/runtime_configuration.dart index 2950352909f..9603a89dea9 100644 --- a/pkg/test_runner/lib/src/runtime_configuration.dart +++ b/pkg/test_runner/lib/src/runtime_configuration.dart @@ -44,7 +44,7 @@ abstract class RuntimeConfiguration { if (configuration.system == System.android) { return DartkAdbRuntimeConfiguration(); } else if (configuration.system == System.fuchsia) { - return DartkFuchsiaEmulatorRuntimeConfiguration(); + return DartkFuchsiaEmulatorRuntimeConfiguration(false); } return StandaloneDartRuntimeConfiguration(); @@ -53,11 +53,12 @@ abstract class RuntimeConfiguration { return DartPrecompiledAdbRuntimeConfiguration( configuration.useElf, ); - } else { - return DartPrecompiledRuntimeConfiguration( + } else if (configuration.system == System.fuchsia) { + return DartkFuchsiaEmulatorRuntimeConfiguration(true); + } + return DartPrecompiledRuntimeConfiguration( configuration.useElf, ); - } } throw "unreachable"; } @@ -513,6 +514,9 @@ class DartPrecompiledAdbRuntimeConfiguration class DartkFuchsiaEmulatorRuntimeConfiguration extends DartVmRuntimeConfiguration { + final bool aot; + DartkFuchsiaEmulatorRuntimeConfiguration(this.aot); + @override List computeRuntimeCommands( CommandArtifact? artifact, @@ -539,12 +543,20 @@ class DartkFuchsiaEmulatorRuntimeConfiguration argument.replaceAll(Directory.current.path, "pkg/data")) .toList(); + var component = "dart_test_component.cm"; + if (aot) { + component = "dartaotruntime_test_component.cm"; + arguments[arguments.length - 1] = + arguments[arguments.length - 1].replaceAll(".dart", ".dart.elf"); + } + arguments.insert(arguments.length - 1, '--disable-dart-dev'); return [ FuchsiaEmulator.instance().getTestCommand( _configuration.buildDirectory, _configuration.mode.name, _configuration.architecture.name, + component, arguments, environmentOverrides) ]; diff --git a/pkg/test_runner/lib/src/test_configurations.dart b/pkg/test_runner/lib/src/test_configurations.dart index 0bb4cd6e0ed..c908a573d5f 100644 --- a/pkg/test_runner/lib/src/test_configurations.dart +++ b/pkg/test_runner/lib/src/test_configurations.dart @@ -190,7 +190,7 @@ Future testConfigurations(List configurations) async { if (configurations.any((configuration) { return configuration.system == System.fuchsia; })) { - FuchsiaEmulator.instance().stop(); + FuchsiaEmulator.instance().stop(firstConf.isVerbose); } DebugLogger.close(); diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn index 3c181b2daaf..57d7006036d 100644 --- a/runtime/bin/BUILD.gn +++ b/runtime/bin/BUILD.gn @@ -59,6 +59,7 @@ template("build_libdart_builtin") { public_configs = [ ":libdart_builtin_config" ] deps = [] if (is_fuchsia) { + deps += [ "$fuchsia_sdk/fidl/fuchsia.io" ] public_deps = [ "$fuchsia_sdk/pkg/fdio" ] } if (is_win) { diff --git a/runtime/bin/elf_loader.cc b/runtime/bin/elf_loader.cc index 6c8b6562479..9028becc47e 100644 --- a/runtime/bin/elf_loader.cc +++ b/runtime/bin/elf_loader.cc @@ -149,7 +149,8 @@ class MemoryMappable : public Mappable { }; Mappable* Mappable::FromPath(const char* path) { - return new FileMappable(File::Open(/*namespc=*/nullptr, path, File::kRead)); + return new FileMappable(File::Open(/*namespc=*/nullptr, path, File::kRead, + /*executable=*/true)); } #if defined(DART_HOST_OS_FUCHSIA) || defined(DART_HOST_OS_LINUX) @@ -583,7 +584,6 @@ DART_EXPORT Dart_LoadedElf* Dart_LoadELF_Fd(int fd, } #endif -#if !defined(DART_HOST_OS_FUCHSIA) DART_EXPORT Dart_LoadedElf* Dart_LoadELF(const char* filename, uint64_t file_offset, const char** error, @@ -608,7 +608,6 @@ DART_EXPORT Dart_LoadedElf* Dart_LoadELF(const char* filename, return reinterpret_cast(elf.release()); } -#endif DART_EXPORT Dart_LoadedElf* Dart_LoadELF_Memory( const uint8_t* snapshot, diff --git a/runtime/bin/elf_loader.h b/runtime/bin/elf_loader.h index 483fbe79bb4..5223a4d9f44 100644 --- a/runtime/bin/elf_loader.h +++ b/runtime/bin/elf_loader.h @@ -36,7 +36,6 @@ DART_EXPORT Dart_LoadedElf* Dart_LoadELF_Fd(int fd, const uint8_t** vm_isolate_instrs); #endif -#if !defined(__Fuchsia__) /// Please see documentation for Dart_LoadElf_Fd. DART_EXPORT Dart_LoadedElf* Dart_LoadELF(const char* filename, uint64_t file_offset, @@ -45,7 +44,6 @@ DART_EXPORT Dart_LoadedElf* Dart_LoadELF(const char* filename, const uint8_t** vm_snapshot_instrs, const uint8_t** vm_isolate_data, const uint8_t** vm_isolate_instrs); -#endif /// Please see documentation for Dart_LoadElf_Fd. DART_EXPORT Dart_LoadedElf* Dart_LoadELF_Memory( diff --git a/runtime/bin/file.h b/runtime/bin/file.h index 17493677367..10dca793241 100644 --- a/runtime/bin/file.h +++ b/runtime/bin/file.h @@ -223,11 +223,17 @@ class File : public ReferenceCounted { // reading and writing. If mode contains kWrite and the file does // not exist the file is created. The file is truncated to length 0 if // mode contains kTruncate. - static File* Open(Namespace* namespc, const char* path, FileOpenMode mode); + static File* Open(Namespace* namespc, + const char* path, + FileOpenMode mode, + bool executable = false); // Same as [File::Open], but attempts to convert uri to path before opening // the file. If conversion fails, uri is treated as a path. - static File* OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode); + static File* OpenUri(Namespace* namespc, + const char* uri, + FileOpenMode mode, + bool executable = false); // Attempts to convert the given [uri] to a file path. static CStringUniquePtr UriToPath(const char* uri); diff --git a/runtime/bin/file_fuchsia.cc b/runtime/bin/file_fuchsia.cc index e54b564b4f8..63da97b14ca 100644 --- a/runtime/bin/file_fuchsia.cc +++ b/runtime/bin/file_fuchsia.cc @@ -7,16 +7,18 @@ #include "bin/file.h" -#include // NOLINT -#include // NOLINT -#include // NOLINT -#include // NOLINT -#include // NOLINT -#include // NOLINT -#include // NOLINT -#include // NOLINT -#include // NOLINT -#include // NOLINT +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include "bin/builtin.h" #include "bin/fdutils.h" @@ -216,7 +218,10 @@ File* File::OpenFD(int fd) { return new File(new FileHandle(fd)); } -File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) { +File* File::Open(Namespace* namespc, + const char* name, + FileOpenMode mode, + bool executable) { NamespaceScope ns(namespc, name); // Report errors for non-regular files. struct stat st; @@ -239,7 +244,18 @@ File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) { flags = flags | O_TRUNC; } flags |= O_CLOEXEC; - int fd = NO_RETRY_EXPECTED(openat(ns.fd(), ns.path(), flags, 0666)); + int fd; + if (executable) { + fuchsia::io::Flags flags = + fuchsia::io::PERM_READABLE | fuchsia::io::PERM_EXECUTABLE; + zx_status_t status = + fdio_open3_fd_at(ns.fd(), ns.path(), uint64_t{flags}, &fd); + if (status != ZX_OK) { + return nullptr; + } + } else { + fd = NO_RETRY_EXPECTED(openat(ns.fd(), ns.path(), flags, 0666)); + } if (fd < 0) { return nullptr; } @@ -264,12 +280,15 @@ CStringUniquePtr File::UriToPath(const char* uri) { return CStringUniquePtr(strdup(uri_decoder.decoded())); } -File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) { +File* File::OpenUri(Namespace* namespc, + const char* uri, + FileOpenMode mode, + bool executable) { auto path = UriToPath(uri); if (path == nullptr) { return nullptr; } - return File::Open(namespc, path.get(), mode); + return File::Open(namespc, path.get(), mode, executable); } File* File::OpenStdio(int fd) { diff --git a/runtime/bin/file_linux.cc b/runtime/bin/file_linux.cc index 6e907ed93c1..471ed13bafc 100644 --- a/runtime/bin/file_linux.cc +++ b/runtime/bin/file_linux.cc @@ -227,7 +227,10 @@ File* File::OpenFD(int fd) { return new File(new FileHandle(fd)); } -File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) { +File* File::Open(Namespace* namespc, + const char* name, + FileOpenMode mode, + bool executable) { NamespaceScope ns(namespc, name); // Report errors for non-regular files. struct stat64 st; @@ -276,12 +279,15 @@ CStringUniquePtr File::UriToPath(const char* uri) { return CStringUniquePtr(strdup(uri_decoder.decoded())); } -File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) { +File* File::OpenUri(Namespace* namespc, + const char* uri, + FileOpenMode mode, + bool executable) { auto path = UriToPath(uri); if (path == nullptr) { return nullptr; } - return File::Open(namespc, path.get(), mode); + return File::Open(namespc, path.get(), mode, executable); } File* File::OpenStdio(int fd) { diff --git a/runtime/bin/file_macos.cc b/runtime/bin/file_macos.cc index 3cf27a0d9eb..00849f2452a 100644 --- a/runtime/bin/file_macos.cc +++ b/runtime/bin/file_macos.cc @@ -266,7 +266,10 @@ File* File::OpenFD(int fd) { return new File(new FileHandle(fd)); } -File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) { +File* File::Open(Namespace* namespc, + const char* name, + FileOpenMode mode, + bool executable) { // Report errors for non-regular files. struct stat st; if (NO_RETRY_EXPECTED(stat(name, &st)) == 0) { @@ -314,12 +317,15 @@ CStringUniquePtr File::UriToPath(const char* uri) { return CStringUniquePtr(strdup(uri_decoder.decoded())); } -File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) { +File* File::OpenUri(Namespace* namespc, + const char* uri, + FileOpenMode mode, + bool executable) { auto path = UriToPath(uri); if (path == nullptr) { return nullptr; } - return File::Open(namespc, path.get(), mode); + return File::Open(namespc, path.get(), mode, executable); } File* File::OpenStdio(int fd) { diff --git a/runtime/bin/file_win.cc b/runtime/bin/file_win.cc index 0fb5f6151d3..84349659449 100644 --- a/runtime/bin/file_win.cc +++ b/runtime/bin/file_win.cc @@ -404,7 +404,10 @@ std::unique_ptr ToWinAPIPath(const char* utf8_path) { return result; } -File* File::Open(Namespace* namespc, const char* name, FileOpenMode mode) { +File* File::Open(Namespace* namespc, + const char* name, + FileOpenMode mode, + bool executable) { const auto path = ToWinAPIPath(name); if (path.get() == nullptr) { SetLastError(ERROR_INVALID_NAME); @@ -437,12 +440,15 @@ CStringUniquePtr File::UriToPath(const char* uri) { return utf8_path.release(); } -File* File::OpenUri(Namespace* namespc, const char* uri, FileOpenMode mode) { +File* File::OpenUri(Namespace* namespc, + const char* uri, + FileOpenMode mode, + bool executable) { auto path = UriToPath(uri); if (path == nullptr) { return nullptr; } - return Open(namespc, path.get(), mode); + return Open(namespc, path.get(), mode, executable); } File* File::OpenStdio(int fd) { diff --git a/runtime/bin/snapshot_utils.cc b/runtime/bin/snapshot_utils.cc index 0ff14997265..460832a4a70 100644 --- a/runtime/bin/snapshot_utils.cc +++ b/runtime/bin/snapshot_utils.cc @@ -188,9 +188,7 @@ static AppSnapshot* TryReadAppSnapshotElf( *isolate_data_buffer = nullptr, *isolate_instructions_buffer = nullptr; Dart_LoadedElf* handle = nullptr; -#if !defined(DART_HOST_OS_FUCHSIA) if (force_load_elf_from_memory) { -#endif File* const file = File::Open(/*namespc=*/nullptr, script_name, File::kRead); if (file == nullptr) return nullptr; @@ -205,13 +203,11 @@ static AppSnapshot* TryReadAppSnapshotElf( &isolate_data_buffer, &isolate_instructions_buffer); delete memory; file->Release(); -#if !defined(DART_HOST_OS_FUCHSIA) } else { handle = Dart_LoadELF(script_name, file_offset, &error, &vm_data_buffer, &vm_instructions_buffer, &isolate_data_buffer, &isolate_instructions_buffer); } -#endif if (handle == nullptr) { Syslog::PrintErr("Loading failed: %s\n", error); return nullptr; diff --git a/tools/bots/test_matrix.json b/tools/bots/test_matrix.json index 2571a916afc..ea46a7affd9 100644 --- a/tools/bots/test_matrix.json +++ b/tools/bots/test_matrix.json @@ -313,7 +313,7 @@ "enable-asserts": true } }, - "vm-aot-(linux|mac)-(debug|product|release)-(x64|x64c|arm64|arm64c)": { + "vm-aot-(linux|mac|fuchsia)-(debug|product|release)-(x64|x64c|arm64|arm64c)": { "options": {} }, "vm-aot-win-(debug|product|release)-(x64|x64c|arm64|arm64c)": { @@ -1641,6 +1641,14 @@ "-j4", "ffi" ] + }, + { + "name": "vm tests", + "arguments": [ + "-nvm-aot-${system}-${mode}-${arch}", + "-j4", + "ffi" + ] } ] }, @@ -1667,6 +1675,14 @@ "-j1", "ffi" ] + }, + { + "name": "vm tests", + "arguments": [ + "-nvm-aot-${system}-${mode}-${arch}", + "-j1", + "ffi" + ] } ] },