From 68735f74e7e50ad562e158ca17287ee0cfdb1715 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Wed, 30 Jun 2021 12:27:45 -0700 Subject: [PATCH] NFC: use enum SymbolData as flags To make it easier to add flags when adding new options in SymbolData. Example: I want to add a flag to disable inline record for https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2915828. Change-Id: Ifc5da27c01efa0b0bc21cfcf769d4e6d604a63c6 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2984198 Reviewed-by: Joshua Peraza --- src/common/linux/dump_symbols.cc | 5 +++-- src/common/mac/dump_syms.cc | 6 +++--- src/common/module.cc | 4 ++-- src/common/module.h | 4 ++-- src/common/module_unittest.cc | 2 +- src/common/symbol_data.h | 22 +++++++++++++++++++--- src/tools/linux/dump_syms/dump_syms.cc | 3 ++- src/tools/mac/dump_syms/dump_syms_tool.cc | 3 ++- src/tools/mac/symupload/symupload.mm | 2 +- 9 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc index e8d0bd80..5909d6ba 100644 --- a/src/common/linux/dump_symbols.cc +++ b/src/common/linux/dump_symbols.cc @@ -680,7 +680,8 @@ bool LoadSymbols(const string& obj_file, bool found_debug_info_section = false; bool found_usable_info = false; - if (options.symbol_data != ONLY_CFI) { + if ((options.symbol_data & SYMBOLS_AND_FILES) || + (options.symbol_data & INLINES)) { #ifndef NO_STABS_SUPPORT // Look for STABS debugging information, and load it if present. const Shdr* stab_section = @@ -789,7 +790,7 @@ bool LoadSymbols(const string& obj_file, } } - if (options.symbol_data != NO_CFI) { + if (options.symbol_data & CFI) { // Dwarf Call Frame Information (CFI) is actually independent from // the other DWARF debugging information, and can be used alone. const Shdr* dwarf_cfi_section = diff --git a/src/common/mac/dump_syms.cc b/src/common/mac/dump_syms.cc index 77e7bee3..e30d8ea9 100644 --- a/src/common/mac/dump_syms.cc +++ b/src/common/mac/dump_syms.cc @@ -584,7 +584,7 @@ bool DumpSymbols::LoadCommandDumper::SegmentCommand(const Segment& segment) { if (segment.name == "__TEXT") { module_->SetLoadAddress(segment.vmaddr); - if (symbol_data_ != NO_CFI) { + if (symbol_data_ & CFI) { mach_o::SectionMap::const_iterator eh_frame = section_map.find("__eh_frame"); if (eh_frame != section_map.end()) { @@ -596,10 +596,10 @@ bool DumpSymbols::LoadCommandDumper::SegmentCommand(const Segment& segment) { } if (segment.name == "__DWARF") { - if (symbol_data_ != ONLY_CFI) { + if ((symbol_data_ & SYMBOLS_AND_FILES) || (symbol_data_ & INLINES)) { dumper_.ReadDwarf(module_, reader_, section_map, handle_inter_cu_refs_); } - if (symbol_data_ != NO_CFI) { + if (symbol_data_ & CFI) { mach_o::SectionMap::const_iterator debug_frame = section_map.find("__debug_frame"); if (debug_frame != section_map.end()) { diff --git a/src/common/module.cc b/src/common/module.cc index fc85e643..0ecf6ca6 100644 --- a/src/common/module.cc +++ b/src/common/module.cc @@ -266,7 +266,7 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) { stream << "INFO CODE_ID " << code_id_ << "\n"; } - if (symbol_data != ONLY_CFI) { + if (symbol_data & SYMBOLS_AND_FILES) { AssignSourceIds(); // Write out files. @@ -324,7 +324,7 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) { } } - if (symbol_data != NO_CFI) { + if (symbol_data & CFI) { // Write out 'STACK CFI INIT' and 'STACK CFI' records. vector::const_iterator frame_it; for (frame_it = stack_frame_entries_.begin(); diff --git a/src/common/module.h b/src/common/module.h index ad282836..f2fff490 100644 --- a/src/common/module.h +++ b/src/common/module.h @@ -279,11 +279,11 @@ class Module { // breakpad symbol format. Return true if all goes well, or false if // an error occurs. This method writes out: // - a header based on the values given to the constructor, - // If symbol_data is not ONLY_CFI then: + // If symbol_data is not CFI then: // - the source files added via FindFile, // - the functions added via AddFunctions, each with its lines, // - all public records, - // If symbol_data is not NO_CFI then: + // If symbol_data is CFI then: // - all CFI records. // Addresses in the output are all relative to the load address // established by SetLoadAddress. diff --git a/src/common/module_unittest.cc b/src/common/module_unittest.cc index 0d8aa044..ddccc320 100644 --- a/src/common/module_unittest.cc +++ b/src/common/module_unittest.cc @@ -255,7 +255,7 @@ TEST(Write, NoCFI) { // the module must work fine. m.SetLoadAddress(0x2ab698b0b6407073ULL); - m.Write(s, NO_CFI); + m.Write(s, SYMBOLS_AND_FILES | INLINES); string contents = s.str(); EXPECT_STREQ("MODULE os-name architecture id-string name with spaces\n" "FILE 0 filename.cc\n" diff --git a/src/common/symbol_data.h b/src/common/symbol_data.h index 2cf15a85..a790974b 100644 --- a/src/common/symbol_data.h +++ b/src/common/symbol_data.h @@ -32,11 +32,27 @@ #ifndef COMMON_SYMBOL_DATA_H_ #define COMMON_SYMBOL_DATA_H_ +#include + // Control what data is used from the symbol file. enum SymbolData { - ALL_SYMBOL_DATA, - NO_CFI, - ONLY_CFI + NO_DATA = 0, + SYMBOLS_AND_FILES = 1, + CFI = 1 << 1, + INLINES = 1 << 2, + ALL_SYMBOL_DATA = INLINES | CFI | SYMBOLS_AND_FILES }; +inline SymbolData operator&(SymbolData data1, SymbolData data2) { + return static_cast( + static_cast::type>(data1) & + static_cast::type>(data2)); +} + +inline SymbolData operator|(SymbolData data1, SymbolData data2) { + return static_cast( + static_cast::type>(data1) | + static_cast::type>(data2)); +} + #endif // COMMON_SYMBOL_DATA_H_ diff --git a/src/tools/linux/dump_syms/dump_syms.cc b/src/tools/linux/dump_syms/dump_syms.cc index e101d470..a562bffb 100644 --- a/src/tools/linux/dump_syms/dump_syms.cc +++ b/src/tools/linux/dump_syms/dump_syms.cc @@ -127,7 +127,8 @@ int main(int argc, char** argv) { return 1; } } else { - SymbolData symbol_data = cfi ? ALL_SYMBOL_DATA : NO_CFI; + SymbolData symbol_data = + INLINES | (cfi ? CFI : NO_DATA) | SYMBOLS_AND_FILES; google_breakpad::DumpOptions options(symbol_data, handle_inter_cu_refs); if (!WriteSymbolFile(binary, obj_name, obj_os, debug_dirs, options, std::cout)) { diff --git a/src/tools/mac/dump_syms/dump_syms_tool.cc b/src/tools/mac/dump_syms/dump_syms_tool.cc index 8d73f0bd..df2f49f1 100644 --- a/src/tools/mac/dump_syms/dump_syms_tool.cc +++ b/src/tools/mac/dump_syms/dump_syms_tool.cc @@ -107,7 +107,8 @@ static void CopyCFIDataBetweenModules(Module* to_module, } static bool Start(const Options& options) { - SymbolData symbol_data = options.cfi ? ALL_SYMBOL_DATA : NO_CFI; + SymbolData symbol_data = + INLINES | (options.cfi ? CFI : NO_DATA) | SYMBOLS_AND_FILES; DumpSymbols dump_symbols(symbol_data, options.handle_inter_cu_refs); // For x86_64 binaries, the CFI data is in the __TEXT,__eh_frame of the diff --git a/src/tools/mac/symupload/symupload.mm b/src/tools/mac/symupload/symupload.mm index 5b39d3ce..24835117 100644 --- a/src/tools/mac/symupload/symupload.mm +++ b/src/tools/mac/symupload/symupload.mm @@ -421,7 +421,7 @@ static void SetupOptions(int argc, const char* argv[], Options* options) { if (!isBreakpadUpload && hasCodeFile && !hasDebugID && ([options->type isEqualToString:kMachOSymbolType] || [options->type isEqualToString:kDSYMSymbolType])) { - DumpSymbols dump_symbols(NO_CFI, false); + DumpSymbols dump_symbols(SYMBOLS_AND_FILES | INLINES, false); if (dump_symbols.Read(argv[optind])) { std::string identifier = dump_symbols.Identifier(); if (identifier.empty()) {