From 3846f6d297339c17663d7a797ba481b3411f13ad Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Wed, 27 Oct 2021 13:40:35 -0700 Subject: [PATCH] Add to INLINE and remove from INLINE_ORIGIN This is the dump_syms side change on https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3232838/. This fixes incorrect source file names when a inlined function's source file name is different from its parent's. Change-Id: I25683912d206c6a8db44e322eca5f7383ea8c47e Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3248438 Reviewed-by: Joshua Peraza --- src/common/dwarf_cu_to_module.cc | 46 +++++++++++++++----------------- src/common/module.cc | 43 ++++++++++------------------- src/common/module.h | 35 ++++++++++++++++++------ 3 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/common/dwarf_cu_to_module.cc b/src/common/dwarf_cu_to_module.cc index 4bd71564..3435e5b1 100644 --- a/src/common/dwarf_cu_to_module.cc +++ b/src/common/dwarf_cu_to_module.cc @@ -254,9 +254,6 @@ struct DwarfCUToModule::CUContext { // A map of function pointers to the its forward specification DIE's offset. map spec_function_offsets; - - // From file index to vector of subprogram's offset in this CU. - map> inline_origins; }; // Information about the context of a particular DIE. This is for @@ -561,7 +558,8 @@ class DwarfCUToModule::InlineHandler : public GenericDIEHandler { DwarfForm high_pc_form_; // DW_AT_high_pc can be length or address. DwarfForm ranges_form_; // DW_FORM_sec_offset or DW_FORM_rnglistx uint64_t ranges_data_; // DW_AT_ranges - int call_site_line_; + int call_site_line_; // DW_AT_call_line + int call_site_file_id_; // DW_AT_call_file int inline_nest_level_; // A vector of inlines in the same nest level. It's owned by its parent // function/inline. At Finish(), add this inline into the vector. @@ -589,6 +587,9 @@ void DwarfCUToModule::InlineHandler::ProcessAttributeUnsigned( case DW_AT_call_line: call_site_line_ = data; break; + case DW_AT_call_file: + call_site_file_id_ = data; + break; default: GenericDIEHandler::ProcessAttributeUnsigned(attr, form, data); break; @@ -666,8 +667,8 @@ void DwarfCUToModule::InlineHandler::Finish() { cu_context_->file_context->module_->inline_origin_map .GetOrCreateInlineOrigin(specification_offset_, name_); unique_ptr in( - new Module::Inline(origin, ranges, call_site_line_, inline_nest_level_, - std::move(child_inlines_))); + new Module::Inline(origin, ranges, call_site_line_, call_site_file_id_, + inline_nest_level_, std::move(child_inlines_))); inlines_.push_back(std::move(in)); } @@ -684,7 +685,6 @@ class DwarfCUToModule::FuncHandler: public GenericDIEHandler { high_pc_form_(DW_FORM_addr), ranges_form_(DW_FORM_sec_offset), ranges_data_(0), - decl_file_data_(UINT64_MAX), inline_(false), handle_inline_(handle_inline) {} @@ -705,9 +705,7 @@ class DwarfCUToModule::FuncHandler: public GenericDIEHandler { uint64_t low_pc_, high_pc_; // DW_AT_low_pc, DW_AT_high_pc DwarfForm high_pc_form_; // DW_AT_high_pc can be length or address. DwarfForm ranges_form_; // DW_FORM_sec_offset or DW_FORM_rnglistx - uint64_t ranges_data_; // DW_AT_ranges - // DW_AT_decl_file, value of UINT64_MAX means undefined. - uint64_t decl_file_data_; + uint64_t ranges_data_; // DW_AT_ranges bool inline_; vector> child_inlines_; bool handle_inline_; @@ -732,9 +730,6 @@ void DwarfCUToModule::FuncHandler::ProcessAttributeUnsigned( ranges_data_ = data; ranges_form_ = form; break; - case DW_AT_decl_file: - decl_file_data_ = data; - break; default: GenericDIEHandler::ProcessAttributeUnsigned(attr, form, data); break; @@ -862,8 +857,7 @@ void DwarfCUToModule::FuncHandler::Finish() { // Only keep track of DW_TAG_subprogram which have the attributes we are // interested. - if (handle_inline_ && - (!empty_range || inline_ || decl_file_data_ != UINT64_MAX)) { + if (handle_inline_ && (!empty_range || inline_)) { StringView name = name_.empty() ? name_omitted : name_; uint64_t offset = specification_offset_ != 0 ? specification_offset_ : offset_; @@ -871,8 +865,6 @@ void DwarfCUToModule::FuncHandler::Finish() { offset); cu_context_->file_context->module_->inline_origin_map .GetOrCreateInlineOrigin(offset_, name); - if (decl_file_data_ != UINT64_MAX) - cu_context_->inline_origins[decl_file_data_].push_back(offset_); } } @@ -880,10 +872,12 @@ void DwarfCUToModule::FuncHandler::Finish() { // component to their names: namespaces, classes, etc. class DwarfCUToModule::NamedScopeHandler: public GenericDIEHandler { public: - NamedScopeHandler(CUContext* cu_context, DIEContext* parent_context, - uint64_t offset, bool handle_inline) - : GenericDIEHandler(cu_context, parent_context, offset), - handle_inline_(handle_inline) { } + NamedScopeHandler(CUContext* cu_context, + DIEContext* parent_context, + uint64_t offset, + bool handle_inline) + : GenericDIEHandler(cu_context, parent_context, offset), + handle_inline_(handle_inline) {} bool EndAttributes(); DIEHandler* FindChildHandler(uint64_t offset, enum DwarfTag tag); @@ -1455,10 +1449,12 @@ void DwarfCUToModule::AssignLinesToFunctions() { } void DwarfCUToModule::AssignFilesToInlines() { - for (auto iter : files_) { - cu_context_->file_context->module_->inline_origin_map - .AssignFilesToInlineOrigins(cu_context_->inline_origins[iter.first], - iter.second); + // Assign File* to Inlines inside this CU. + auto assignFile = [this](unique_ptr& in) { + in->call_site_file = files_[in->call_site_file_id]; + }; + for (auto func : cu_context_->functions) { + Module::Inline::InlineDFS(func->inlines, assignFile); } } diff --git a/src/common/module.cc b/src/common/module.cc index 3945e2dd..13c229eb 100644 --- a/src/common/module.cc +++ b/src/common/module.cc @@ -97,17 +97,6 @@ void Module::InlineOriginMap::SetReference(uint64_t offset, references_[offset] = specification_offset; } -void Module::InlineOriginMap::AssignFilesToInlineOrigins( - const vector& inline_origin_offsets, - Module::File* file) { - for (uint64_t offset : inline_origin_offsets) - if (references_.find(offset) != references_.end()) { - auto origin = inline_origins_.find(references_[offset]); - if (origin != inline_origins_.end()) - origin->second->file = file; - } -} - Module::Module(const string& name, const string& os, const string& architecture, const string& id, const string& code_id /* = "" */) : @@ -276,13 +265,19 @@ void Module::AssignSourceIds( line_it != func->lines.end(); ++line_it) line_it->file->source_id = 0; } - // Also mark all files cited by inline functions by setting each one's source + + // Also mark all files cited by inline callsite by setting each one's source // id to zero. - for (InlineOrigin* origin : inline_origins) + auto markInlineFiles = [](unique_ptr& in) { // There are some artificial inline functions which don't belong to // any file. Those will have file id -1. - if (origin->file) - origin->file->source_id = 0; + if (in->call_site_file) { + in->call_site_file->source_id = 0; + } + }; + for (auto func : functions_) { + Inline::InlineDFS(func->inlines, markInlineFiles); + } // Finally, assign source ids to those files that have been marked. // We could have just assigned source id numbers while traversing @@ -296,15 +291,6 @@ void Module::AssignSourceIds( } } -static void InlineDFS( - vector>& inlines, - std::function&)> const& forEach) { - for (unique_ptr& in : inlines) { - forEach(in); - InlineDFS(in->child_inlines, forEach); - } -} - void Module::CreateInlineOrigins( set& inline_origins) { // Only add origins that have file and deduplicate origins with same name and @@ -317,7 +303,7 @@ void Module::CreateInlineOrigins( in->origin = *it; }; for (Function* func : functions_) - InlineDFS(func->inlines, addInlineOrigins); + Module::Inline::InlineDFS(func->inlines, addInlineOrigins); int next_id = 0; for (InlineOrigin* origin : inline_origins) { origin->id = next_id++; @@ -381,8 +367,7 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) { } // Write out inline origins. for (InlineOrigin* origin : inline_origins) { - stream << "INLINE_ORIGIN " << origin->id << " " << origin->getFileID() - << " " << origin->name << "\n"; + stream << "INLINE_ORIGIN " << origin->id << " " << origin->name << "\n"; if (!stream.good()) return ReportError(); } @@ -407,12 +392,12 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) { auto write_inline = [&](unique_ptr& in) { stream << "INLINE "; stream << in->inline_nest_level << " " << in->call_site_line << " " - << in->origin->id << hex; + << in->getCallSiteFileID() << " " << in->origin->id << hex; for (const Range& r : in->ranges) stream << " " << (r.address - load_address_) << " " << r.size; stream << dec << "\n"; }; - InlineDFS(func->inlines, write_inline); + Module::Inline::InlineDFS(func->inlines, write_inline); if (!stream.good()) return ReportError(); diff --git a/src/common/module.h b/src/common/module.h index c5e0abfc..01ecfa8a 100644 --- a/src/common/module.h +++ b/src/common/module.h @@ -38,6 +38,7 @@ #ifndef COMMON_LINUX_MODULE_H__ #define COMMON_LINUX_MODULE_H__ +#include #include #include #include @@ -131,7 +132,7 @@ class Module { }; struct InlineOrigin { - explicit InlineOrigin(StringView name): id(-1), name(name), file(nullptr) {} + explicit InlineOrigin(StringView name) : id(-1), name(name) {} // A unique id for each InlineOrigin object. INLINE records use the id to // refer to its INLINE_ORIGIN record. @@ -150,11 +151,14 @@ class Module { Inline(InlineOrigin* origin, const vector& ranges, int call_site_line, + int call_site_file_id, int inline_nest_level, vector> child_inlines) : origin(origin), ranges(ranges), call_site_line(call_site_line), + call_site_file_id(call_site_file_id), + call_site_file(nullptr), inline_nest_level(inline_nest_level), child_inlines(std::move(child_inlines)) {} @@ -165,10 +169,29 @@ class Module { int call_site_line; + // The id is only meanful inside a CU. It's only used for looking up real + // File* after scanning a CU. + int call_site_file_id; + + File* call_site_file; + int inline_nest_level; // A list of inlines which are children of this inline. vector> child_inlines; + + int getCallSiteFileID() const { + return call_site_file ? call_site_file->source_id : -1; + } + + static void InlineDFS( + vector>& inlines, + std::function&)> const& forEach) { + for (std::unique_ptr& in : inlines) { + forEach(in); + InlineDFS(in->child_inlines, forEach); + } + } }; typedef map InlineOriginByOffset; @@ -182,9 +205,7 @@ class Module { // value of its DW_AT_specification or equals to offset if // DW_AT_specification doesn't exist in that DIE. void SetReference(uint64_t offset, uint64_t specification_offset); - void AssignFilesToInlineOrigins( - const vector& inline_origin_offsets, - File* file); + ~InlineOriginMap() { for (const auto& iter : inline_origins_) { delete iter.second; @@ -261,10 +282,8 @@ class Module { }; struct InlineOriginCompare { - bool operator() (const InlineOrigin* lhs, const InlineOrigin* rhs) const { - if (lhs->getFileID() == rhs->getFileID()) - return lhs->name < rhs->name; - return lhs->getFileID() < rhs->getFileID(); + bool operator()(const InlineOrigin* lhs, const InlineOrigin* rhs) const { + return lhs->name < rhs->name; } };