diff --git a/src/common/dwarf_cu_to_module.cc b/src/common/dwarf_cu_to_module.cc index 04d19479..82131e7b 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; @@ -661,8 +662,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)); } @@ -679,7 +680,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) {} @@ -700,9 +700,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_; @@ -727,9 +725,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; @@ -857,8 +852,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_; @@ -866,8 +860,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_); } } @@ -1450,10 +1442,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..381dd5d5 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,18 @@ 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 +290,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 +302,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 +366,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 +391,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..bfd344dd 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. @@ -139,10 +140,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. @@ -150,11 +147,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 +165,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 +201,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 +278,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; } }; diff --git a/src/google_breakpad/processor/basic_source_line_resolver.h b/src/google_breakpad/processor/basic_source_line_resolver.h index fe76ad4d..e9459c77 100644 --- a/src/google_breakpad/processor/basic_source_line_resolver.h +++ b/src/google_breakpad/processor/basic_source_line_resolver.h @@ -98,28 +98,28 @@ class SymbolParseHelper { char** filename); // out // Parses a |inline_origin_line| declaration. Returns true on success. - // Format: INLINE_ORIGIN . + // Format: INLINE_ORIGIN . // Notice, that this method modifies the input |inline_origin_line| which is - // why it can't be const. On success, , and are - // stored in |*origin_id|, |*file_id|, and |*name|. No allocation is + // why it can't be const. On success, and are + // stored in |*origin_id| and |*name|. No allocation is // done, |*name| simply points inside |inline_origin_line|. static bool ParseInlineOrigin(char* inline_origin_line, // in long* origin_id, // out - long* file_id, // out char** name); // out // Parses a |inline| declaration. Returns true on success. - // Format: INLINE
- // .... - // Notice, that this method modifies the input |inline| - // which is why it can't be const. On success, , - // and are stored in |*inline_nest_level|, - // |*call_site_line|, and |*origin_id|, and all pairs of (
, ) - // are added into ranges . + // Format: INLINE + // [
]+ + // Notice, that this method modifies the input + // |inline| which is why it can't be const. On success, , + // , and are stored in + // |*inline_nest_level|, |*call_site_line|, |*call_site_file_id| and + // |*origin_id|, and all pairs of (
, ) are added into ranges. static bool ParseInline( char* inline_line, // in long* inline_nest_level, // out long* call_site_line, // out + long* call_site_file_id, // out long* origin_id, // out std::vector>* ranges); // out diff --git a/src/google_breakpad/processor/source_line_resolver_base.h b/src/google_breakpad/processor/source_line_resolver_base.h index 2d2e4b35..ba68798a 100644 --- a/src/google_breakpad/processor/source_line_resolver_base.h +++ b/src/google_breakpad/processor/source_line_resolver_base.h @@ -41,6 +41,7 @@ #ifndef GOOGLE_BREAKPAD_PROCESSOR_SOURCE_LINE_RESOLVER_BASE_H__ #define GOOGLE_BREAKPAD_PROCESSOR_SOURCE_LINE_RESOLVER_BASE_H__ +#include #include #include #include @@ -86,7 +87,7 @@ class SourceLineResolverBase : public SourceLineResolverInterface { virtual bool IsModuleCorrupt(const CodeModule* module); virtual void FillSourceLineInfo( StackFrame* frame, - std::vector>* inlined_frames); + std::deque>* inlined_frames); virtual WindowsFrameInfo* FindWindowsFrameInfo(const StackFrame* frame); virtual CFIFrameInfo* FindCFIFrameInfo(const StackFrame* frame); diff --git a/src/google_breakpad/processor/source_line_resolver_interface.h b/src/google_breakpad/processor/source_line_resolver_interface.h index 7880c922..2614f65e 100644 --- a/src/google_breakpad/processor/source_line_resolver_interface.h +++ b/src/google_breakpad/processor/source_line_resolver_interface.h @@ -34,6 +34,7 @@ #ifndef GOOGLE_BREAKPAD_PROCESSOR_SOURCE_LINE_RESOLVER_INTERFACE_H__ #define GOOGLE_BREAKPAD_PROCESSOR_SOURCE_LINE_RESOLVER_INTERFACE_H__ +#include #include #include #include @@ -98,7 +99,7 @@ class SourceLineResolverInterface { // inlined_frames in an order from outermost frame to inner most frame. virtual void FillSourceLineInfo( StackFrame* frame, - std::vector>* inlined_frames) = 0; + std::deque>* inlined_frames) = 0; // If Windows stack walking information is available covering // FRAME's instruction address, return a WindowsFrameInfo structure diff --git a/src/google_breakpad/processor/stack_frame_symbolizer.h b/src/google_breakpad/processor/stack_frame_symbolizer.h index 0e2c6088..91ef64b5 100644 --- a/src/google_breakpad/processor/stack_frame_symbolizer.h +++ b/src/google_breakpad/processor/stack_frame_symbolizer.h @@ -35,6 +35,7 @@ #ifndef GOOGLE_BREAKPAD_PROCESSOR_STACK_FRAME_SYMBOLIZER_H__ #define GOOGLE_BREAKPAD_PROCESSOR_STACK_FRAME_SYMBOLIZER_H__ +#include #include #include #include @@ -82,7 +83,7 @@ class StackFrameSymbolizer { const CodeModules* unloaded_modules, const SystemInfo* system_info, StackFrame* stack_frame, - std::vector>* inlined_frames); + std::deque>* inlined_frames); virtual WindowsFrameInfo* FindWindowsFrameInfo(const StackFrame* frame); diff --git a/src/processor/basic_source_line_resolver.cc b/src/processor/basic_source_line_resolver.cc index bbaa1331..0db0d735 100644 --- a/src/processor/basic_source_line_resolver.cc +++ b/src/processor/basic_source_line_resolver.cc @@ -50,10 +50,11 @@ #include "processor/tokenize.h" -using std::map; -using std::vector; +using std::deque; using std::make_pair; +using std::map; using std::unique_ptr; +using std::vector; namespace google_breakpad { @@ -237,42 +238,43 @@ bool BasicSourceLineResolver::Module::LoadMapFromMemory( return true; } -int BasicSourceLineResolver::Module::ConstructInlineFrames( +void BasicSourceLineResolver::Module::ConstructInlineFrames( StackFrame* frame, MemAddr address, const RangeMap>& inlines, - vector>* inlined_frames) const { + deque>* inlined_frames) const { linked_ptr in; MemAddr inline_base; if (!inlines.RetrieveRange(address, &in, &inline_base, nullptr, nullptr)) - return -1; + return; auto origin = inline_origins_.find(in->origin_id); if (origin == inline_origins_.end()) - return -1; + return; - StackFrame new_frame = StackFrame(*frame); - new_frame.function_name = origin->second->name; - // Use the starting adress of the inlined range as inlined function base. - new_frame.function_base = new_frame.module->base_address() + inline_base; - auto it = files_.find(origin->second->source_file_id); - if (it != files_.end()) - new_frame.source_file_name = it->second; - - new_frame.trust = StackFrame::FRAME_TRUST_INLINE; - // Must add frames before calling ConstructInlineFrames to get correct order. - int current_idx = inlined_frames->size(); - inlined_frames->push_back(unique_ptr(new StackFrame(new_frame))); - int source_line = ConstructInlineFrames(&new_frame, address, - in->child_inlines, inlined_frames); - if (source_line != -1) { - (*inlined_frames)[current_idx]->source_line = source_line; + // Update parent frame's source line and source file. + frame->source_line = in->call_site_line; + auto file = files_.find(in->call_site_file_id); + if (file != files_.end()) { + frame->source_file_name = file->second; } - return in->call_site_line; + + // Create a child frame of `frame`. + StackFrame child_frame = StackFrame(*frame); + child_frame.function_name = origin->second->name; + // Use the starting adress of the inlined range as inlined function base. + child_frame.function_base = child_frame.module->base_address() + inline_base; + child_frame.trust = StackFrame::FRAME_TRUST_INLINE; + ConstructInlineFrames(&child_frame, address, in->child_inlines, + inlined_frames); + // Add child_frame after ConstructInlineFrames so that the innermost frame is + // the first frame inside inlined_frames. + inlined_frames->push_back( + unique_ptr(new StackFrame(child_frame))); } void BasicSourceLineResolver::Module::LookupAddress( StackFrame* frame, - vector>* inlined_frames) const { + deque>* inlined_frames) const { MemAddr address = frame->instruction - frame->module->base_address(); // First, look for a FUNC record that covers address. Use @@ -306,10 +308,13 @@ void BasicSourceLineResolver::Module::LookupAddress( // Check if this is inlined function call. if (inlined_frames) { - int source_line = - ConstructInlineFrames(frame, address, func->inlines, inlined_frames); - if (source_line != -1) { - frame->source_line = source_line; + int source_line = frame->source_line; + string source_file_name = frame->source_file_name; + ConstructInlineFrames(frame, address, func->inlines, inlined_frames); + if (!inlined_frames->empty()) { + // Update the inner most frame's source line and source file name. + inlined_frames->front()->source_line = source_line; + inlined_frames->front()->source_file_name = source_file_name; } } } else if (public_symbols_.Retrieve(address, @@ -416,12 +421,10 @@ bool BasicSourceLineResolver::Module::ParseFile(char* file_line) { bool BasicSourceLineResolver::Module::ParseInlineOrigin( char* inline_origin_line) { long origin_id; - long source_file_id; char* origin_name; if (SymbolParseHelper::ParseInlineOrigin(inline_origin_line, &origin_id, - &source_file_id, &origin_name)) { - inline_origins_.insert( - make_pair(origin_id, new InlineOrigin(source_file_id, origin_name))); + &origin_name)) { + inline_origins_.insert(make_pair(origin_id, new InlineOrigin(origin_name))); return true; } return false; @@ -431,12 +434,14 @@ linked_ptr BasicSourceLineResolver::Module::ParseInline(char* inline_line) { long inline_nest_level; long call_site_line; + long call_site_file_id; long origin_id; vector> ranges; if (SymbolParseHelper::ParseInline(inline_line, &inline_nest_level, - &call_site_line, &origin_id, &ranges)) { - return linked_ptr( - new Inline(inline_nest_level, call_site_line, origin_id, ranges)); + &call_site_line, &call_site_file_id, + &origin_id, &ranges)) { + return linked_ptr(new Inline(inline_nest_level, call_site_line, + call_site_file_id, origin_id, ranges)); } return linked_ptr(); } @@ -636,13 +641,12 @@ bool SymbolParseHelper::ParseFile(char* file_line, long* index, // static bool SymbolParseHelper::ParseInlineOrigin(char* inline_origin_line, long* origin_id, - long* file_id, char** name) { - // INLINE_ORIGIN + // INLINE_ORIGIN assert(strncmp(inline_origin_line, "INLINE_ORIGIN ", 14) == 0); inline_origin_line += 14; // skip prefix vector tokens; - if (!Tokenize(inline_origin_line, kWhitespace, 3, &tokens)) { + if (!Tokenize(inline_origin_line, kWhitespace, 2, &tokens)) { return false; } @@ -653,15 +657,7 @@ bool SymbolParseHelper::ParseInlineOrigin(char* inline_origin_line, return false; } - *file_id = strtol(tokens[1], &after_number, 10); - // If the file id is -1, it might be an artificial function that doesn't have - // file id. So, we consider -1 as a valid special case. - if (!IsValidAfterNumber(after_number) || - *file_id < -1 | *origin_id == std::numeric_limits::max()) { - return false; - } - - *name = tokens[2]; + *name = tokens[1]; if (!*name) { return false; } @@ -674,18 +670,19 @@ bool SymbolParseHelper::ParseInline( char* inline_line, long* inline_nest_level, long* call_site_line, + long* call_site_file_id, long* origin_id, vector>* ranges) { - // INLINE
- // ... + // INLINE + // [
]+ assert(strncmp(inline_line, "INLINE ", 7) == 0); inline_line += 7; // skip prefix vector tokens; Tokenize(inline_line, kWhitespace, std::numeric_limits::max(), &tokens); - // The length of the vector should be at least 5 and an odd number. - if (tokens.size() < 5 && tokens.size() % 2 == 0) + // The length of the vector should be at least 6 and an even number. + if (tokens.size() < 6 || tokens.size() % 2 != 0) return false; char* after_number; @@ -701,13 +698,21 @@ bool SymbolParseHelper::ParseInline( return false; } - *origin_id = strtol(tokens[2], &after_number, 10); + *call_site_file_id = strtol(tokens[2], &after_number, 10); + // If the file id is -1, it might be an artificial function that doesn't have + // file id. So, we consider -1 as a valid special case. + if (!IsValidAfterNumber(after_number) || *call_site_file_id < -1 || + *call_site_file_id == std::numeric_limits::max()) { + return false; + } + + *origin_id = strtol(tokens[3], &after_number, 10); if (!IsValidAfterNumber(after_number) || *origin_id < 0 || *origin_id == std::numeric_limits::max()) { return false; } - for (size_t i = 3; i < tokens.size();) { + for (size_t i = 4; i < tokens.size();) { MemAddr address = strtoull(tokens[i++], &after_number, 16); if (!IsValidAfterNumber(after_number) || address == std::numeric_limits::max()) { diff --git a/src/processor/basic_source_line_resolver_types.h b/src/processor/basic_source_line_resolver_types.h index 482176f4..60c06756 100644 --- a/src/processor/basic_source_line_resolver_types.h +++ b/src/processor/basic_source_line_resolver_types.h @@ -37,6 +37,7 @@ #ifndef PROCESSOR_BASIC_SOURCE_LINE_RESOLVER_TYPES_H__ #define PROCESSOR_BASIC_SOURCE_LINE_RESOLVER_TYPES_H__ +#include #include #include @@ -108,15 +109,18 @@ class BasicSourceLineResolver::Module : public SourceLineResolverBase::Module { // with the result. virtual void LookupAddress( StackFrame* frame, - std::vector>* inlined_frame) const; + std::deque>* inlined_frame) const; - // Construct inlined frame for frame and return inlined function call site - // source line. If failed to construct inlined frame, return -1. - virtual int ConstructInlineFrames( + // Construct inlined frames for frame. Return true on success. + // If successfully construct inlined frames for `frame`, `inline_frames` will + // be filled with lined frames and frame->source_file_name and + // frame->source_line will be update to represents the outermost frame. + // If failed, `inline_frames` will be empty and frame remains unchanged. + virtual void ConstructInlineFrames( StackFrame* frame, MemAddr address, const RangeMap>& inlines, - std::vector>* inline_frames) const; + std::deque>* inline_frames) const; // If Windows stack walking information is available covering ADDRESS, // return a WindowsFrameInfo structure describing it. If the information diff --git a/src/processor/basic_source_line_resolver_unittest.cc b/src/processor/basic_source_line_resolver_unittest.cc index 914f0963..8db56217 100644 --- a/src/processor/basic_source_line_resolver_unittest.cc +++ b/src/processor/basic_source_line_resolver_unittest.cc @@ -421,40 +421,40 @@ TEST_F(TestBasicSourceLineResolver, TestLoadAndResolveInlines) { "linux_inline.sym")); ASSERT_TRUE(resolver.HasModule(&module)); StackFrame frame; - std::vector> inlined_frames; + std::deque> inlined_frames; frame.instruction = 0x161b6; frame.module = &module; // main frame. resolver.FillSourceLineInfo(&frame, &inlined_frames); ASSERT_EQ(frame.function_name, "main"); ASSERT_EQ(frame.function_base, 0x15b30U); - ASSERT_EQ(frame.source_file_name, "linux_inline.cpp"); + ASSERT_EQ(frame.source_file_name, "a.cpp"); ASSERT_EQ(frame.source_line, 42); ASSERT_EQ(frame.source_line_base, 0x161b6U); ASSERT_EQ(inlined_frames.size(), 3UL); // Inlined frames inside main frame. - ASSERT_EQ(inlined_frames[0]->function_name, "foo()"); - ASSERT_EQ(inlined_frames[0]->function_base, 0x15b45U); - ASSERT_EQ(inlined_frames[0]->source_file_name, "linux_inline.cpp"); - ASSERT_EQ(inlined_frames[0]->source_line, 39); - ASSERT_EQ(inlined_frames[0]->source_line_base, 0x161b6U); - ASSERT_EQ(inlined_frames[0]->trust, StackFrame::FRAME_TRUST_INLINE); + ASSERT_EQ(inlined_frames[2]->function_name, "foo()"); + ASSERT_EQ(inlined_frames[2]->function_base, 0x15b45U); + ASSERT_EQ(inlined_frames[2]->source_file_name, "b.cpp"); + ASSERT_EQ(inlined_frames[2]->source_line, 39); + ASSERT_EQ(inlined_frames[2]->source_line_base, 0x161b6U); + ASSERT_EQ(inlined_frames[2]->trust, StackFrame::FRAME_TRUST_INLINE); ASSERT_EQ(inlined_frames[1]->function_name, "bar()"); ASSERT_EQ(inlined_frames[1]->function_base, 0x15b72U); - ASSERT_EQ(inlined_frames[1]->source_file_name, "linux_inline.cpp"); + ASSERT_EQ(inlined_frames[1]->source_file_name, "c.cpp"); ASSERT_EQ(inlined_frames[1]->source_line, 32); ASSERT_EQ(inlined_frames[1]->source_line_base, 0x161b6U); ASSERT_EQ(inlined_frames[1]->trust, StackFrame::FRAME_TRUST_INLINE); - ASSERT_EQ(inlined_frames[2]->function_name, "func()"); - ASSERT_EQ(inlined_frames[2]->function_base, 0x15b83U); - ASSERT_EQ(inlined_frames[2]->source_file_name, "linux_inline.cpp"); - ASSERT_EQ(inlined_frames[2]->source_line, 27); - ASSERT_EQ(inlined_frames[2]->source_line_base, 0x161b6U); - ASSERT_EQ(inlined_frames[2]->trust, StackFrame::FRAME_TRUST_INLINE); + ASSERT_EQ(inlined_frames[0]->function_name, "func()"); + ASSERT_EQ(inlined_frames[0]->function_base, 0x15b83U); + ASSERT_EQ(inlined_frames[0]->source_file_name, "linux_inline.cpp"); + ASSERT_EQ(inlined_frames[0]->source_line, 27); + ASSERT_EQ(inlined_frames[0]->source_line_base, 0x161b6U); + ASSERT_EQ(inlined_frames[0]->trust, StackFrame::FRAME_TRUST_INLINE); } // Test parsing of valid FILE lines. The format is: @@ -781,25 +781,15 @@ TEST(SymbolParseHelper, ParsePublicSymbolInvalid) { } // Test parsing of valid INLINE_ORIGIN lines. The format is: -// INLINE_ORIGIN +// INLINE_ORIGIN TEST(SymbolParseHelper, ParseInlineOriginValid) { long origin_id; - long file_id; char* name; - char kTestLine[] = "INLINE_ORIGIN 1 1 function name"; - ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id, - &file_id, &name)); + char kTestLine[] = "INLINE_ORIGIN 1 function name"; + ASSERT_TRUE( + SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id, &name)); EXPECT_EQ(1, origin_id); - EXPECT_EQ(1, file_id); - EXPECT_EQ("function name", string(name)); - - // -1 is a file id, which is used when the function is artifical. - char kTestLine1[] = "INLINE_ORIGIN 0 -1 function name"; - ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin(kTestLine1, &origin_id, - &file_id, &name)); - EXPECT_EQ(0, origin_id); - EXPECT_EQ(-1, file_id); EXPECT_EQ("function name", string(name)); } @@ -807,28 +797,27 @@ TEST(SymbolParseHelper, ParseInlineOriginValid) { // INLINE_ORIGIN TEST(SymbolParseHelper, ParseInlineOriginInvalid) { long origin_id; - long file_id; char* name; // Test missing function name. - char kTestLine[] = "INLINE_ORIGIN 1 1"; - ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id, - &file_id, &name)); + char kTestLine[] = "INLINE_ORIGIN 1"; + ASSERT_FALSE( + SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id, &name)); // Test bad origin id. - char kTestLine1[] = "INLINE_ORIGIN x1 1 function name"; - ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine1, &origin_id, - &file_id, &name)); + char kTestLine1[] = "INLINE_ORIGIN x1 function name"; + ASSERT_FALSE( + SymbolParseHelper::ParseInlineOrigin(kTestLine1, &origin_id, &name)); // Test large origin id. - char kTestLine2[] = "INLINE_ORIGIN 123123123123123123123123 1 function name"; - ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine2, &origin_id, - &file_id, &name)); + char kTestLine2[] = "INLINE_ORIGIN 123123123123123123123123 function name"; + ASSERT_FALSE( + SymbolParseHelper::ParseInlineOrigin(kTestLine2, &origin_id, &name)); // Test negative origin id. - char kTestLine3[] = "INLINE_ORIGIN -1 1 function name"; - ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine3, &origin_id, - &file_id, &name)); + char kTestLine3[] = "INLINE_ORIGIN -1 function name"; + ASSERT_FALSE( + SymbolParseHelper::ParseInlineOrigin(kTestLine3, &origin_id, &name)); } // Test parsing of valid INLINE lines. The format is: @@ -836,26 +825,31 @@ TEST(SymbolParseHelper, ParseInlineOriginInvalid) { TEST(SymbolParseHelper, ParseInlineValid) { long inline_nest_level; long call_site_line; + long call_site_file_id; long origin_id; std::vector> ranges; - char kTestLine[] = "INLINE 0 1 2 3 4"; + char kTestLine[] = "INLINE 0 1 2 3 4 5"; ASSERT_TRUE(SymbolParseHelper::ParseInline( - kTestLine, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine, &inline_nest_level, &call_site_line, &call_site_file_id, + &origin_id, &ranges)); EXPECT_EQ(0, inline_nest_level); EXPECT_EQ(1, call_site_line); - EXPECT_EQ(2, origin_id); - EXPECT_EQ(0x3ULL, ranges[0].first); - EXPECT_EQ(0x4ULL, ranges[0].second); + EXPECT_EQ(2, call_site_file_id); + EXPECT_EQ(3, origin_id); + EXPECT_EQ(0x4ULL, ranges[0].first); + EXPECT_EQ(0x5ULL, ranges[0].second); ranges.clear(); // Test hex and discontinuous ranges. - char kTestLine1[] = "INLINE 0 1 2 a b 1a 1b"; + char kTestLine1[] = "INLINE 0 1 2 3 a b 1a 1b"; ASSERT_TRUE(SymbolParseHelper::ParseInline( - kTestLine1, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine1, &inline_nest_level, &call_site_line, &call_site_file_id, + &origin_id, &ranges)); EXPECT_EQ(0, inline_nest_level); EXPECT_EQ(1, call_site_line); - EXPECT_EQ(2, origin_id); + EXPECT_EQ(2, call_site_file_id); + EXPECT_EQ(3, origin_id); EXPECT_EQ(0xaULL, ranges[0].first); EXPECT_EQ(0xbULL, ranges[0].second); EXPECT_EQ(0x1aULL, ranges[1].first); @@ -867,33 +861,39 @@ TEST(SymbolParseHelper, ParseInlineValid) { TEST(SymbolParseHelper, ParseInlineInvalid) { long inline_nest_level; long call_site_line; + long call_site_file_id; long origin_id; std::vector> ranges; // Test negative inline_nest_level. - char kTestLine[] = "INLINE -1 1 2 3 4"; + char kTestLine[] = "INLINE -1 1 2 3 4 5"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine, &inline_nest_level, &call_site_line, &call_site_file_id, + &origin_id, &ranges)); // Test negative call_site_line. - char kTestLine1[] = "INLINE 0 -1 2 3 4"; + char kTestLine1[] = "INLINE 0 -1 2 3 4 5"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine1, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine1, &inline_nest_level, &call_site_line, &call_site_file_id, + &origin_id, &ranges)); // Test negative origin_id. - char kTestLine2[] = "INLINE 0 1 -2 3 4"; + char kTestLine2[] = "INLINE 0 1 2 -3 4 5"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine2, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine2, &inline_nest_level, &call_site_line, &call_site_file_id, + &origin_id, &ranges)); // Test missing ranges. - char kTestLine3[] = "INLINE 0 1 -2"; + char kTestLine3[] = "INLINE 0 1 2 -2"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine3, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine3, &inline_nest_level, &call_site_line, &call_site_file_id, + &origin_id, &ranges)); // Test missing size for range. - char kTestLine4[] = "INLINE 0 1 -2 3"; + char kTestLine4[] = "INLINE 0 1 -2 3 4"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine4, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine4, &inline_nest_level, &call_site_line, &call_site_file_id, + &origin_id, &ranges)); } } // namespace diff --git a/src/processor/fast_source_line_resolver.cc b/src/processor/fast_source_line_resolver.cc index 029f21f4..31786460 100644 --- a/src/processor/fast_source_line_resolver.cc +++ b/src/processor/fast_source_line_resolver.cc @@ -64,7 +64,7 @@ bool FastSourceLineResolver::ShouldDeleteMemoryBufferAfterLoadModule() { void FastSourceLineResolver::Module::LookupAddress( StackFrame* frame, - vector>* inlined_frames) const { + std::deque>* inlined_frames) const { MemAddr address = frame->instruction - frame->module->base_address(); // First, look for a FUNC record that covers address. Use diff --git a/src/processor/fast_source_line_resolver_types.h b/src/processor/fast_source_line_resolver_types.h index aa02fc11..2d1bcfcb 100644 --- a/src/processor/fast_source_line_resolver_types.h +++ b/src/processor/fast_source_line_resolver_types.h @@ -119,7 +119,7 @@ class FastSourceLineResolver::Module: public SourceLineResolverBase::Module { // with the result. virtual void LookupAddress( StackFrame* frame, - std::vector>* inlined_frames) const; + std::deque>* inlined_frames) const; // Loads a map from the given buffer in char* type. virtual bool LoadMapFromMemory(char* memory_buffer, diff --git a/src/processor/source_line_resolver_base.cc b/src/processor/source_line_resolver_base.cc index 16c82224..cea1a46d 100644 --- a/src/processor/source_line_resolver_base.cc +++ b/src/processor/source_line_resolver_base.cc @@ -297,7 +297,7 @@ bool SourceLineResolverBase::IsModuleCorrupt(const CodeModule* module) { void SourceLineResolverBase::FillSourceLineInfo( StackFrame* frame, - std::vector>* inlined_frames) { + std::deque>* inlined_frames) { if (frame->module) { ModuleMap::const_iterator it = modules_->find(frame->module->code_file()); if (it != modules_->end()) { diff --git a/src/processor/source_line_resolver_base_types.h b/src/processor/source_line_resolver_base_types.h index 3e3afd0e..44968bf6 100644 --- a/src/processor/source_line_resolver_base_types.h +++ b/src/processor/source_line_resolver_base_types.h @@ -40,6 +40,7 @@ #include +#include #include #include #include @@ -70,10 +71,7 @@ class SourceLineResolverBase::AutoFileCloser { }; struct SourceLineResolverBase::InlineOrigin { - InlineOrigin(int32_t source_file_id, const string& name) - : source_file_id(source_file_id), name(name) {} - - int32_t source_file_id; + explicit InlineOrigin(const string& name) : name(name) {} string name; }; @@ -82,15 +80,18 @@ struct SourceLineResolverBase::Inline { using InlineRanges = std::vector>; Inline(int32_t inline_nest_level, int32_t call_site_line, + int32_t call_site_file_id, int32_t origin_id, InlineRanges inline_ranges) : inline_nest_level(inline_nest_level), call_site_line(call_site_line), + call_site_file_id(call_site_file_id), origin_id(origin_id), inline_ranges(inline_ranges) {} int32_t inline_nest_level; int32_t call_site_line; + int32_t call_site_file_id; int32_t origin_id; InlineRanges inline_ranges; RangeMap> child_inlines; @@ -174,7 +175,7 @@ class SourceLineResolverBase::Module { // with the result. virtual void LookupAddress( StackFrame* frame, - std::vector>* inlined_frames) const = 0; + std::deque>* inlined_frames) const = 0; // If Windows stack walking information is available covering ADDRESS, // return a WindowsFrameInfo structure describing it. If the information diff --git a/src/processor/stack_frame_symbolizer.cc b/src/processor/stack_frame_symbolizer.cc index 6490ca90..a460bc9e 100644 --- a/src/processor/stack_frame_symbolizer.cc +++ b/src/processor/stack_frame_symbolizer.cc @@ -58,7 +58,7 @@ StackFrameSymbolizer::SymbolizerResult StackFrameSymbolizer::FillSourceLineInfo( const CodeModules* unloaded_modules, const SystemInfo* system_info, StackFrame* frame, - std::vector>* inlined_frames) { + std::deque>* inlined_frames) { assert(frame); const CodeModule* module = NULL; diff --git a/src/processor/stackwalk_common.cc b/src/processor/stackwalk_common.cc index 856a6a66..c1e17084 100644 --- a/src/processor/stackwalk_common.cc +++ b/src/processor/stackwalk_common.cc @@ -218,7 +218,7 @@ static void PrintStackContents(const string& indent, modules->GetModuleForAddress(pointee_frame.instruction); // Try to look up the function name. - vector> inlined_frames; + std::deque> inlined_frames; if (pointee_frame.module) resolver->FillSourceLineInfo(&pointee_frame, &inlined_frames); diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index d4897d4c..cb383119 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -138,7 +138,7 @@ bool Stackwalker::Walk( // frame_pointer fields. The frame structure comes from either the // context frame (above) or a caller frame (below). - vector> inlined_frames; + std::deque> inlined_frames; // Resolve the module information, if a module map was provided. StackFrameSymbolizer::SymbolizerResult symbolizer_result = frame_symbolizer_->FillSourceLineInfo(modules_, unloaded_modules_, @@ -174,10 +174,10 @@ bool Stackwalker::Walk( default: break; } - // Add all nested inlined frames belonging to this frame in reverse order. + // Add all nested inlined frames belonging to this frame from left to right. while (!inlined_frames.empty()) { - stack->frames_.push_back(inlined_frames.back().release()); - inlined_frames.pop_back(); + stack->frames_.push_back(inlined_frames.front().release()); + inlined_frames.pop_front(); } // Add the frame to the call stack. Relinquish the ownership claim // over the frame, because the stack now owns it. diff --git a/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.sym b/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.sym index 775640f0..db797142 100644 --- a/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.sym +++ b/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.sym @@ -1,13 +1,16 @@ MODULE Linux x86_64 BBA6FA10B8AAB33D00000000000000000 linux_inline INFO CODE_ID 10FAA6BBAAB83DB3 FILE 0 linux_inline.cpp -INLINE_ORIGIN 0 0 bar() -INLINE_ORIGIN 1 0 foo() -INLINE_ORIGIN 2 0 func() +FILE 1 a.cpp +FILE 2 b.cpp +FILE 3 c.cpp +INLINE_ORIGIN 0 bar() +INLINE_ORIGIN 1 foo() +INLINE_ORIGIN 2 func() FUNC 15b30 6cf 0 main -INLINE 0 42 1 15b45 6b1 -INLINE 1 39 0 15b72 684 -INLINE 2 32 2 15b83 673 +INLINE 0 42 1 1 15b45 6b1 +INLINE 1 39 2 0 15b72 684 +INLINE 2 32 3 2 15b83 673 15b30 15 41 0 15b45 11 36 0 15b56 a 37 0