From 9f96d5c7b7960cbc67942dc28664753557ed186a Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Thu, 18 May 2023 15:22:25 -0400 Subject: [PATCH] Fix inline_origin_map key collision when split dwarf is enabled. It fixes following two problems: 1. When we have skeleton compilation unit (DW_TAG_skeleton_unit) in a binary file refers to the complete unit in a split dwarf file (.dwo/.dwp file), we should use the split dwarf file's path in warning reporting. Right now, it uses the original file (binary file) path in warning report, which is incorrect. For example, if we have chrome.debug which is the binary with skeleton debug info and chrome.dwp which is the complete debug info and the debug info in chrome.dwp has some incorrect reference, it will warn on chrome.debug rather than chrome.dwp 2. When split dwarf is enabled, the global inline_origin_map will likely encounter key collision because the offsets as keys are now relative to each CU's offset which is relative to .debug_info section. Also offsets from different files might collide. This change makes a inline_origin_map for each debug file and use offsets only relative to .debug_info section as keys. Bug: b/280290608 Change-Id: If70e2e1bfcbeeeef2d425c918796d351a0e9ab3b Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4544694 Reviewed-by: Joshua Peraza Reviewed-by: Mark Mentovai --- src/common/dwarf/dwarf2reader.cc | 109 ++++++++++-------- src/common/dwarf/dwarf2reader.h | 34 ++++-- src/common/dwarf/dwarf2reader_die_unittest.cc | 3 +- src/common/dwarf_cu_to_module.cc | 18 +-- src/common/linux/dump_symbols.cc | 39 +++++++ src/common/mac/dump_syms.cc | 50 +++++++- src/common/mac/dump_syms.h | 8 ++ src/common/module.cc | 5 +- src/common/module.h | 8 +- src/common/module_unittest.cc | 4 +- 10 files changed, 190 insertions(+), 88 deletions(-) diff --git a/src/common/dwarf/dwarf2reader.cc b/src/common/dwarf/dwarf2reader.cc index 65cb8e7d..4f63b979 100644 --- a/src/common/dwarf/dwarf2reader.cc +++ b/src/common/dwarf/dwarf2reader.cc @@ -81,8 +81,8 @@ CompilationUnit::CompilationUnit(const string& path, addr_buffer_(NULL), addr_buffer_length_(0), is_split_dwarf_(false), is_type_unit_(false), dwo_id_(0), dwo_name_(), skeleton_dwo_id_(0), ranges_base_(0), addr_base_(0), - str_offsets_base_(0), have_checked_for_dwp_(false), dwp_path_(), - dwp_byte_reader_(), dwp_reader_() {} + str_offsets_base_(0), have_checked_for_dwp_(false), + should_process_split_dwarf_(false) {} // Initialize a compilation unit from a .dwo or .dwp file. // In this case, we need the .debug_addr section from the @@ -400,7 +400,13 @@ uint64_t CompilationUnit::Start() { // Set up our buffer buffer_ = iter->second.first + offset_from_section_start_; - buffer_length_ = iter->second.second - offset_from_section_start_; + if (is_split_dwarf_) { + iter = GetSectionByName(sections_, ".debug_info_offset"); + assert(iter != sections_.end()); + buffer_length_ = iter->second.second; + } else { + buffer_length_ = iter->second.second - offset_from_section_start_; + } // Read the header ReadHeader(); @@ -461,10 +467,8 @@ uint64_t CompilationUnit::Start() { // If this is a skeleton compilation unit generated with split DWARF, // and the client needs the full debug info, we need to find the full // compilation unit in a .dwo or .dwp file. - if (!is_split_dwarf_ - && dwo_name_ != NULL - && handler_->NeedSplitDebugInfo()) - ProcessSplitDwarf(); + should_process_split_dwarf_ = + !is_split_dwarf_ && dwo_name_ != NULL && handler_->NeedSplitDebugInfo(); return ourlength; } @@ -994,66 +998,68 @@ inline int GetElfWidth(const ElfReader& elf) { return 0; } -void CompilationUnit::ProcessSplitDwarf() { +bool CompilationUnit::ProcessSplitDwarf(std::string& split_file, + SectionMap& sections, + ByteReader& split_byte_reader, + uint64_t& cu_offset) { + if (!should_process_split_dwarf_) + return false; struct stat statbuf; + bool found_in_dwp = false; if (!have_checked_for_dwp_) { // Look for a .dwp file in the same directory as the executable. have_checked_for_dwp_ = true; string dwp_suffix(".dwp"); - dwp_path_ = path_ + dwp_suffix; - if (stat(dwp_path_.c_str(), &statbuf) != 0) { + std::string dwp_path = path_ + dwp_suffix; + if (stat(dwp_path.c_str(), &statbuf) != 0) { // Fall back to a split .debug file in the same directory. string debug_suffix(".debug"); - dwp_path_ = path_; + dwp_path = path_; size_t found = path_.rfind(debug_suffix); if (found + debug_suffix.length() == path_.length()) - dwp_path_ = dwp_path_.replace(found, debug_suffix.length(), dwp_suffix); + dwp_path = dwp_path.replace(found, debug_suffix.length(), dwp_suffix); } - if (stat(dwp_path_.c_str(), &statbuf) == 0) { - ElfReader* elf = new ElfReader(dwp_path_); - int width = GetElfWidth(*elf); + if (stat(dwp_path.c_str(), &statbuf) == 0) { + split_elf_reader_ = std::make_unique(dwp_path); + int width = GetElfWidth(*split_elf_reader_.get()); if (width != 0) { - dwp_byte_reader_.reset(new ByteReader(reader_->GetEndianness())); - dwp_byte_reader_->SetAddressSize(width); - dwp_reader_.reset(new DwpReader(*dwp_byte_reader_, elf)); + split_byte_reader = ByteReader(reader_->GetEndianness()); + split_byte_reader.SetAddressSize(width); + dwp_reader_ = std::make_unique(split_byte_reader, + split_elf_reader_.get()); dwp_reader_->Initialize(); - } else { - delete elf; + // If we have a .dwp file, read the debug sections for the requested CU. + dwp_reader_->ReadDebugSectionsForCU(dwo_id_, §ions); + if (!sections.empty()) { + SectionMap::const_iterator cu_iter = + GetSectionByName(sections, ".debug_info_offset"); + SectionMap::const_iterator debug_info_iter = + GetSectionByName(sections, ".debug_info"); + assert(cu_iter != sections.end()); + assert(debug_info_iter != sections.end()); + cu_offset = cu_iter->second.first - debug_info_iter->second.first; + found_in_dwp = true; + split_file = dwp_path; + } } } } - bool found_in_dwp = false; - if (dwp_reader_) { - // If we have a .dwp file, read the debug sections for the requested CU. - SectionMap sections; - dwp_reader_->ReadDebugSectionsForCU(dwo_id_, §ions); - if (!sections.empty()) { - found_in_dwp = true; - CompilationUnit dwp_comp_unit(dwp_path_, sections, 0, - dwp_byte_reader_.get(), handler_); - dwp_comp_unit.SetSplitDwarf(addr_buffer_, addr_buffer_length_, addr_base_, - ranges_base_, dwo_id_); - dwp_comp_unit.Start(); - } - } if (!found_in_dwp) { // If no .dwp file, try to open the .dwo file. if (stat(dwo_name_, &statbuf) == 0) { - ElfReader elf(dwo_name_); - int width = GetElfWidth(elf); + split_elf_reader_ = std::make_unique(dwo_name_); + int width = GetElfWidth(*split_elf_reader_.get()); if (width != 0) { - ByteReader reader(ENDIANNESS_LITTLE); - reader.SetAddressSize(width); - SectionMap sections; - ReadDebugSectionsFromDwo(&elf, §ions); - CompilationUnit dwo_comp_unit(dwo_name_, sections, 0, &reader, - handler_); - dwo_comp_unit.SetSplitDwarf(addr_buffer_, addr_buffer_length_, - addr_base_, ranges_base_, dwo_id_); - dwo_comp_unit.Start(); + split_byte_reader = ByteReader(ENDIANNESS_LITTLE); + split_byte_reader.SetAddressSize(width); + ReadDebugSectionsFromDwo(split_elf_reader_.get(), §ions); + if (!sections.empty()) { + split_file = dwo_name_; + } } } } + return !split_file.empty(); } void CompilationUnit::ReadDebugSectionsFromDwo(ElfReader* elf_reader, @@ -1088,10 +1094,6 @@ DwpReader::DwpReader(const ByteReader& byte_reader, ElfReader* elf_reader) abbrev_size_(0), info_data_(NULL), info_size_(0), str_offsets_data_(NULL), str_offsets_size_(0) {} -DwpReader::~DwpReader() { - if (elf_reader_) delete elf_reader_; -} - void DwpReader::Initialize() { cu_index_ = elf_reader_->GetSectionByName(".debug_cu_index", &cu_index_size_); @@ -1231,8 +1233,13 @@ void DwpReader::ReadDebugSectionsForCU(uint64_t dwo_id, } else if (section_id == DW_SECT_INFO) { sections->insert(std::make_pair( ".debug_info", - std::make_pair(reinterpret_cast (info_data_) - + offset, size))); + std::make_pair(reinterpret_cast(info_data_), 0))); + // .debug_info_offset will points the buffer for the CU with given + // dwo_id. + sections->insert(std::make_pair( + ".debug_info_offset", + std::make_pair( + reinterpret_cast(info_data_) + offset, size))); } else if (section_id == DW_SECT_STR_OFFSETS) { sections->insert(std::make_pair( ".debug_str_offsets", diff --git a/src/common/dwarf/dwarf2reader.h b/src/common/dwarf/dwarf2reader.h index ddcdd801..cd676dea 100644 --- a/src/common/dwarf/dwarf2reader.h +++ b/src/common/dwarf/dwarf2reader.h @@ -481,6 +481,24 @@ class CompilationUnit { // start of the next compilation unit, if there is one. uint64_t Start(); + // Process the actual debug information in a split DWARF file. + bool ProcessSplitDwarf(std::string& split_file, + SectionMap& sections, + ByteReader& split_byte_reader, + uint64_t& cu_offset); + + const uint8_t* GetAddrBuffer() { return addr_buffer_; } + + uint64_t GetAddrBufferLen() { return addr_buffer_length_; } + + uint64_t GetAddrBase() { return addr_base_; } + + uint64_t GetRangeBase() { return ranges_base_; } + + uint64_t GetDWOID() { return dwo_id_; } + + bool ShouldProcessSplitDwarf() { return should_process_split_dwarf_; } + private: // This struct represents a single DWARF2/3 abbreviation @@ -647,9 +665,6 @@ class CompilationUnit { // new place to position the stream to. const uint8_t* SkipAttribute(const uint8_t* start, enum DwarfForm form); - // Process the actual debug information in a split DWARF file. - void ProcessSplitDwarf(); - // Read the debug sections from a .dwo file. void ReadDebugSectionsFromDwo(ElfReader* elf_reader, SectionMap* sections); @@ -658,7 +673,7 @@ class CompilationUnit { const string path_; // Offset from section start is the offset of this compilation unit - // from the beginning of the .debug_info section. + // from the beginning of the .debug_info/.debug_info.dwo section. uint64_t offset_from_section_start_; // buffer is the buffer for our CU, starting at .debug_info + offset @@ -743,14 +758,13 @@ class CompilationUnit { // True if we have already looked for a .dwp file. bool have_checked_for_dwp_; - // Path to the .dwp file. - string dwp_path_; - - // ByteReader for the DWP file. - std::unique_ptr dwp_byte_reader_; + // ElfReader for the dwo/dwo file. + std::unique_ptr split_elf_reader_; // DWP reader. std::unique_ptr dwp_reader_; + + bool should_process_split_dwarf_; }; // A Reader for a .dwp file. Supports the fetching of DWARF debug @@ -770,8 +784,6 @@ class DwpReader { public: DwpReader(const ByteReader& byte_reader, ElfReader* elf_reader); - ~DwpReader(); - // Read the CU index and initialize data members. void Initialize(); diff --git a/src/common/dwarf/dwarf2reader_die_unittest.cc b/src/common/dwarf/dwarf2reader_die_unittest.cc index 442fa66c..2b365396 100644 --- a/src/common/dwarf/dwarf2reader_die_unittest.cc +++ b/src/common/dwarf/dwarf2reader_die_unittest.cc @@ -333,7 +333,8 @@ struct DwarfFormsFixture: public DIEFixture { uint64_t offset=0) { ByteReader byte_reader(params.endianness == kLittleEndian ? ENDIANNESS_LITTLE : ENDIANNESS_BIG); - CompilationUnit parser("", MakeSectionMap(), offset, &byte_reader, &handler); + CompilationUnit parser("", MakeSectionMap(), offset, &byte_reader, + &handler); EXPECT_EQ(offset + parser.Start(), info_contents.size()); } diff --git a/src/common/dwarf_cu_to_module.cc b/src/common/dwarf_cu_to_module.cc index 708ed143..2a23ca7c 100644 --- a/src/common/dwarf_cu_to_module.cc +++ b/src/common/dwarf_cu_to_module.cc @@ -696,11 +696,12 @@ void DwarfCUToModule::InlineHandler::Finish() { // Every DW_TAG_inlined_subroutine should have a DW_AT_abstract_origin. assert(specification_offset_ != 0); - cu_context_->file_context->module_->inline_origin_map.SetReference( - specification_offset_, specification_offset_); + Module::InlineOriginMap& inline_origin_map = + cu_context_->file_context->module_ + ->inline_origin_maps[cu_context_->file_context->filename_]; + inline_origin_map.SetReference(specification_offset_, specification_offset_); Module::InlineOrigin* origin = - cu_context_->file_context->module_->inline_origin_map - .GetOrCreateInlineOrigin(specification_offset_, name_); + inline_origin_map.GetOrCreateInlineOrigin(specification_offset_, name_); unique_ptr in( new Module::Inline(origin, ranges, call_site_line_, call_site_file_id_, inline_nest_level_, std::move(child_inlines_))); @@ -929,10 +930,11 @@ void DwarfCUToModule::FuncHandler::Finish() { StringView name = name_.empty() ? name_omitted : name_; uint64_t offset = specification_offset_ != 0 ? specification_offset_ : offset_; - cu_context_->file_context->module_->inline_origin_map.SetReference(offset_, - offset); - cu_context_->file_context->module_->inline_origin_map - .GetOrCreateInlineOrigin(offset_, name); + Module::InlineOriginMap& inline_origin_map = + cu_context_->file_context->module_ + ->inline_origin_maps[cu_context_->file_context->filename_]; + inline_origin_map.SetReference(offset_, offset); + inline_origin_map.GetOrCreateInlineOrigin(offset_, name); } } diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc index 8179663b..0bfc74c3 100644 --- a/src/common/linux/dump_symbols.cc +++ b/src/common/linux/dump_symbols.cc @@ -334,6 +334,40 @@ std::pair UncompressSectionContents( : std::make_pair(uncompressed_buffer.release(), uncompressed_size); } +void StartProcessSplitDwarf(google_breakpad::CompilationUnit* reader, + Module* module, + google_breakpad::Endianness endianness, + bool handle_inter_cu_refs, + bool handle_inline) { + std::string split_file; + google_breakpad::SectionMap split_sections; + google_breakpad::ByteReader split_byte_reader(endianness); + uint64_t cu_offset = 0; + if (!reader->ProcessSplitDwarf(split_file, split_sections, split_byte_reader, + cu_offset)) + return; + DwarfCUToModule::FileContext file_context(split_file, module, + handle_inter_cu_refs); + DumperRangesHandler ranges_handler(&split_byte_reader); + DumperLineToModule line_to_module(&split_byte_reader); + DwarfCUToModule::WarningReporter reporter(split_file, cu_offset); + DwarfCUToModule root_handler(&file_context, &line_to_module, &ranges_handler, + &reporter, handle_inline); + google_breakpad::DIEDispatcher die_dispatcher(&root_handler); + google_breakpad::CompilationUnit split_reader(split_file, split_sections, + cu_offset, &split_byte_reader, + &die_dispatcher); + split_reader.SetSplitDwarf(reader->GetAddrBuffer(), + reader->GetAddrBufferLen(), reader->GetAddrBase(), + reader->GetRangeBase(), reader->GetDWOID()); + split_reader.Start(); + // Normally, it won't happen unless we have transitive reference. + if (split_reader.ShouldProcessSplitDwarf()) { + StartProcessSplitDwarf(&split_reader, module, endianness, + handle_inter_cu_refs, handle_inline); + } +} + template bool LoadDwarf(const string& dwarf_filename, const typename ElfClass::Ehdr* elf_header, @@ -421,6 +455,11 @@ bool LoadDwarf(const string& dwarf_filename, &die_dispatcher); // Process the entire compilation unit; get the offset of the next. offset += reader.Start(); + // Start to process split dwarf file. + if (reader.ShouldProcessSplitDwarf()) { + StartProcessSplitDwarf(&reader, module, endianness, handle_inter_cu_refs, + handle_inline); + } } return true; } diff --git a/src/common/mac/dump_syms.cc b/src/common/mac/dump_syms.cc index dd91196a..d7c3e695 100644 --- a/src/common/mac/dump_syms.cc +++ b/src/common/mac/dump_syms.cc @@ -424,14 +424,49 @@ bool DumpSymbols::CreateEmptyModule(scoped_ptr& module) { return true; } +void DumpSymbols::StartProcessSplitDwarf( + google_breakpad::CompilationUnit* reader, + Module* module, + google_breakpad::Endianness endianness, + bool handle_inter_cu_refs, + bool handle_inline) const { + std::string split_file; + google_breakpad::SectionMap split_sections; + google_breakpad::ByteReader split_byte_reader(endianness); + uint64_t cu_offset = 0; + if (reader->ProcessSplitDwarf(split_file, split_sections, split_byte_reader, + cu_offset)) + return; + DwarfCUToModule::FileContext file_context(split_file, module, + handle_inter_cu_refs); + DumperRangesHandler ranges_handler(&split_byte_reader); + DumperLineToModule line_to_module(&split_byte_reader); + DwarfCUToModule::WarningReporter reporter(split_file, cu_offset); + DwarfCUToModule root_handler(&file_context, &line_to_module, &ranges_handler, + &reporter, handle_inline); + google_breakpad::DIEDispatcher die_dispatcher(&root_handler); + google_breakpad::CompilationUnit split_reader(split_file, split_sections, + cu_offset, &split_byte_reader, + &die_dispatcher); + split_reader.SetSplitDwarf(reader->GetAddrBuffer(), + reader->GetAddrBufferLen(), reader->GetAddrBase(), + reader->GetRangeBase(), reader->GetDWOID()); + split_reader.Start(); + // Normally, it won't happen unless we have transitive reference. + if (split_reader.ShouldProcessSplitDwarf()) { + StartProcessSplitDwarf(&split_reader, module, endianness, + handle_inter_cu_refs, handle_inline); + } +} + void DumpSymbols::ReadDwarf(google_breakpad::Module* module, const mach_o::Reader& macho_reader, const mach_o::SectionMap& dwarf_sections, bool handle_inter_cu_refs) const { // Build a byte reader of the appropriate endianness. - ByteReader byte_reader(macho_reader.big_endian() - ? ENDIANNESS_BIG - : ENDIANNESS_LITTLE); + google_breakpad::Endianness endianness = + macho_reader.big_endian() ? ENDIANNESS_BIG : ENDIANNESS_LITTLE; + ByteReader byte_reader(endianness); // Construct a context for this file. DwarfCUToModule::FileContext file_context(selected_object_name_, @@ -467,14 +502,14 @@ void DumpSymbols::ReadDwarf(google_breakpad::Module* module, // Walk the __debug_info section, one compilation unit at a time. uint64_t debug_info_length = debug_info_section.second; + bool handle_inline = symbol_data_ & INLINES; for (uint64_t offset = 0; offset < debug_info_length;) { // Make a handler for the root DIE that populates MODULE with the // debug info. DwarfCUToModule::WarningReporter reporter(selected_object_name_, offset); DwarfCUToModule root_handler(&file_context, &line_to_module, - &ranges_handler, &reporter, - symbol_data_ & INLINES); + &ranges_handler, &reporter, handle_inline); // Make a Dwarf2Handler that drives our DIEHandler. DIEDispatcher die_dispatcher(&root_handler); // Make a DWARF parser for the compilation unit at OFFSET. @@ -485,6 +520,11 @@ void DumpSymbols::ReadDwarf(google_breakpad::Module* module, &die_dispatcher); // Process the entire compilation unit; get the offset of the next. offset += dwarf_reader.Start(); + // Start to process split dwarf file. + if (dwarf_reader.ShouldProcessSplitDwarf()) { + StartProcessSplitDwarf(&dwarf_reader, module, endianness, + handle_inter_cu_refs, handle_inline); + } } } diff --git a/src/common/mac/dump_syms.h b/src/common/mac/dump_syms.h index 5bcb0b58..5ccf49e3 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/dwarf/dwarf2reader.h" #include "common/mac/arch_utilities.h" #include "common/mac/macho_reader.h" #include "common/mac/super_fat_arch.h" @@ -143,6 +144,13 @@ class DumpSymbols { // Creates an empty module object. bool CreateEmptyModule(scoped_ptr& module); + // Process the split dwarf file referenced by reader. + void StartProcessSplitDwarf(google_breakpad::CompilationUnit* reader, + Module* module, + google_breakpad::Endianness endianness, + bool handle_inter_cu_refs, + bool handle_inline) const; + // Read debugging information from |dwarf_sections|, which was taken from // |macho_reader|, and add it to |module|. void ReadDwarf(google_breakpad::Module* module, diff --git a/src/common/module.cc b/src/common/module.cc index 73c4a8b1..0eb5aad8 100644 --- a/src/common/module.cc +++ b/src/common/module.cc @@ -289,8 +289,7 @@ void Module::GetStackFrameEntries(vector* vec) const { } } -void Module::AssignSourceIds( - set& inline_origins) { +void Module::AssignSourceIds() { // First, give every source file an id of -1. for (FileByNameMap::iterator file_it = files_.begin(); file_it != files_.end(); ++file_it) { @@ -394,7 +393,7 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) { // Get all referenced inline origins. set inline_origins; CreateInlineOrigins(inline_origins); - AssignSourceIds(inline_origins); + AssignSourceIds(); // Write out files. for (FileByNameMap::iterator file_it = files_.begin(); diff --git a/src/common/module.h b/src/common/module.h index d736701f..28e8e9c5 100644 --- a/src/common/module.h +++ b/src/common/module.h @@ -146,10 +146,6 @@ class Module { // The inlined function's name. StringView name; - - File* file; - - int getFileID() const { return file ? file->source_id : -1; } }; // A inlined call site. @@ -228,7 +224,7 @@ class Module { map references_; }; - InlineOriginMap inline_origin_map; + map inline_origin_maps; // A source line. struct Line { @@ -407,7 +403,7 @@ class Module { // Set the source id numbers for all other files --- unused by the // source line data --- to -1. We do this before writing out the // symbol file, at which point we omit any unused files. - void AssignSourceIds(set& inline_origins); + void AssignSourceIds(); // This function should be called before AssignSourceIds() to get the set of // valid InlineOrigins*. diff --git a/src/common/module_unittest.cc b/src/common/module_unittest.cc index 6a6762e5..c51162e5 100644 --- a/src/common/module_unittest.cc +++ b/src/common/module_unittest.cc @@ -198,9 +198,7 @@ TEST(Module, WriteOmitUnusedFiles) { function->lines.push_back(line1); function->lines.push_back(line2); m.AddFunction(function); - - std::set inline_origins; - m.AssignSourceIds(inline_origins); + m.AssignSourceIds(); vector vec; m.GetFiles(&vec);