From ef55207540d1d0b686f53145d87f6fb29edf3380 Mon Sep 17 00:00:00 2001 From: Leonard Grey Date: Thu, 9 Mar 2023 09:54:53 -0500 Subject: [PATCH] Mac: stop using NXArchInfo as a vocabulary type It's deprecated in macOS 13/iOS 16, so this is an incremental step towards using newly introduced APIs for those OSes. Since the description field is no longer available in the new mach-o/util.h API, stop using it, especially since architecture name is sufficiently informative. Bug: chromium:1420654 Change-Id: If2cec4f1fc88d13a71f011822bff61f173486b68 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4322265 Reviewed-by: Mark Mentovai --- src/common/mac/arch_utilities.cc | 79 ++++++----------------- src/common/mac/arch_utilities.h | 26 +++++--- src/common/mac/dump_syms.cc | 47 +++++--------- src/common/mac/dump_syms.h | 20 ++---- src/common/mac/macho_walker.cc | 9 ++- src/tools/mac/dump_syms/dump_syms_tool.cc | 26 +++----- 6 files changed, 72 insertions(+), 135 deletions(-) diff --git a/src/common/mac/arch_utilities.cc b/src/common/mac/arch_utilities.cc index 543c7c4a..cdc1dfa5 100644 --- a/src/common/mac/arch_utilities.cc +++ b/src/common/mac/arch_utilities.cc @@ -53,86 +53,44 @@ #define CPU_SUBTYPE_ARM64_E (static_cast(2)) #endif // CPU_SUBTYPE_ARM64_E -namespace { - -const NXArchInfo* ArchInfo_arm64(cpu_subtype_t cpu_subtype) { - const char* name = NULL; - switch (cpu_subtype) { - case CPU_SUBTYPE_ARM64_ALL: - name = "arm64"; - break; - case CPU_SUBTYPE_ARM64_E: - name = "arm64e"; - break; - default: - return NULL; - } - - NXArchInfo* arm64 = new NXArchInfo; - *arm64 = *NXGetArchInfoFromCpuType(CPU_TYPE_ARM, - CPU_SUBTYPE_ARM_V7); - arm64->name = name; - arm64->cputype = CPU_TYPE_ARM64; - arm64->cpusubtype = cpu_subtype; - arm64->description = "arm 64"; - return arm64; -} - -const NXArchInfo* ArchInfo_armv7s() { - NXArchInfo* armv7s = new NXArchInfo; - *armv7s = *NXGetArchInfoFromCpuType(CPU_TYPE_ARM, - CPU_SUBTYPE_ARM_V7); - armv7s->name = "armv7s"; - armv7s->cpusubtype = CPU_SUBTYPE_ARM_V7S; - armv7s->description = "arm v7s"; - return armv7s; -} - -} // namespace - -namespace google_breakpad { - -const NXArchInfo* BreakpadGetArchInfoFromName(const char* arch_name) { +std::optional GetArchInfoFromName(const char* arch_name) { // TODO: Remove this when the OS knows about arm64. if (!strcmp("arm64", arch_name)) - return BreakpadGetArchInfoFromCpuType(CPU_TYPE_ARM64, - CPU_SUBTYPE_ARM64_ALL); + return ArchInfo{CPU_TYPE_ARM64, CPU_SUBTYPE_ARM64_ALL}; if (!strcmp("arm64e", arch_name)) - return BreakpadGetArchInfoFromCpuType(CPU_TYPE_ARM64, - CPU_SUBTYPE_ARM64_E); - + return ArchInfo{CPU_TYPE_ARM64, CPU_SUBTYPE_ARM64_E}; // TODO: Remove this when the OS knows about armv7s. if (!strcmp("armv7s", arch_name)) - return BreakpadGetArchInfoFromCpuType(CPU_TYPE_ARM, CPU_SUBTYPE_ARM_V7S); + return ArchInfo{CPU_TYPE_ARM, CPU_SUBTYPE_ARM_V7S}; - return NXGetArchInfoFromName(arch_name); + const NXArchInfo* info = NXGetArchInfoFromName(arch_name); + if (info) + return ArchInfo{info->cputype, info->cpusubtype}; + return std::nullopt; } -const NXArchInfo* BreakpadGetArchInfoFromCpuType(cpu_type_t cpu_type, - cpu_subtype_t cpu_subtype) { +const char* GetNameFromCPUType(cpu_type_t cpu_type, cpu_subtype_t cpu_subtype) { // TODO: Remove this when the OS knows about arm64. if (cpu_type == CPU_TYPE_ARM64 && cpu_subtype == CPU_SUBTYPE_ARM64_ALL) { - static const NXArchInfo* arm64 = ArchInfo_arm64(cpu_subtype); - return arm64; + return "arm64"; } if (cpu_type == CPU_TYPE_ARM64 && cpu_subtype == CPU_SUBTYPE_ARM64_E) { - static const NXArchInfo* arm64e = ArchInfo_arm64(cpu_subtype); - return arm64e; + return "arm64e"; } // TODO: Remove this when the OS knows about armv7s. if (cpu_type == CPU_TYPE_ARM && cpu_subtype == CPU_SUBTYPE_ARM_V7S) { - static const NXArchInfo* armv7s = ArchInfo_armv7s(); - return armv7s; + return "armv7s"; } - return NXGetArchInfoFromCpuType(cpu_type, cpu_subtype); + const NXArchInfo* info = NXGetArchInfoFromCpuType(cpu_type, cpu_subtype); + if (info) + return info->name; + return kUnknownArchName; } -} // namespace google_breakpad - // TODO(crbug.com/1242776): The "#ifndef __APPLE__" should be here, but the // system version of NXGetLocalArchInfo returns incorrect information on // x86_64 machines (treating them as just x86), so use the Breakpad version @@ -207,7 +165,7 @@ const NXArchInfo kKnownArchitectures[] = { } // namespace -const NXArchInfo *NXGetLocalArchInfo(void) { +ArchInfo GetLocalArchInfo(void) { Architecture arch; #if defined(__i386__) arch = kArch_i386; @@ -222,7 +180,8 @@ const NXArchInfo *NXGetLocalArchInfo(void) { #else #error "Unsupported CPU architecture" #endif - return &kKnownArchitectures[arch]; + NXArchInfo info = kKnownArchitectures[arch]; + return {info.cputype, info.cpusubtype}; } #ifndef __APPLE__ diff --git a/src/common/mac/arch_utilities.h b/src/common/mac/arch_utilities.h index d267c43b..3b036738 100644 --- a/src/common/mac/arch_utilities.h +++ b/src/common/mac/arch_utilities.h @@ -31,16 +31,26 @@ #ifndef COMMON_MAC_ARCH_UTILITIES_H__ #define COMMON_MAC_ARCH_UTILITIES_H__ -#include +#include -namespace google_breakpad { +#include -// Custom implementation of |NXGetArchInfoFromName| and -// |NXGetArchInfoFromCpuType| that handle newer CPU on older OSes. -const NXArchInfo* BreakpadGetArchInfoFromName(const char* arch_name); -const NXArchInfo* BreakpadGetArchInfoFromCpuType(cpu_type_t cpu_type, - cpu_subtype_t cpu_subtype); +static constexpr const char* kUnknownArchName = ""; -} // namespace google_breakpad +struct ArchInfo { + cpu_type_t cputype; + cpu_subtype_t cpusubtype; +}; + +// Returns architecture info if `arch_name` corresponds to a valid, known +// architecture, and std::nullopt otherwise. +std::optional GetArchInfoFromName(const char* arch_name); + +// Returns the name of the architecture specified by `cpu_type` and +// `cpu_subtype`, or `kUnknownArchName` if it's unknown or invalid. +const char* GetNameFromCPUType(cpu_type_t cpu_type, cpu_subtype_t cpu_subtype); + +// Returns the architecture of the machine this code is running on. +ArchInfo GetLocalArchInfo(); #endif // COMMON_MAC_ARCH_UTILITIES_H__ diff --git a/src/common/mac/dump_syms.cc b/src/common/mac/dump_syms.cc index e1025f79..efa60f5b 100644 --- a/src/common/mac/dump_syms.cc +++ b/src/common/mac/dump_syms.cc @@ -221,11 +221,10 @@ bool DumpSymbols::ReadData(uint8_t* contents, size_t size, return true; } -bool DumpSymbols::SetArchitecture(cpu_type_t cpu_type, - cpu_subtype_t cpu_subtype) { +bool DumpSymbols::SetArchitecture(const ArchInfo& info) { // Find the best match for the architecture the user requested. - const SuperFatArch* best_match = FindBestMatchForArchitecture( - cpu_type, cpu_subtype); + const SuperFatArch* best_match = + FindBestMatchForArchitecture(info.cputype, info.cpusubtype); if (!best_match) return false; // Record the selected object file. @@ -233,16 +232,6 @@ bool DumpSymbols::SetArchitecture(cpu_type_t cpu_type, return true; } -bool DumpSymbols::SetArchitecture(const std::string& arch_name) { - bool arch_set = false; - const NXArchInfo* arch_info = - google_breakpad::BreakpadGetArchInfoFromName(arch_name.c_str()); - if (arch_info) { - arch_set = SetArchitecture(arch_info->cputype, arch_info->cpusubtype); - } - return arch_set; -} - SuperFatArch* DumpSymbols::FindBestMatchForArchitecture( cpu_type_t cpu_type, cpu_subtype_t cpu_subtype) { // Check if all the object files can be converted to struct fat_arch. @@ -402,8 +391,8 @@ bool DumpSymbols::CreateEmptyModule(scoped_ptr& module) { selected_object_file_ = &object_files_[0]; else { // Look for an object file whose architecture matches our own. - const NXArchInfo* local_arch = NXGetLocalArchInfo(); - if (!SetArchitecture(local_arch->cputype, local_arch->cpusubtype)) { + ArchInfo local_arch = GetLocalArchInfo(); + if (!SetArchitecture(local_arch)) { fprintf(stderr, "%s: object file contains more than one" " architecture, none of which match the current" " architecture; specify an architecture explicitly" @@ -418,18 +407,16 @@ bool DumpSymbols::CreateEmptyModule(scoped_ptr& module) { // Find the name of the selected file's architecture, to appear in // the MODULE record and in error messages. - const NXArchInfo* selected_arch_info = - google_breakpad::BreakpadGetArchInfoFromCpuType( - selected_object_file_->cputype, selected_object_file_->cpusubtype); + const char* selected_arch_name = GetNameFromCPUType( + selected_object_file_->cputype, selected_object_file_->cpusubtype); // In certain cases, it is possible that architecture info can't be reliably // determined, e.g. new architectures that breakpad is unware of. In that // case, avoid crashing and return false instead. - if (selected_arch_info == NULL) { + if (selected_arch_name == kUnknownArchName) { return false; } - const char* selected_arch_name = selected_arch_info->name; if (strcmp(selected_arch_name, "i386") == 0) selected_arch_name = "x86"; @@ -540,16 +527,14 @@ bool DumpSymbols::ReadCFI(google_breakpad::Module* module, register_names = DwarfCFIToModule::RegisterNames::ARM64(); break; default: { - const NXArchInfo* arch = google_breakpad::BreakpadGetArchInfoFromCpuType( - macho_reader.cpu_type(), macho_reader.cpu_subtype()); - fprintf(stderr, "%s: cannot convert DWARF call frame information for ", - selected_object_name_.c_str()); - if (arch) - fprintf(stderr, "architecture '%s'", arch->name); - else - fprintf(stderr, "architecture %d,%d", - macho_reader.cpu_type(), macho_reader.cpu_subtype()); - fprintf(stderr, " to Breakpad symbol file: no register name table\n"); + const char* arch_name = GetNameFromCPUType(macho_reader.cpu_type(), + macho_reader.cpu_subtype()); + fprintf( + stderr, + "%s: cannot convert DWARF call frame information for architecture " + "'%s' (%d, %d) to Breakpad symbol file: no register name table\n", + selected_object_name_.c_str(), arch_name, macho_reader.cpu_type(), + macho_reader.cpu_subtype()); return false; } } diff --git a/src/common/mac/dump_syms.h b/src/common/mac/dump_syms.h index c2e1b40b..c22a0575 100644 --- a/src/common/mac/dump_syms.h +++ b/src/common/mac/dump_syms.h @@ -43,6 +43,7 @@ #include #include "common/byte_cursor.h" +#include "common/mac/arch_utilities.h" #include "common/mac/macho_reader.h" #include "common/mac/super_fat_arch.h" #include "common/module.h" @@ -82,26 +83,15 @@ class DumpSymbols { // problem reading |contents|, report it and return false. bool ReadData(uint8_t* contents, size_t size, const std::string& filename); - // If this dumper's file includes an object file for |cpu_type| and - // |cpu_subtype|, then select that object file for dumping, and return - // true. Otherwise, return false, and leave this dumper's selected - // architecture unchanged. + // If this dumper's file includes an object file for `info`, then select that + // object file for dumping, and return true. Otherwise, return false, and + // leave this dumper's selected architecture unchanged. // // By default, if this dumper's file contains only one object file, then // the dumper will dump those symbols; and if it contains more than one // object file, then the dumper will dump the object file whose // architecture matches that of this dumper program. - bool SetArchitecture(cpu_type_t cpu_type, cpu_subtype_t cpu_subtype); - - // If this dumper's file includes an object file for |arch_name|, then select - // that object file for dumping, and return true. Otherwise, return false, - // and leave this dumper's selected architecture unchanged. - // - // By default, if this dumper's file contains only one object file, then - // the dumper will dump those symbols; and if it contains more than one - // object file, then the dumper will dump the object file whose - // architecture matches that of this dumper program. - bool SetArchitecture(const std::string& arch_name); + bool SetArchitecture(const ArchInfo& info); // Return a pointer to an array of SuperFatArch structures describing the // object files contained in this dumper's file. Set *|count| to the number diff --git a/src/common/mac/macho_walker.cc b/src/common/mac/macho_walker.cc index f9cd9327..4b9f56c2 100644 --- a/src/common/mac/macho_walker.cc +++ b/src/common/mac/macho_walker.cc @@ -38,15 +38,15 @@ #include #include -#include #include #include #include #include +#include "common/mac/arch_utilities.h" #include "common/mac/byteswap.h" -#include "common/mac/macho_walker.h" #include "common/mac/macho_utilities.h" +#include "common/mac/macho_walker.h" namespace MacFileUtilities { @@ -85,9 +85,8 @@ bool MachoWalker::WalkHeader(cpu_type_t cpu_type, cpu_subtype_t cpu_subtype) { cpu_subtype_t valid_cpu_subtype = cpu_subtype; // if |cpu_type| is 0, use the native cpu type. if (cpu_type == 0) { - const NXArchInfo* arch = NXGetLocalArchInfo(); - assert(arch); - valid_cpu_type = arch->cputype; + ArchInfo arch = GetLocalArchInfo(); + valid_cpu_type = arch.cputype; valid_cpu_subtype = CPU_SUBTYPE_MULTIPLE; } off_t offset; diff --git a/src/tools/mac/dump_syms/dump_syms_tool.cc b/src/tools/mac/dump_syms/dump_syms_tool.cc index 2f2815c5..4d6f25c9 100644 --- a/src/tools/mac/dump_syms/dump_syms_tool.cc +++ b/src/tools/mac/dump_syms/dump_syms_tool.cc @@ -67,7 +67,7 @@ struct Options { string srcPath; string dsymPath; - const NXArchInfo *arch; + std::optional arch; bool header_only; bool cfi; bool handle_inter_cu_refs; @@ -121,11 +121,12 @@ static void CopyCFIDataBetweenModules(Module* to_module, } static bool SetArchitecture(DumpSymbols& dump_symbols, - const NXArchInfo* arch, + const ArchInfo& arch, const std::string& filename) { - if (!dump_symbols.SetArchitecture(arch->cputype, arch->cpusubtype)) { + if (!dump_symbols.SetArchitecture(arch)) { fprintf(stderr, "%s: no architecture '%s' is present in file.\n", - filename.c_str(), arch->name); + filename.c_str(), + GetNameFromCPUType(arch.cputype, arch.cpusubtype)); size_t available_size; const SuperFatArch* available = dump_symbols.AvailableArchitectures(&available_size); @@ -135,14 +136,8 @@ static bool SetArchitecture(DumpSymbols& dump_symbols, fprintf(stderr, "architectures present in the file are:\n"); for (size_t i = 0; i < available_size; i++) { const SuperFatArch* arch = &available[i]; - const NXArchInfo* arch_info = - google_breakpad::BreakpadGetArchInfoFromCpuType(arch->cputype, - arch->cpusubtype); - if (arch_info) - fprintf(stderr, "%s (%s)\n", arch_info->name, arch_info->description); - else - fprintf(stderr, "unrecognized cpu type 0x%x, subtype 0x%x\n", - arch->cputype, arch->cpusubtype); + fprintf(stderr, "%s\n", + GetNameFromCPUType(arch->cputype, arch->cpusubtype)); } return false; } @@ -173,7 +168,7 @@ static bool Start(const Options& options) { return false; if (options.arch && - !SetArchitecture(dump_symbols, options.arch, primary_file)) { + !SetArchitecture(dump_symbols, *options.arch, primary_file)) { return false; } @@ -193,7 +188,7 @@ static bool Start(const Options& options) { return false; if (options.arch && - !SetArchitecture(dump_symbols, options.arch, options.srcPath)) { + !SetArchitecture(dump_symbols, *options.arch, options.srcPath)) { return false; } Module* cfi_module = NULL; @@ -248,8 +243,7 @@ static void SetupOptions(int argc, const char *argv[], Options *options) { options->header_only = true; break; case 'a': { - const NXArchInfo *arch_info = - google_breakpad::BreakpadGetArchInfoFromName(optarg); + std::optional arch_info = GetArchInfoFromName(optarg); if (!arch_info) { fprintf(stderr, "%s: Invalid architecture: %s\n", argv[0], optarg); Usage(argc, argv);