Fix incorrect source file name for inlined frames

Processor shows incorrect source file name if a frame have an inlined
frame and their source files are different.
Consider this example:
FILE 0 /tmp/a.h
FILE 1 /tmp/a.cpp
INLINE_ORIGIN 0 0 foo()
FUNC 1110 a 0 main
INLINE 0 22 0 1110 7
1110 7 3 0
1117 3 23 1

When querying the address 0x1110, we know this line 0x1110 corresponds
to /tmp/a.h line 3 and it's inside a inlined function foo() which is
defined at /tmp/a.h and called at line 22. But we don't know at which
file it's being called at line 22. So, we will get stacks like this:
void foo() /tmp/a.h:3
int main() /tmp/a.h:22

The correct stacks should be this:
void foo() /tmp/a.h:3
int main() /tmp/a.cpp:22

In this change:
1. Remove file_id field for INLINE_ORIGIN record.
2. Add call_site_file_id for INLINE record to represents the file where
this call being inlined.

After adding call_site_file_id to it (as third field), it looks like
this:
FILE 0 /tmp/a.h
FILE 1 /tmp/a.cpp
INLINE_ORIGIN 0 foo()
FUNC 1110 a 0 main
INLINE 0 22 1 0 1110 7
1110 7 3 0
1117 3 23 1

Bug: 1190878
Change-Id: Ibbb697d2f7e1b6ac3208cac6fae4353c8743198d
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3232838
Reviewed-by: Joshua Peraza <jperaza@chromium.org>

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<Module::Function*, uint64_t> spec_function_offsets;
-
-  // From file index to vector of subprogram's offset in this CU.
-  map<uint64_t, vector<uint64_t>> 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<Module::Inline> in(
-      new Module::Inline(origin, ranges, call_site_line_, inline_nest_level_,
+      new Module::Inline(origin, ranges, call_site_line_, call_site_file_id_,
-                         std::move(child_inlines_)));
+                         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) {}
 
@@ -701,8 +701,6 @@ class DwarfCUToModule::FuncHandler: 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
-  // DW_AT_decl_file, value of UINT64_MAX means undefined.
-  uint64_t decl_file_data_;
   bool inline_;
   vector<unique_ptr<Module::Inline>> 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_ &&
+  if (handle_inline_ && (!empty_range || inline_)) {
-      (!empty_range || inline_ || decl_file_data_ != UINT64_MAX)) {
     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_) {
+  // Assign File* to Inlines inside this CU.
-    cu_context_->file_context->module_->inline_origin_map
+  auto assignFile = [this](unique_ptr<Module::Inline>& in) {
-        .AssignFilesToInlineOrigins(cu_context_->inline_origins[iter.first],
+    in->call_site_file = files_[in->call_site_file_id];
-                                    iter.second);
+  };
+  for (auto func : cu_context_->functions) {
+    Module::Inline::InlineDFS(func->inlines, assignFile);
   }
 }
 

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<uint64_t>& 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<Inline>& in) {
     // There are some artificial inline functions which don't belong to
     // any file. Those will have file id -1.
-    if (origin->file)
+    if (in->call_site_file)
-      origin->file->source_id = 0;
+      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<unique_ptr<Module::Inline>>& inlines,
-    std::function<void(unique_ptr<Module::Inline>&)> const& forEach) {
-  for (unique_ptr<Module::Inline>& in : inlines) {
-    forEach(in);
-    InlineDFS(in->child_inlines, forEach);
-  }
-}
-
 void Module::CreateInlineOrigins(
     set<InlineOrigin*, InlineOriginCompare>& 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()
+      stream << "INLINE_ORIGIN " << origin->id << " " << origin->name << "\n";
-             << " " << 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<Inline>& 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();
 

src/common/module.h

@@ -38,6 +38,7 @@
 #ifndef COMMON_LINUX_MODULE_H__
 #define COMMON_LINUX_MODULE_H__
 
+#include <functional>
 #include <iostream>
 #include <limits>
 #include <map>
@@ -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<Range>& ranges,
            int call_site_line,
+           int call_site_file_id,
            int inline_nest_level,
            vector<std::unique_ptr<Inline>> 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<std::unique_ptr<Inline>> child_inlines;
+
+    int getCallSiteFileID() const {
+      return call_site_file ? call_site_file->source_id : -1;
+    }
+
+    static void InlineDFS(
+        vector<std::unique_ptr<Module::Inline>>& inlines,
+        std::function<void(std::unique_ptr<Module::Inline>&)> const& forEach) {
+      for (std::unique_ptr<Module::Inline>& in : inlines) {
+        forEach(in);
+        InlineDFS(in->child_inlines, forEach);
+      }
+    }
   };
 
   typedef map<uint64_t, InlineOrigin*> 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<uint64_t>& inline_origin_offsets,
-        File* file);
     ~InlineOriginMap() {
       for (const auto& iter : inline_origins_) {
         delete iter.second;
@@ -262,9 +279,7 @@ 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();
     }
   };
 

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 <origin_id> <file_id> <name>.
+  // Format: INLINE_ORIGIN <origin_id> <name>.
   // Notice, that this method modifies the input |inline_origin_line| which is
-  // why it can't be const.  On success, <origin_id>, <file_id> and <name> are
+  // why it can't be const.  On success, <origin_id> and <name> are
-  // stored in |*origin_id|, |*file_id|, and |*name|.  No allocation is
+  // 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 <inline_nest_level> <call_site_line> <origin_id> <address>
+  // Format: INLINE <inline_nest_level> <call_site_line> <call_site_file_id>
-  // <size> ....
+  // <origin_id> [<address> <size>]+
-  // Notice, that this method modifies the input |inline|
+  // Notice, that this method modifies the input
-  // which is why it can't be const.  On success, <inline_nest_level>,
+  // |inline| which is why it can't be const.  On success, <inline_nest_level>,
-  // <call_site_line> and <origin_id> are stored in |*inline_nest_level|,
+  // <call_site_line>, <call_site_file_id> and <origin_id> are stored in
-  // |*call_site_line|, and |*origin_id|, and all pairs of (<address>, <size>)
+  // |*inline_nest_level|, |*call_site_line|, |*call_site_file_id| and
-  // are added into ranges .
+  // |*origin_id|, and all pairs of (<address>, <size>) 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<std::pair<MemAddr, MemAddr>>* ranges);  // out
 

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 <deque>
 #include <map>
 #include <set>
 #include <string>
@@ -86,7 +87,7 @@ class SourceLineResolverBase : public SourceLineResolverInterface {
   virtual bool IsModuleCorrupt(const CodeModule* module);
   virtual void FillSourceLineInfo(
       StackFrame* frame,
-      std::vector<std::unique_ptr<StackFrame>>* inlined_frames);
+      std::deque<std::unique_ptr<StackFrame>>* inlined_frames);
   virtual WindowsFrameInfo* FindWindowsFrameInfo(const StackFrame* frame);
   virtual CFIFrameInfo* FindCFIFrameInfo(const StackFrame* frame);
 

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 <deque>
 #include <memory>
 #include <string>
 #include <vector>
@@ -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<std::unique_ptr<StackFrame>>* inlined_frames) = 0;
+      std::deque<std::unique_ptr<StackFrame>>* inlined_frames) = 0;
 
   // If Windows stack walking information is available covering
   // FRAME's instruction address, return a WindowsFrameInfo structure

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 <deque>
 #include <memory>
 #include <set>
 #include <string>
@@ -82,7 +83,7 @@ class StackFrameSymbolizer {
       const CodeModules* unloaded_modules,
       const SystemInfo* system_info,
       StackFrame* stack_frame,
-      std::vector<std::unique_ptr<StackFrame>>* inlined_frames);
+      std::deque<std::unique_ptr<StackFrame>>* inlined_frames);
 
   virtual WindowsFrameInfo* FindWindowsFrameInfo(const StackFrame* frame);
 

src/processor/basic_source_line_resolver.cc

@@ -50,10 +50,11 @@
 
 #include "processor/tokenize.h"
 
-using std::map;
+using std::deque;
-using std::vector;
 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<uint64_t, linked_ptr<Inline>>& inlines,
-    vector<unique_ptr<StackFrame>>* inlined_frames) const {
+    deque<unique_ptr<StackFrame>>* inlined_frames) const {
   linked_ptr<Inline> 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);
+  // Update parent frame's source line and source file.
-  new_frame.function_name = origin->second->name;
+  frame->source_line = in->call_site_line;
-  // Use the starting adress of the inlined range as inlined function base.
+  auto file = files_.find(in->call_site_file_id);
-  new_frame.function_base = new_frame.module->base_address() + inline_base;
+  if (file != files_.end()) {
-  auto it = files_.find(origin->second->source_file_id);
+    frame->source_file_name = file->second;
-  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<StackFrame>(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;
+
+  // 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<StackFrame>(new StackFrame(child_frame)));
 }
 
 void BasicSourceLineResolver::Module::LookupAddress(
     StackFrame* frame,
-    vector<unique_ptr<StackFrame>>* inlined_frames) const {
+    deque<unique_ptr<StackFrame>>* 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 =
+      int source_line = frame->source_line;
+      string source_file_name = frame->source_file_name;
       ConstructInlineFrames(frame, address, func->inlines, inlined_frames);
-      if (source_line != -1) {
+      if (!inlined_frames->empty()) {
-        frame->source_line = source_line;
+        // 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)) {
+                                           &origin_name)) {
-    inline_origins_.insert(
+    inline_origins_.insert(make_pair(origin_id, new InlineOrigin(origin_name)));
-        make_pair(origin_id, new InlineOrigin(source_file_id, origin_name)));
     return true;
   }
   return false;
@@ -431,12 +434,14 @@ linked_ptr<BasicSourceLineResolver::Inline>
 BasicSourceLineResolver::Module::ParseInline(char* inline_line) {
   long inline_nest_level;
   long call_site_line;
+  long call_site_file_id;
   long origin_id;
   vector<std::pair<MemAddr, MemAddr>> ranges;
   if (SymbolParseHelper::ParseInline(inline_line, &inline_nest_level,
-                                     &call_site_line, &origin_id, &ranges)) {
+                                     &call_site_line, &call_site_file_id,
-    return linked_ptr<Inline>(
+                                     &origin_id, &ranges)) {
-        new Inline(inline_nest_level, call_site_line, origin_id, ranges));
+    return linked_ptr<Inline>(new Inline(inline_nest_level, call_site_line,
+                                         call_site_file_id, origin_id, ranges));
   }
   return linked_ptr<Inline>();
 }
@@ -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 <origin_id> <file_id> <name>
+  // INLINE_ORIGIN <origin_id> <name>
   assert(strncmp(inline_origin_line, "INLINE_ORIGIN ", 14) == 0);
   inline_origin_line += 14;  // skip prefix
   vector<char*> 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);
+  *name = tokens[1];
-  // 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<long>::max()) {
-    return false;
-  }
-
-  *name = tokens[2];
   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<std::pair<MemAddr, MemAddr>>* ranges) {
-  // INLINE <inline_nest_level> <call_site_line> <origin_id> <address> <size>
+  // INLINE <inline_nest_level> <call_site_line> <call_site_file_id> <origin_id>
-  // ...
+  // [<address> <size>]+
   assert(strncmp(inline_line, "INLINE ", 7) == 0);
   inline_line += 7; // skip prefix
 
   vector<char*> tokens;
   Tokenize(inline_line, kWhitespace, std::numeric_limits<int>::max(), &tokens);
 
-  // The length of the vector should be at least 5 and an odd number.
+  // The length of the vector should be at least 6 and an even number.
-  if (tokens.size() < 5 && tokens.size() % 2 == 0)
+  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<long>::max()) {
+    return false;
+  }
+
+  *origin_id = strtol(tokens[3], &after_number, 10);
   if (!IsValidAfterNumber(after_number) || *origin_id < 0 ||
       *origin_id == std::numeric_limits<long>::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<unsigned long long>::max()) {

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 <deque>
 #include <map>
 #include <string>
 
@@ -108,15 +109,18 @@ class BasicSourceLineResolver::Module : public SourceLineResolverBase::Module {
   // with the result.
   virtual void LookupAddress(
       StackFrame* frame,
-      std::vector<std::unique_ptr<StackFrame>>* inlined_frame) const;
+      std::deque<std::unique_ptr<StackFrame>>* inlined_frame) const;
 
-  // Construct inlined frame for frame and return inlined function call site
+  // Construct inlined frames for frame. Return true on success.
-  // source line. If failed to construct inlined frame, return -1.
+  // If successfully construct inlined frames for `frame`, `inline_frames` will
-  virtual int ConstructInlineFrames(
+  // 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<uint64_t, linked_ptr<Inline>>& inlines,
-      std::vector<std::unique_ptr<StackFrame>>* inline_frames) const;
+      std::deque<std::unique_ptr<StackFrame>>* inline_frames) const;
 
   // If Windows stack walking information is available covering ADDRESS,
   // return a WindowsFrameInfo structure describing it. If the information

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<std::unique_ptr<StackFrame>> inlined_frames;
+  std::deque<std::unique_ptr<StackFrame>> 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[2]->function_name, "foo()");
-  ASSERT_EQ(inlined_frames[0]->function_base, 0x15b45U);
+  ASSERT_EQ(inlined_frames[2]->function_base, 0x15b45U);
-  ASSERT_EQ(inlined_frames[0]->source_file_name, "linux_inline.cpp");
+  ASSERT_EQ(inlined_frames[2]->source_file_name, "b.cpp");
-  ASSERT_EQ(inlined_frames[0]->source_line, 39);
+  ASSERT_EQ(inlined_frames[2]->source_line, 39);
-  ASSERT_EQ(inlined_frames[0]->source_line_base, 0x161b6U);
+  ASSERT_EQ(inlined_frames[2]->source_line_base, 0x161b6U);
-  ASSERT_EQ(inlined_frames[0]->trust, StackFrame::FRAME_TRUST_INLINE);
+  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[0]->function_name, "func()");
-  ASSERT_EQ(inlined_frames[2]->function_base, 0x15b83U);
+  ASSERT_EQ(inlined_frames[0]->function_base, 0x15b83U);
-  ASSERT_EQ(inlined_frames[2]->source_file_name, "linux_inline.cpp");
+  ASSERT_EQ(inlined_frames[0]->source_file_name, "linux_inline.cpp");
-  ASSERT_EQ(inlined_frames[2]->source_line, 27);
+  ASSERT_EQ(inlined_frames[0]->source_line, 27);
-  ASSERT_EQ(inlined_frames[2]->source_line_base, 0x161b6U);
+  ASSERT_EQ(inlined_frames[0]->source_line_base, 0x161b6U);
-  ASSERT_EQ(inlined_frames[2]->trust, StackFrame::FRAME_TRUST_INLINE);
+  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 <origin_id> <file_id> <name>
+// INLINE_ORIGIN <origin_id> <name>
 TEST(SymbolParseHelper, ParseInlineOriginValid) {
   long origin_id;
-  long file_id;
   char* name;
 
-  char kTestLine[] = "INLINE_ORIGIN 1 1 function name";
+  char kTestLine[] = "INLINE_ORIGIN 1 function name";
-  ASSERT_TRUE(SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id,
+  ASSERT_TRUE(
-                                                   &file_id, &name));
+      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 <origin_id> <file_id> <name>
 TEST(SymbolParseHelper, ParseInlineOriginInvalid) {
   long origin_id;
-  long file_id;
   char* name;
 
   // Test missing function name.
-  char kTestLine[] = "INLINE_ORIGIN 1 1";
+  char kTestLine[] = "INLINE_ORIGIN 1";
-  ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id,
+  ASSERT_FALSE(
-                                                   &file_id, &name));
+      SymbolParseHelper::ParseInlineOrigin(kTestLine, &origin_id, &name));
 
   // Test bad origin id.
-  char kTestLine1[] = "INLINE_ORIGIN x1 1 function name";
+  char kTestLine1[] = "INLINE_ORIGIN x1 function name";
-  ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine1, &origin_id,
+  ASSERT_FALSE(
-                                                   &file_id, &name));
+      SymbolParseHelper::ParseInlineOrigin(kTestLine1, &origin_id, &name));
 
   // Test large origin id.
-  char kTestLine2[] = "INLINE_ORIGIN 123123123123123123123123 1 function name";
+  char kTestLine2[] = "INLINE_ORIGIN 123123123123123123123123 function name";
-  ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine2, &origin_id,
+  ASSERT_FALSE(
-                                                   &file_id, &name));
+      SymbolParseHelper::ParseInlineOrigin(kTestLine2, &origin_id, &name));
 
   // Test negative origin id.
-  char kTestLine3[] = "INLINE_ORIGIN -1 1 function name";
+  char kTestLine3[] = "INLINE_ORIGIN -1 function name";
-  ASSERT_FALSE(SymbolParseHelper::ParseInlineOrigin(kTestLine3, &origin_id,
+  ASSERT_FALSE(
-                                                   &file_id, &name));
+      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<std::pair<uint64_t, uint64_t>> 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(2, call_site_file_id);
-  EXPECT_EQ(0x3ULL, ranges[0].first);
+  EXPECT_EQ(3, origin_id);
-  EXPECT_EQ(0x4ULL, ranges[0].second);
+  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<std::pair<uint64_t, uint64_t>> 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

src/processor/fast_source_line_resolver.cc

@@ -64,7 +64,7 @@ bool FastSourceLineResolver::ShouldDeleteMemoryBufferAfterLoadModule() {
 
 void FastSourceLineResolver::Module::LookupAddress(
     StackFrame* frame,
-    vector<std::unique_ptr<StackFrame>>* inlined_frames) const {
+    std::deque<std::unique_ptr<StackFrame>>* inlined_frames) const {
   MemAddr address = frame->instruction - frame->module->base_address();
 
   // First, look for a FUNC record that covers address. Use

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<std::unique_ptr<StackFrame>>* inlined_frames) const;
+      std::deque<std::unique_ptr<StackFrame>>* inlined_frames) const;
 
   // Loads a map from the given buffer in char* type.
   virtual bool LoadMapFromMemory(char* memory_buffer,

src/processor/source_line_resolver_base.cc

@@ -297,7 +297,7 @@ bool SourceLineResolverBase::IsModuleCorrupt(const CodeModule* module) {
 
 void SourceLineResolverBase::FillSourceLineInfo(
     StackFrame* frame,
-    std::vector<std::unique_ptr<StackFrame>>* inlined_frames) {
+    std::deque<std::unique_ptr<StackFrame>>* inlined_frames) {
   if (frame->module) {
     ModuleMap::const_iterator it = modules_->find(frame->module->code_file());
     if (it != modules_->end()) {

src/processor/source_line_resolver_base_types.h

@@ -40,6 +40,7 @@
 
 #include <stdio.h>
 
+#include <deque>
 #include <map>
 #include <memory>
 #include <string>
@@ -70,10 +71,7 @@ class SourceLineResolverBase::AutoFileCloser {
 };
 
 struct SourceLineResolverBase::InlineOrigin {
-  InlineOrigin(int32_t source_file_id, const string& name)
+  explicit InlineOrigin(const string& name) : name(name) {}
-      : source_file_id(source_file_id), name(name) {}
-
-  int32_t source_file_id;
   string name;
 };
 
@@ -82,15 +80,18 @@ struct SourceLineResolverBase::Inline {
   using InlineRanges = std::vector<std::pair<MemAddr, MemAddr>>;
   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<MemAddr, linked_ptr<Inline>> child_inlines;
@@ -174,7 +175,7 @@ class SourceLineResolverBase::Module {
   // with the result.
   virtual void LookupAddress(
       StackFrame* frame,
-      std::vector<std::unique_ptr<StackFrame>>* inlined_frames) const = 0;
+      std::deque<std::unique_ptr<StackFrame>>* inlined_frames) const = 0;
 
   // If Windows stack walking information is available covering ADDRESS,
   // return a WindowsFrameInfo structure describing it. If the information

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<std::unique_ptr<StackFrame>>* inlined_frames) {
+    std::deque<std::unique_ptr<StackFrame>>* inlined_frames) {
   assert(frame);
 
   const CodeModule* module = NULL;

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<unique_ptr<StackFrame>> inlined_frames;
+    std::deque<unique_ptr<StackFrame>> inlined_frames;
     if (pointee_frame.module)
       resolver->FillSourceLineInfo(&pointee_frame, &inlined_frames);
 

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<std::unique_ptr<StackFrame>> inlined_frames;
+    std::deque<std::unique_ptr<StackFrame>> 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());
+      stack->frames_.push_back(inlined_frames.front().release());
-      inlined_frames.pop_back();
+      inlined_frames.pop_front();
     }
     // Add the frame to the call stack.  Relinquish the ownership claim
     // over the frame, because the stack now owns it.

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()
+FILE 1 a.cpp
-INLINE_ORIGIN 1 0 foo()
+FILE 2 b.cpp
-INLINE_ORIGIN 2 0 func()
+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 0 42 1 1 15b45 6b1
-INLINE 1 39 0 15b72 684
+INLINE 1 39 2 0 15b72 684
-INLINE 2 32 2 15b83 673
+INLINE 2 32 3 2 15b83 673
 15b30 15 41 0
 15b45 11 36 0
 15b56 a 37 0