Add <call_site_file_id> to INLINE and remove <file_id> from INLINE_ORIGIN

This is the dump_syms side change on
https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3232838/.
This fixes incorrect source file names when a inlined function's source
file name is different from its parent's.

Change-Id: I25683912d206c6a8db44e322eca5f7383ea8c47e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3248438
Reviewed-by: Joshua Peraza <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;
@@ -666,8 +667,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));
 }
 
@@ -684,7 +685,6 @@ class DwarfCUToModule::FuncHandler: public GenericDIEHandler {
         high_pc_form_(DW_FORM_addr),
         ranges_form_(DW_FORM_sec_offset),
         ranges_data_(0),
-        decl_file_data_(UINT64_MAX),
         inline_(false),
         handle_inline_(handle_inline) {}
 
@@ -705,9 +705,7 @@ class DwarfCUToModule::FuncHandler: public GenericDIEHandler {
   uint64_t low_pc_, high_pc_; // DW_AT_low_pc, DW_AT_high_pc
   DwarfForm high_pc_form_; // DW_AT_high_pc can be length or address.
   DwarfForm ranges_form_; // DW_FORM_sec_offset or DW_FORM_rnglistx
-  uint64_t ranges_data_; // DW_AT_ranges
+  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_;
@@ -732,9 +730,6 @@ void DwarfCUToModule::FuncHandler::ProcessAttributeUnsigned(
       ranges_data_ = data;
       ranges_form_ = form;
       break;
-    case DW_AT_decl_file:
-      decl_file_data_ = data;
-      break;
     default:
       GenericDIEHandler::ProcessAttributeUnsigned(attr, form, data);
       break;
@@ -862,8 +857,7 @@ void DwarfCUToModule::FuncHandler::Finish() {
 
   // Only keep track of DW_TAG_subprogram which have the attributes we are
   // interested.
-  if (handle_inline_ &&
+  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_;
@@ -871,8 +865,6 @@ void DwarfCUToModule::FuncHandler::Finish() {
                                                                        offset);
     cu_context_->file_context->module_->inline_origin_map
         .GetOrCreateInlineOrigin(offset_, name);
-    if (decl_file_data_ != UINT64_MAX)
-      cu_context_->inline_origins[decl_file_data_].push_back(offset_);
   }
 }
 
@@ -880,10 +872,12 @@ void DwarfCUToModule::FuncHandler::Finish() {
 // component to their names: namespaces, classes, etc.
 class DwarfCUToModule::NamedScopeHandler: public GenericDIEHandler {
  public:
-  NamedScopeHandler(CUContext* cu_context, DIEContext* parent_context,
+  NamedScopeHandler(CUContext* cu_context,
-                    uint64_t offset, bool handle_inline)
+                    DIEContext* parent_context,
-      : GenericDIEHandler(cu_context, parent_context, offset), 
+                    uint64_t offset,
-        handle_inline_(handle_inline) { }
+                    bool handle_inline)
+      : GenericDIEHandler(cu_context, parent_context, offset),
+        handle_inline_(handle_inline) {}
   bool EndAttributes();
   DIEHandler* FindChildHandler(uint64_t offset, enum DwarfTag tag);
 
@@ -1455,10 +1449,12 @@ void DwarfCUToModule::AssignLinesToFunctions() {
 }
 
 void DwarfCUToModule::AssignFilesToInlines() {
-  for (auto iter : files_) {
+  // 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,19 @@ void Module::AssignSourceIds(
          line_it != func->lines.end(); ++line_it)
       line_it->file->source_id = 0;
   }
-  // Also mark all files cited by inline functions by setting each one's source
+
+  // Also mark all files cited by inline callsite by setting each one's source
   // id to zero.
-  for (InlineOrigin* origin : inline_origins)
+  auto markInlineFiles = [](unique_ptr<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 +291,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 +303,7 @@ void Module::CreateInlineOrigins(
       in->origin = *it;
   };
   for (Function* func : functions_)
-    InlineDFS(func->inlines, addInlineOrigins);
+    Module::Inline::InlineDFS(func->inlines, addInlineOrigins);
   int next_id = 0;
   for (InlineOrigin* origin : inline_origins) {
     origin->id = next_id++;
@@ -381,8 +367,7 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) {
     }
     // Write out inline origins.
     for (InlineOrigin* origin : inline_origins) {
-      stream << "INLINE_ORIGIN " << origin->id << " " << origin->getFileID()
+      stream << "INLINE_ORIGIN " << origin->id << " " << origin->name << "\n";
-             << " " << origin->name << "\n";
       if (!stream.good())
         return ReportError();
     }
@@ -407,12 +392,12 @@ bool Module::Write(std::ostream& stream, SymbolData symbol_data) {
         auto write_inline = [&](unique_ptr<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.
@@ -150,11 +151,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 +169,29 @@ class Module {
 
     int call_site_line;
 
+    // The id is only meanful inside a CU. It's only used for looking up real
+    // File* after scanning a CU.
+    int call_site_file_id;
+
+    File* call_site_file;
+
     int inline_nest_level;
 
     // A list of inlines which are children of this inline.
     vector<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 +205,7 @@ class Module {
     // value of its DW_AT_specification or equals to offset if
     // DW_AT_specification doesn't exist in that DIE.
     void SetReference(uint64_t offset, uint64_t specification_offset);
-    void AssignFilesToInlineOrigins(
+
-        const vector<uint64_t>& inline_origin_offsets,
-        File* file);
     ~InlineOriginMap() {
       for (const auto& iter : inline_origins_) {
         delete iter.second;
@@ -261,10 +282,8 @@ class Module {
   };
 
   struct InlineOriginCompare {
-    bool operator() (const InlineOrigin* lhs, const InlineOrigin* rhs) const {
+    bool operator()(const InlineOrigin* lhs, const InlineOrigin* rhs) const {
-      if (lhs->getFileID() == rhs->getFileID())
+      return lhs->name < rhs->name;
-        return lhs->name < rhs->name;
-      return lhs->getFileID() < rhs->getFileID();
     }
   };