From f6669d6df42686aea832762f701359a3f9bdc762 Mon Sep 17 00:00:00 2001 From: Sterling Augustine Date: Fri, 2 Oct 2020 07:44:24 -0700 Subject: Revert "Refactor rangelist handling to prepare for dwarf5 .debug_rngslist" This reverts commit 2b936b06c12657b684f6c7276d6ae5a24cb48ab5. After getting deep into the dwarf5 range reader, I realized that this should be done a somewhat different way. So reverting in favor or a better design, coming in a few minutes. Change-Id: Ie0b2846e70b3df1e637831e96ea69fe093f4e712 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2446011 Reviewed-by: Mark Mentovai --- src/common/dwarf/dwarf2reader.cc | 13 ++++++------- src/common/dwarf/dwarf2reader.h | 6 ++++-- src/common/dwarf_cu_to_module.cc | 15 +-------------- src/common/dwarf_cu_to_module.h | 25 +++++-------------------- src/common/linux/dump_symbols.cc | 24 ++++++++++++++++-------- src/common/mac/dump_syms.cc | 24 ++++++++++++++++-------- 6 files changed, 48 insertions(+), 59 deletions(-) (limited to 'src') diff --git a/src/common/dwarf/dwarf2reader.cc b/src/common/dwarf/dwarf2reader.cc index ab31a980..aca83677 100644 --- a/src/common/dwarf/dwarf2reader.cc +++ b/src/common/dwarf/dwarf2reader.cc @@ -1569,11 +1569,10 @@ void LineInfo::ReadLines() { } RangeListReader::RangeListReader(const uint8_t* buffer, uint64_t size, - ByteReader* reader) - : buffer_(buffer), size_(size), reader_(reader) { } + ByteReader* reader, RangeListHandler* handler) + : buffer_(buffer), size_(size), reader_(reader), handler_(handler) { } -bool RangeListReader::ReadRangeList(uint64_t offset, - RangeListHandler* handler) { +bool RangeListReader::ReadRangeList(uint64_t offset) { const uint64_t max_address = (reader_->AddressSize() == 4) ? 0xffffffffUL : 0xffffffffffffffffULL; @@ -1590,12 +1589,12 @@ bool RangeListReader::ReadRangeList(uint64_t offset, reader_->ReadAddress(buffer_ + offset + reader_->AddressSize()); if (start_address == max_address) { // Base address selection - handler->SetBaseAddress(end_address); + handler_->SetBaseAddress(end_address); } else if (start_address == 0 && end_address == 0) { // End-of-list - handler->Finish(); + handler_->Finish(); list_end = true; } else { // Add a range entry - handler->AddRange(start_address, end_address); + handler_->AddRange(start_address, end_address); } offset += entry_size; diff --git a/src/common/dwarf/dwarf2reader.h b/src/common/dwarf/dwarf2reader.h index aa9e270d..e405e3a7 100644 --- a/src/common/dwarf/dwarf2reader.h +++ b/src/common/dwarf/dwarf2reader.h @@ -243,14 +243,16 @@ class RangeListHandler { class RangeListReader { public: - RangeListReader(const uint8_t* buffer, uint64_t size, ByteReader* reader); + RangeListReader(const uint8_t* buffer, uint64_t size, ByteReader* reader, + RangeListHandler* handler); - bool ReadRangeList(uint64_t offset, RangeListHandler* handler); + bool ReadRangeList(uint64_t offset); private: const uint8_t* buffer_; uint64_t size_; ByteReader* reader_; + RangeListHandler* handler_; }; // This class is the main interface between the reader and the diff --git a/src/common/dwarf_cu_to_module.cc b/src/common/dwarf_cu_to_module.cc index 589f8bb2..a5bc7d6c 100644 --- a/src/common/dwarf_cu_to_module.cc +++ b/src/common/dwarf_cu_to_module.cc @@ -134,7 +134,6 @@ DwarfCUToModule::FileContext::FileContext(const string& filename, : filename_(filename), module_(module), handle_inter_cu_refs_(handle_inter_cu_refs), - range_list_reader_(nullptr, 0, nullptr), file_private_(new FilePrivate()) { } @@ -194,7 +193,7 @@ struct DwarfCUToModule::CUContext { // For printing error messages. WarningReporter* reporter; - // For handling ranges, however they may be specified. + // For reading ranges from the .debug_ranges section RangesHandler* ranges_handler; // The source language of this compilation unit. @@ -207,9 +206,6 @@ struct DwarfCUToModule::CUContext { uint64_t high_pc; uint64_t ranges; - // For reading dwarf4 ranges. - scoped_ptr range_list_reader_; - // The functions defined in this compilation unit. We accumulate // them here during parsing. Then, in DwarfCUToModule::Finish, we // assign them lines and add them to file_context->module. @@ -516,15 +512,6 @@ void DwarfCUToModule::FuncHandler::ProcessAttributeUnsigned( break; case dwarf2reader::DW_AT_ranges: ranges_ = data; - if (cu_context_->ranges_handler) { - cu_context_->ranges_handler->SetRangesReader( - &cu_context_->file_context->range_list_reader_); - } else { - cu_context_->reporter->MissingRanges(); - // The rest of the code will fall back to low-pc, which is better than - // nothing. - ranges_ = 0; - } break; default: diff --git a/src/common/dwarf_cu_to_module.h b/src/common/dwarf_cu_to_module.h index e71c3e77..3e15b667 100644 --- a/src/common/dwarf_cu_to_module.h +++ b/src/common/dwarf_cu_to_module.h @@ -89,11 +89,6 @@ class DwarfCUToModule: public dwarf2reader::RootDIEHandler { const uint8_t* contents, uint64_t length); - void SetDebugRangeInfo(const uint8_t* contents, uint64_t size, - dwarf2reader::ByteReader* reader) { - range_list_reader_ = dwarf2reader::RangeListReader(contents, size, reader); - } - // Clear the section map for testing. void ClearSectionMapForTest(); @@ -124,9 +119,6 @@ class DwarfCUToModule: public dwarf2reader::RootDIEHandler { // True if we are handling references between compilation units. const bool handle_inter_cu_refs_; - // Reader for .debug_ranges section, which is global to the file. - dwarf2reader::RangeListReader range_list_reader_; - // Inter-compilation unit data used internally by the handlers. scoped_ptr file_private_; }; @@ -135,23 +127,16 @@ class DwarfCUToModule: public dwarf2reader::RootDIEHandler { // DwarfCUToModule. class RangesHandler { public: - RangesHandler() : reader_(nullptr) { } + RangesHandler() { } virtual ~RangesHandler() { } // Called when finishing a function to populate the function's ranges. - // base_address holds the base PC the range list values are offsets - // off. Return false if the rangelist falls out of the relevant section. + // The ranges' entries are read starting from offset in the .debug_ranges + // section, base_address holds the base PC the range list values are + // offsets off. Return false if the rangelist falls out of the + // .debug_ranges section. virtual bool ReadRanges(uint64_t offset, Module::Address base_address, vector* ranges) = 0; - - // Read ranges from this buffer and interpret them according to attr. Called - // upon seeing a DW_AT_ranges or DW_AT_rngslist attribute. - void SetRangesReader(dwarf2reader::RangeListReader* reader) { - reader_ = reader; - } - - protected: - dwarf2reader::RangeListReader* reader_; }; // An abstract base class for handlers that handle DWARF line data diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc index ac2559cd..8ecf0bc4 100644 --- a/src/common/linux/dump_symbols.cc +++ b/src/common/linux/dump_symbols.cc @@ -232,14 +232,23 @@ bool LoadStabs(const typename ElfClass::Ehdr* elf_header, // owned by a function) with the results. class DumperRangesHandler : public DwarfCUToModule::RangesHandler { public: - DumperRangesHandler() { } + DumperRangesHandler(const uint8_t* buffer, uint64_t size, + dwarf2reader::ByteReader* reader) + : buffer_(buffer), size_(size), reader_(reader) { } bool ReadRanges(uint64_t offset, Module::Address base_address, vector* ranges) { DwarfRangeListHandler handler(base_address, ranges); + dwarf2reader::RangeListReader rangelist_reader(buffer_, size_, reader_, + &handler); - return reader_->ReadRangeList(offset, &handler); + return rangelist_reader.ReadRangeList(offset); } + + private: + const uint8_t* buffer_; + uint64_t size_; + dwarf2reader::ByteReader* reader_; }; // A line-to-module loader that accepts line number info parsed by @@ -305,18 +314,17 @@ bool LoadDwarf(const string& dwarf_filename, } // Optional .debug_ranges reader + scoped_ptr ranges_handler; dwarf2reader::SectionMap::const_iterator ranges_entry = file_context.section_map().find(".debug_ranges"); if (ranges_entry != file_context.section_map().end()) { const std::pair& ranges_section = ranges_entry->second; - file_context.SetDebugRangeInfo(ranges_section.first, - ranges_section.second, - &byte_reader); + ranges_handler.reset( + new DumperRangesHandler(ranges_section.first, ranges_section.second, + &byte_reader)); } - DumperRangesHandler ranges_handler; - // Parse all the compilation units in the .debug_info section. DumperLineToModule line_to_module(&byte_reader); dwarf2reader::SectionMap::const_iterator debug_info_entry = @@ -333,7 +341,7 @@ bool LoadDwarf(const string& dwarf_filename, // data that was found. DwarfCUToModule::WarningReporter reporter(dwarf_filename, offset); DwarfCUToModule root_handler(&file_context, &line_to_module, - &ranges_handler, &reporter); + ranges_handler.get(), &reporter); // Make a Dwarf2Handler that drives the DIEHandler. dwarf2reader::DIEDispatcher die_dispatcher(&root_handler); // Make a DWARF parser for the compilation unit at OFFSET. diff --git a/src/common/mac/dump_syms.cc b/src/common/mac/dump_syms.cc index 9ee6e006..24bcd653 100644 --- a/src/common/mac/dump_syms.cc +++ b/src/common/mac/dump_syms.cc @@ -311,14 +311,23 @@ string DumpSymbols::Identifier() { class DumpSymbols::DumperRangesHandler: public DwarfCUToModule::RangesHandler { public: - DumperRangesHandler() { } + DumperRangesHandler(const uint8_t* buffer, uint64_t size, + dwarf2reader::ByteReader* reader) + : buffer_(buffer), size_(size), reader_(reader) { } bool ReadRanges(uint64_t offset, Module::Address base_address, vector* ranges) { DwarfRangeListHandler handler(base_address, ranges); + dwarf2reader::RangeListReader rangelist_reader(buffer_, size_, reader_, + &handler); - return reader_->ReadRangeList(offset, &handler); + return rangelist_reader.ReadRangeList(offset); } + + private: + const uint8_t* buffer_; + uint64_t size_; + dwarf2reader::ByteReader* reader_; }; // A line-to-module loader that accepts line number info parsed by @@ -448,18 +457,17 @@ void DumpSymbols::ReadDwarf(google_breakpad::Module* module, DumperLineToModule line_to_module(&byte_reader); // Optional .debug_ranges reader + scoped_ptr ranges_handler; dwarf2reader::SectionMap::const_iterator ranges_entry = file_context.section_map().find("__debug_ranges"); if (ranges_entry != file_context.section_map().end()) { const std::pair& ranges_section = ranges_entry->second; - file_context.SetDebugRangeInfo(ranges_section.first, - ranges_section.second, - &byte_reader); + ranges_handler.reset( + new DumperRangesHandler(ranges_section.first, ranges_section.second, + &byte_reader)); } - DumperRangesHandler ranges_handler; - // Walk the __debug_info section, one compilation unit at a time. uint64_t debug_info_length = debug_info_section.second; for (uint64_t offset = 0; offset < debug_info_length;) { @@ -468,7 +476,7 @@ void DumpSymbols::ReadDwarf(google_breakpad::Module* module, DwarfCUToModule::WarningReporter reporter(selected_object_name_, offset); DwarfCUToModule root_handler(&file_context, &line_to_module, - &ranges_handler, &reporter); + ranges_handler.get(), &reporter); // Make a Dwarf2Handler that drives our DIEHandler. dwarf2reader::DIEDispatcher die_dispatcher(&root_handler); // Make a DWARF parser for the compilation unit at OFFSET. -- cgit v1.2.1