From ee2ad61263ebc54396df7d7a835e1e3f8455134e Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Fri, 29 Oct 2021 16:20:55 -0700 Subject: [PATCH] Make processor compatible with both old and new format INLINE/INLINE_ORIGIN This is similar to the processor part of https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3232838/, but added compatibility to process both old and new format of INLINE/INLINE_ORIGIN records in symbol file. Old INLINE format: INLINE [
]+ New INLINE format: INLINE [
]+ Old INLINE_ORIGIN format: INLINE_ORIGIN New INLINE_ORIGIN format: INLINE_ORIGIN Change-Id: I555d9747bfd44a1a95113b9946dcd509b7710876 Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3248433 Reviewed-by: Joshua Peraza --- .../processor/basic_source_line_resolver.h | 27 ++- .../processor/source_line_resolver_base.h | 3 +- .../source_line_resolver_interface.h | 3 +- .../processor/stack_frame_symbolizer.h | 3 +- src/processor/basic_source_line_resolver.cc | 162 +++++++++++----- .../basic_source_line_resolver_types.h | 12 +- .../basic_source_line_resolver_unittest.cc | 174 ++++++++++++++---- src/processor/fast_source_line_resolver.cc | 2 +- .../fast_source_line_resolver_types.h | 2 +- src/processor/source_line_resolver_base.cc | 2 +- .../source_line_resolver_base_types.h | 24 ++- src/processor/stack_frame_symbolizer.cc | 2 +- src/processor/stackwalk_common.cc | 2 +- src/processor/stackwalker.cc | 9 +- .../linux_inline.new.sym | 71 +++++++ ...{linux_inline.sym => linux_inline.old.sym} | 0 16 files changed, 377 insertions(+), 121 deletions(-) create mode 100644 src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.new.sym rename src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/{linux_inline.sym => linux_inline.old.sym} (100%) diff --git a/src/google_breakpad/processor/basic_source_line_resolver.h b/src/google_breakpad/processor/basic_source_line_resolver.h index fe76ad4d..d676ab70 100644 --- a/src/google_breakpad/processor/basic_source_line_resolver.h +++ b/src/google_breakpad/processor/basic_source_line_resolver.h @@ -98,28 +98,35 @@ class SymbolParseHelper { char** filename); // out // Parses a |inline_origin_line| declaration. Returns true on success. - // Format: INLINE_ORIGIN . + // Old Format: INLINE_ORIGIN . + // New 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 - // done, |*name| simply points inside |inline_origin_line|. + // why it can't be const. On success, , , + // and are stored in |*has_file_id*|, |*origin_id|, |*file_id|, and + // |*name|. No allocation is done, |*name| simply points inside + // |inline_origin_line|. static bool ParseInlineOrigin(char* inline_origin_line, // in + bool* has_file_id, // out long* origin_id, // out long* file_id, // out char** name); // out // Parses a |inline| declaration. Returns true on success. - // Format: INLINE
- // .... + // Old Format: INLINE + // [
]+ + // New 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 . + // which is why it can't be const. On success, , + // , and are stored in + // |*has_call_site_file_id*|, |*inline_nest_level|, |*call_site_line|, and + // |*origin_id|, and all pairs of (
, ) are added into ranges. static bool ParseInline( char* inline_line, // in + bool* has_call_site_file_id, // out 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..2e05d72a 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,47 @@ 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; + + // Update parent frame's source line (and source file if it's the new format). + frame->source_line = in->call_site_line; + if (in->has_call_site_file_id) { + auto file = files_.find(in->call_site_file_id); + if (file != files_.end()) { + frame->source_file_name = file->second; + } + } StackFrame new_frame = StackFrame(*frame); new_frame.function_name = origin->second->name; + if (origin->second->has_file_id) { + auto file = files_.find(origin->second->source_file_id); + if (file != files_.end()) + new_frame.source_file_name = file->second; + } // 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(); + ConstructInlineFrames(&new_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(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; - } - return in->call_site_line; } 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 +312,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, @@ -415,13 +424,16 @@ bool BasicSourceLineResolver::Module::ParseFile(char* file_line) { bool BasicSourceLineResolver::Module::ParseInlineOrigin( char* inline_origin_line) { + bool has_file_id; 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))); + if (SymbolParseHelper::ParseInlineOrigin(inline_origin_line, &has_file_id, + &origin_id, &source_file_id, + &origin_name)) { + inline_origins_.insert(make_pair( + origin_id, + new InlineOrigin(has_file_id, source_file_id, origin_name))); return true; } return false; @@ -429,14 +441,18 @@ bool BasicSourceLineResolver::Module::ParseInlineOrigin( linked_ptr BasicSourceLineResolver::Module::ParseInline(char* inline_line) { + bool has_call_site_file_id; 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)); + if (SymbolParseHelper::ParseInline(inline_line, &has_call_site_file_id, + &inline_nest_level, &call_site_line, + &call_site_file_id, &origin_id, &ranges)) { + return linked_ptr(new Inline(has_call_site_file_id, + inline_nest_level, call_site_line, + call_site_file_id, origin_id, ranges)); } return linked_ptr(); } @@ -635,14 +651,21 @@ bool SymbolParseHelper::ParseFile(char* file_line, long* index, // static bool SymbolParseHelper::ParseInlineOrigin(char* inline_origin_line, + bool* has_file_id, long* origin_id, long* file_id, char** name) { + // Old INLINE_ORIGIN format: // INLINE_ORIGIN + // New INLINE_ORIGIN format: + // 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)) { + // Split the line into two parts so that the first token is "", and + // second token is either " "" or """ depending on the + // format version. + if (!Tokenize(inline_origin_line, kWhitespace, 2, &tokens)) { return false; } @@ -653,15 +676,35 @@ 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; + // If the field after origin_id is a number, then it's old format. + char* remaining_line = tokens[1]; + *has_file_id = true; + for (size_t i = 0; + i < strlen(remaining_line) && remaining_line[i] != ' ' && *has_file_id; + ++i) { + // 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 (remaining_line[i] == '-' && i == 0) { + continue; + } + *has_file_id = isdigit(remaining_line[i]); } - *name = tokens[2]; + if (*has_file_id) { + // If it's old format, split " " to {"", ""}. + if (!Tokenize(remaining_line, kWhitespace, 2, &tokens)) { + return false; + } + *file_id = strtol(tokens[0], &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 || + *file_id == std::numeric_limits::max()) { + return false; + } + } + + *name = tokens[1]; if (!*name) { return false; } @@ -672,48 +715,69 @@ bool SymbolParseHelper::ParseInlineOrigin(char* inline_origin_line, // static bool SymbolParseHelper::ParseInline( char* inline_line, + bool* has_call_site_file_id, long* inline_nest_level, long* call_site_line, + long* call_site_file_id, long* origin_id, vector>* ranges) { - // INLINE
- // ... + // Old INLINE format: + // INLINE [
]+ + // New INLINE format: + // 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) + // Determine the version of INLINE record by parity of the vector length. + *has_call_site_file_id = tokens.size() % 2 == 0; + + // The length of the vector should be at least 5. + if (tokens.size() < 5) { return false; + } char* after_number; - *inline_nest_level = strtol(tokens[0], &after_number, 10); + size_t next_idx = 0; + + *inline_nest_level = strtol(tokens[next_idx++], &after_number, 10); if (!IsValidAfterNumber(after_number) || *inline_nest_level < 0 || *inline_nest_level == std::numeric_limits::max()) { return false; } - *call_site_line = strtol(tokens[1], &after_number, 10); + *call_site_line = strtol(tokens[next_idx++], &after_number, 10); if (!IsValidAfterNumber(after_number) || *call_site_line < 0 || *call_site_line == std::numeric_limits::max()) { return false; } - *origin_id = strtol(tokens[2], &after_number, 10); + if (*has_call_site_file_id) { + *call_site_file_id = strtol(tokens[next_idx++], &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[next_idx++], &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();) { - MemAddr address = strtoull(tokens[i++], &after_number, 16); + while (next_idx < tokens.size()) { + MemAddr address = strtoull(tokens[next_idx++], &after_number, 16); if (!IsValidAfterNumber(after_number) || address == std::numeric_limits::max()) { return false; } - MemAddr size = strtoull(tokens[i++], &after_number, 16); + MemAddr size = strtoull(tokens[next_idx++], &after_number, 16); if (!IsValidAfterNumber(after_number) || size == std::numeric_limits::max()) { return false; diff --git a/src/processor/basic_source_line_resolver_types.h b/src/processor/basic_source_line_resolver_types.h index 482176f4..7d9d5cb5 100644 --- a/src/processor/basic_source_line_resolver_types.h +++ b/src/processor/basic_source_line_resolver_types.h @@ -108,15 +108,17 @@ 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( + // Recursively construct inlined frames for |frame| and store them in + // |inline_frames|. |frame|'s source line and source file name may be updated + // if an inlined frame is found inside |frame|. As a result, the innermost + // inlined frame will be the first one in |inline_frames|. + 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..e608a548 100644 --- a/src/processor/basic_source_line_resolver_unittest.cc +++ b/src/processor/basic_source_line_resolver_unittest.cc @@ -413,15 +413,15 @@ TEST_F(TestBasicSourceLineResolver, TestUnload) ASSERT_TRUE(resolver.HasModule(&module1)); } -TEST_F(TestBasicSourceLineResolver, TestLoadAndResolveInlines) { +TEST_F(TestBasicSourceLineResolver, TestLoadAndResolveOldInlines) { TestCodeModule module("linux_inline"); ASSERT_TRUE(resolver.LoadModule( &module, testdata_dir + "/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/" - "linux_inline.sym")); + "linux_inline.old.sym")); ASSERT_TRUE(resolver.HasModule(&module)); StackFrame frame; - std::vector> inlined_frames; + std::deque> inlined_frames; frame.instruction = 0x161b6; frame.module = &module; // main frame. @@ -435,12 +435,12 @@ TEST_F(TestBasicSourceLineResolver, TestLoadAndResolveInlines) { 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, "linux_inline.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); @@ -449,12 +449,56 @@ TEST_F(TestBasicSourceLineResolver, TestLoadAndResolveInlines) { 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[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_F(TestBasicSourceLineResolver, TestLoadAndResolveNewInlines) { + TestCodeModule module("linux_inline"); + ASSERT_TRUE(resolver.LoadModule( + &module, testdata_dir + + "/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/" + "linux_inline.new.sym")); + ASSERT_TRUE(resolver.HasModule(&module)); + StackFrame frame; + 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, "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[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, "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[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: @@ -780,68 +824,99 @@ TEST(SymbolParseHelper, ParsePublicSymbolInvalid) { &name)); } -// Test parsing of valid INLINE_ORIGIN lines. The format is: +// Test parsing of valid INLINE_ORIGIN lines. +// The old format: // INLINE_ORIGIN +// The new format: +// INLINE_ORIGIN TEST(SymbolParseHelper, ParseInlineOriginValid) { + bool has_file_id; long origin_id; long file_id; char* name; - + // Test for old format. char kTestLine[] = "INLINE_ORIGIN 1 1 function name"; - ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id, - &file_id, &name)); + ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin( + kTestLine, &has_file_id, &origin_id, &file_id, &name)); + EXPECT_EQ(true, has_file_id); 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)); + ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin( + kTestLine1, &has_file_id, &origin_id, &file_id, &name)); + EXPECT_EQ(true, has_file_id); EXPECT_EQ(0, origin_id); EXPECT_EQ(-1, file_id); EXPECT_EQ("function name", string(name)); + + // Test for new format. + char kTestLine2[] = "INLINE_ORIGIN 0 function name"; + ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin( + kTestLine2, &has_file_id, &origin_id, &file_id, &name)); + EXPECT_EQ(false, has_file_id); + EXPECT_EQ(0, origin_id); + EXPECT_EQ("function name", string(name)); + + char kTestLine3[] = "INLINE_ORIGIN 0 function"; + ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin( + kTestLine3, &has_file_id, &origin_id, &file_id, &name)); + EXPECT_EQ(false, has_file_id); + EXPECT_EQ(0, origin_id); + EXPECT_EQ("function", string(name)); } // Test parsing of valid INLINE ORIGIN lines. The format is: // INLINE_ORIGIN TEST(SymbolParseHelper, ParseInlineOriginInvalid) { + bool has_file_id; 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)); + ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin( + kTestLine, &has_file_id, &origin_id, &file_id, &name)); // Test bad origin id. char kTestLine1[] = "INLINE_ORIGIN x1 1 function name"; - ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine1, &origin_id, - &file_id, &name)); + ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin( + kTestLine1, &has_file_id, &origin_id, &file_id, &name)); // Test large origin id. char kTestLine2[] = "INLINE_ORIGIN 123123123123123123123123 1 function name"; - ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine2, &origin_id, - &file_id, &name)); + ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin( + kTestLine2, &has_file_id, &origin_id, &file_id, &name)); // Test negative origin id. char kTestLine3[] = "INLINE_ORIGIN -1 1 function name"; - ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine3, &origin_id, - &file_id, &name)); + ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin( + kTestLine3, &has_file_id, &origin_id, &file_id, &name)); } -// Test parsing of valid INLINE lines. The format is: -// INLINE
... +// Test parsing of valid INLINE lines. +// The old format: +// INLINE [
]+ +// The new format: +// INLINE +// [
]+ TEST(SymbolParseHelper, ParseInlineValid) { + bool has_call_site_file_id; long inline_nest_level; long call_site_line; + long call_site_file_id; long origin_id; std::vector> ranges; + // Test for old format. char kTestLine[] = "INLINE 0 1 2 3 4"; ASSERT_TRUE(SymbolParseHelper::ParseInline( - kTestLine, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine, &has_call_site_file_id, &inline_nest_level, &call_site_line, + &call_site_file_id, &origin_id, &ranges)); + EXPECT_EQ(false, has_call_site_file_id); EXPECT_EQ(0, inline_nest_level); EXPECT_EQ(1, call_site_line); EXPECT_EQ(2, origin_id); @@ -852,7 +927,9 @@ TEST(SymbolParseHelper, ParseInlineValid) { // Test hex and discontinuous ranges. char kTestLine1[] = "INLINE 0 1 2 a b 1a 1b"; ASSERT_TRUE(SymbolParseHelper::ParseInline( - kTestLine1, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine1, &has_call_site_file_id, &inline_nest_level, &call_site_line, + &call_site_file_id, &origin_id, &ranges)); + EXPECT_EQ(false, has_call_site_file_id); EXPECT_EQ(0, inline_nest_level); EXPECT_EQ(1, call_site_line); EXPECT_EQ(2, origin_id); @@ -860,40 +937,61 @@ TEST(SymbolParseHelper, ParseInlineValid) { EXPECT_EQ(0xbULL, ranges[0].second); EXPECT_EQ(0x1aULL, ranges[1].first); EXPECT_EQ(0x1bULL, ranges[1].second); + + // Test for new format. + char kTestLine2[] = "INLINE 0 1 2 3 a b 1a 1b"; + ASSERT_TRUE(SymbolParseHelper::ParseInline( + kTestLine2, &has_call_site_file_id, &inline_nest_level, &call_site_line, + &call_site_file_id, &origin_id, &ranges)); + EXPECT_EQ(true, has_call_site_file_id); + EXPECT_EQ(0, inline_nest_level); + EXPECT_EQ(1, call_site_line); + 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); + EXPECT_EQ(0x1bULL, ranges[1].second); } -// Test parsing of Invalid INLINE lines. The format is: -// INLINE
... +// Test parsing of Invalid INLINE lines. TEST(SymbolParseHelper, ParseInlineInvalid) { + bool has_call_site_file_id; 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"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine, &has_call_site_file_id, &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"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine1, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine1, &has_call_site_file_id, &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"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine2, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine2, &has_call_site_file_id, &inline_nest_level, &call_site_line, + &call_site_file_id, &origin_id, &ranges)); // Test missing ranges. char kTestLine3[] = "INLINE 0 1 -2"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine3, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine3, &has_call_site_file_id, &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"; ASSERT_FALSE(SymbolParseHelper::ParseInline( - kTestLine4, &inline_nest_level, &call_site_line, &origin_id, &ranges)); + kTestLine4, &has_call_site_file_id, &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..bac91802 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,9 +71,12 @@ class SourceLineResolverBase::AutoFileCloser { }; struct SourceLineResolverBase::InlineOrigin { - InlineOrigin(int32_t source_file_id, const string& name) - : source_file_id(source_file_id), name(name) {} - + InlineOrigin(bool has_file_id, int32_t source_file_id, const string& name) + : has_file_id(has_file_id), + source_file_id(source_file_id), + name(name) {} + // If it's old format, source file id is set, otherwise not useful. + bool has_file_id; int32_t source_file_id; string name; }; @@ -80,17 +84,23 @@ struct SourceLineResolverBase::InlineOrigin { struct SourceLineResolverBase::Inline { // A vector of (address, size) pair for a INLINE record. using InlineRanges = std::vector>; - Inline(int32_t inline_nest_level, + Inline(bool has_call_site_file_id, + 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), + : has_call_site_file_id(has_call_site_file_id), + 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) {} - + // If it's new format, call site file id is set, otherwise not useful. + bool has_call_site_file_id; 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 +184,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..e123b027 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,11 @@ 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 the innermost + // frame to the outermost frame. 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.new.sym b/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.new.sym new file mode 100644 index 00000000..5cd83209 --- /dev/null +++ b/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.new.sym @@ -0,0 +1,71 @@ +MODULE Linux x86_64 BBA6FA10B8AAB33D00000000000000000 linux_inline +INFO CODE_ID 10FAA6BBAAB83DB3 +FILE 0 linux_inline.cpp +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 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 +15b60 6 37 0 +15b66 5 38 0 +15b6b 7 0 0 +15b72 11 31 0 +15b83 a 9 0 +15b8d 4 9 0 +15b91 6 9 0 +15b97 7 0 0 +15b9e 11 10 0 +15baf 7 0 0 +15bb6 2e 12 0 +15be4 7 0 0 +15beb 5 12 0 +15bf0 1d 13 0 +15c0d 1d 14 0 +15c2a e 0 0 +15c38 1c 15 0 +15c54 a 16 0 +15c5e 7 0 0 +15c65 2c 16 0 +15c91 15 0 0 +15ca6 a 16 0 +15cb0 87 15 0 +15d37 7 0 0 +15d3e 33 15 0 +15d71 7 0 0 +15d78 24 15 0 +15d9c a 17 0 +15da6 e 0 0 +15db4 a 18 0 +15dbe e 0 0 +15dcc a 19 0 +15dd6 7 0 0 +15ddd a 20 0 +15de7 7 0 0 +15dee 2c 21 0 +15e1a 3c 22 0 +15e56 28 23 0 +15e7e 5a 18 0 +15ed8 d 28 0 +15ee5 11 12 0 +15ef6 67 28 0 +15f5d 2b 15 0 +15f88 7 0 0 +15f8f 8c 15 0 +1601b 7 0 0 +16022 3d 15 0 +1605f 67 28 0 +160c6 54 18 0 +1611a 3c 28 0 +16156 c 12 0 +16162 54 18 0 +161b6 2 27 0 +161b8 3e 28 0 +161f6 9 43 0 \ No newline at end of file diff --git a/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.sym b/src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.old.sym similarity index 100% rename from src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.sym rename to src/processor/testdata/symbols/linux_inline/BBA6FA10B8AAB33D00000000000000000/linux_inline.old.sym