Introduce a new DWARFContext::getInliningInfoForAddress API to expose pointers to strings stored in DWARF file.

[danielcdh]

Summary

With the current getInliningInfoForAddress API, the FileName and FunctioName are both stored as std::string. This is not efficient for batch symbolization job as it incurs too much redundant string copy operation. This patch introduces a new API to allow user to specify customized inserter so that user can build a container that directly reference strings in the DWARF file to avoid string copy, which will speeds up batch symbolization by 2X.

[danielcdh]

set initial value for local variables.

[dblaikie]

include/llvm/DebugInfo/DWARF/DWARFContext.h

205

I'd expect to see changes to DIContext to add this as a virtual function there - and, say, changes to llvm-symbolizer to use this new API to exercise and benefit from the improvement?

(& actually the old version could become non-virtual & implemented in DIContext so all clients don't have to implement both - that'd involve changing the COFF implementation to implement the std::function version too, which will help exercise/demonstrate that this API choice is generically beneficial to both formats, potentially)

207

You can use llvm::function_ref here (it's like std::function, but doesn't have the overhead of separate allocation - used for when the functor doesn't need to be copied to outlive its scope)

207

Is StringRef an option, rather than const char*? (especially if the length is already known due to previous string ops, perhaps)

207–208

Might be worth using a struct rather than 8 arguments, for readability?

lib/DebugInfo/DWARF/DWARFContext.cpp

560

I'd probably rename this from "Inserter" (& probably update the comment) - it doesn't have to insert anything into anything. "Visitor", "Callback", "FrameHandler"?

lib/DebugInfo/DWARF/DWARFDebugLine.cpp

707

Pushing the use case through LLVM's symbolizer might also remove the need for keeping this function, maybe? Perhaps not.

[aprantl]

include/llvm/DebugInfo/DWARF/DWARFContext.h

208

I'm not a fan of this type. Seems awfully easy to confuse the arguments. Since this function is only used in one place, do we even have to generalize it with std::function argument?

include/llvm/DebugInfo/DWARF/DWARFContext.h

public:
 
   /// Get a pointer to a parsed line table corresponding to a compile unit.
   const DWARFDebugLine::LineTable *getLineTableForUnit(DWARFUnit *cu);
 
   DILineInfo getLineInfoForAddress(uint64_t Address,
       DILineInfoSpecifier Specifier = DILineInfoSpecifier()) override;
   DILineInfoTable getLineInfoForAddressRange(uint64_t Address, uint64_t Size,
       DILineInfoSpecifier Specifier = DILineInfoSpecifier()) override;
+  /// Gets the inline stack of \p Address by calling \p Inserter on all levels
+  /// of the inline stack.
+  void getInliningInfoForAddress(
+      uint64_t Address, DILineInfoSpecifier Spec,
+      std::function<void(const char *, const char *, const char *, const char *,
+                         uint32_t, uint32_t, uint32_t, uint32_t)>
+          Inserter);
   DIInliningInfo getInliningInfoForAddress(uint64_t Address,
       DILineInfoSpecifier Specifier = DILineInfoSpecifier()) override;
 
   virtual bool isLittleEndian() const = 0;
   virtual uint8_t getAddressSize() const = 0;
   virtual const DWARFSection &getInfoSection() = 0;
   typedef MapVector<object::SectionRef, DWARFSection,
                     std::map<object::SectionRef, unsigned>> TypeSectionMap;

include/llvm/DebugInfo/DWARF/DWARFDebugLine.h

struct Sequence {
     bool containsPC(uint64_t pc) const {
       return (LowPC <= pc && pc < HighPC);
     }
   };
 
   struct LineTable {
     LineTable();
 
+    // Returns the file path string with the given \p CompDir, \p IncludeDir
+    // and \p FileName, for the given \p Kind.
+    static std::string getFilePath(const char *CompDir, const char *IncludeDir,
+                                   const char *FileName,
+                                   DILineInfoSpecifier::FileLineInfoKind Kind);
+
     // Represents an invalid row
     const uint32_t UnknownRowIndex = UINT32_MAX;
 
     void appendRow(const DWARFDebugLine::Row &R) {
       Rows.push_back(R);
     }
 
     void appendSequence(const DWARFDebugLine::Sequence &S) {
       Sequences.push_back(S);
     }
 
     // Returns the index of the row with file/line info for a given address,
     // or UnknownRowIndex if there is no such row.
     uint32_t lookupAddress(uint64_t address) const;
 
     bool lookupAddressRange(uint64_t address, uint64_t size,
                             std::vector<uint32_t> &result) const;
 
     bool hasFileAtIndex(uint64_t FileIndex) const;
 
+    // Extracts \p IncludeDir and \p FileName by its index in filename table
+    // in prologue. Returns true on success.
+    bool getFileNameByIndex(uint64_t FileIndex, const char *&IncludeDir,
+                            const char *&FileName) const;
+
     // Extracts filename by its index in filename table in prologue.
     // Returns true on success.
     bool getFileNameByIndex(uint64_t FileIndex, const char *CompDir,
                             DILineInfoSpecifier::FileLineInfoKind Kind,
                             std::string &Result) const;
 
+    // Fills the \p IncludeDir, \p FileName, \p Line, \p Column and
+    // \p Discriminator with the given \p Address. Returns true on success.
+    bool getFileLineInfoForAddress(uint64_t Address, const char *&IncludeDir,
+                                   const char *&FileName, uint32_t &Line,
+                                   uint32_t &Column,
+                                   uint32_t &Discriminator) const;
+
     // Fills the Result argument with the file and line information
     // corresponding to Address. Returns true on success.
     bool getFileLineInfoForAddress(uint64_t Address, const char *CompDir,
                                    DILineInfoSpecifier::FileLineInfoKind Kind,
                                    DILineInfo &Result) const;
 
     void dump(raw_ostream &OS) const;
     void clear();

lib/DebugInfo/DWARF/DWARFContext.cpp

for (uint32_t RowIndex : RowVector) {
     Result.Column = Row.Column;
     Result.StartLine = StartLine;
     Lines.push_back(std::make_pair(Row.Address, Result));
   }
 
   return Lines;
 }
 
-DIInliningInfo
-DWARFContext::getInliningInfoForAddress(uint64_t Address,
-                                        DILineInfoSpecifier Spec) {
-  DIInliningInfo InliningInfo;
-
+void DWARFContext::getInliningInfoForAddress(
+    uint64_t Address, DILineInfoSpecifier Spec,
+    std::function<void(const char *, const char *, const char *, const char *,
+                       uint32_t, uint32_t, uint32_t, uint32_t)>
+        Inserter) {
   DWARFCompileUnit *CU = getCompileUnitForAddress(Address);
   if (!CU)
-    return InliningInfo;
+    return;
 
   const DWARFLineTable *LineTable = nullptr;
   SmallVector<DWARFDie, 4> InlinedChain;
   CU->getInlinedChainForAddress(Address, InlinedChain);
   if (InlinedChain.size() == 0) {
+    if (Spec.FLIKind == FileLineInfoKind::None)
+      return;
     // If there is no DIE for address (e.g. it is in unavailable .dwo file),
     // try to at least get file/line info from symbol table.
-    if (Spec.FLIKind != FileLineInfoKind::None) {
-      DILineInfo Frame;
-      LineTable = getLineTableForUnit(CU);
-      if (LineTable &&
-          LineTable->getFileLineInfoForAddress(Address, CU->getCompilationDir(),
-                                               Spec.FLIKind, Frame))
-        InliningInfo.addFrame(Frame);
-    }
-    return InliningInfo;
+    const char *FileName = nullptr;
+    const char *IncludeDir = nullptr;
+    uint32_t Line = 0;
+    uint32_t Column = 0;
+    uint32_t Discriminator = 0;
+        LineTable->getFileLineInfoForAddress(Address, IncludeDir, FileName,
+                                             Line, Column, Discriminator))
+      Inserter(nullptr, CU->getCompilationDir(), IncludeDir, FileName, 0, Line,
+               Column, Discriminator);
+    return;
   }
 
   uint32_t CallFile = 0, CallLine = 0, CallColumn = 0, CallDiscriminator = 0;
   for (uint32_t i = 0, n = InlinedChain.size(); i != n; i++) {
     DWARFDie &FunctionDIE = InlinedChain[i];
-    DILineInfo Frame;
-    // Get function name if necessary.
-    if (const char *Name = FunctionDIE.getSubroutineName(Spec.FNKind))
-      Frame.FunctionName = Name;
-    if (auto DeclLineResult = FunctionDIE.getDeclLine())
-      Frame.StartLine = DeclLineResult;
+    const char *FunctionName = FunctionDIE.getSubroutineName(Spec.FNKind);
+    const char *FileName = nullptr;
+    const char *IncludeDir = nullptr;
+    uint32_t StartLine = FunctionDIE.getDeclLine();
+    uint32_t Line = 0;
+    uint32_t Column = 0;
+    uint32_t Discriminator = 0;
     if (Spec.FLIKind != FileLineInfoKind::None) {
       if (i == 0) {
         // For the topmost frame, initialize the line table of this
         // compile unit and fetch file/line info from it.
         LineTable = getLineTableForUnit(CU);
         // For the topmost routine, get file/line info from line table.
         if (LineTable)
-          LineTable->getFileLineInfoForAddress(Address, CU->getCompilationDir(),
-                                               Spec.FLIKind, Frame);
+          LineTable->getFileLineInfoForAddress(Address, IncludeDir, FileName,
+                                               Line, Column, Discriminator);
       } else {
         // Otherwise, use call file, call line and call column from
         // previous DIE in inlined chain.
         if (LineTable)
-          LineTable->getFileNameByIndex(CallFile, CU->getCompilationDir(),
-                                        Spec.FLIKind, Frame.FileName);
-        Frame.Line = CallLine;
-        Frame.Column = CallColumn;
-        Frame.Discriminator = CallDiscriminator;
+          LineTable->getFileNameByIndex(CallFile, IncludeDir, FileName);
+        Line = CallLine;
+        Column = CallColumn;
+        Discriminator = CallDiscriminator;
       }
       // Get call file/line/column of a current DIE.
       if (i + 1 < n) {
         FunctionDIE.getCallerFrame(CallFile, CallLine, CallColumn,
                                    CallDiscriminator);
       }
     }
-    InliningInfo.addFrame(Frame);
+    Inserter(FunctionName, CU->getCompilationDir(), IncludeDir, FileName,
+             StartLine, Line, Column, Discriminator);
   }
+}
+
+DIInliningInfo
+DWARFContext::getInliningInfoForAddress(uint64_t Address,
+                                        DILineInfoSpecifier Spec) {
+  DIInliningInfo InliningInfo;
+
+  getInliningInfoForAddress(
+      Address, Spec,
+      [&](const char *FunctionName, const char *CompDir, const char *IncludeDir,
+          const char *FileName, uint32_t StartLine, uint32_t Line,
+          uint32_t Column, uint32_t Discriminator) {
+        DILineInfo Frame;
+        if (FileName)
+          Frame.FileName = DWARFLineTable::getFilePath(
+              CompDir, IncludeDir, FileName, Spec.FLIKind);
+        if (FunctionName)
+          Frame.FunctionName = FunctionName;
+        Frame.Line = Line;
+        Frame.Column = Column;
+        Frame.StartLine = StartLine;
+        Frame.Discriminator = Discriminator;
+        InliningInfo.addFrame(Frame);
+      });
   return InliningInfo;
 }
 
 static Error createError(const Twine &Reason, llvm::Error E) {
   return make_error<StringError>(Reason + toString(std::move(E)),
                                  inconvertibleErrorCode());
 }
 

lib/DebugInfo/DWARF/DWARFDebugLine.cpp

bool DWARFDebugLine::LineTable::lookupAddressRange(
   return true;
 }
 
 bool
 DWARFDebugLine::LineTable::hasFileAtIndex(uint64_t FileIndex) const {
   return FileIndex != 0 && FileIndex <= Prologue.FileNames.size();
 }
 
-bool
-DWARFDebugLine::LineTable::getFileNameByIndex(uint64_t FileIndex,
-                                              const char *CompDir,
-                                              FileLineInfoKind Kind,
-                                              std::string &Result) const {
-  if (Kind == FileLineInfoKind::None || !hasFileAtIndex(FileIndex))
-    return false;
-  const FileNameEntry &Entry = Prologue.FileNames[FileIndex - 1];
-  const char *FileName = Entry.Name;
+std::string DWARFDebugLine::LineTable::getFilePath(const char *CompDir,
+                                                   const char *IncludeDir,
+                                                   const char *FileName,
+                                                   FileLineInfoKind Kind) {
+  if (FileName == nullptr)
+    return "";
   if (Kind != FileLineInfoKind::AbsoluteFilePath ||
       sys::path::is_absolute(FileName)) {
-    Result = FileName;
-    return true;
+    return FileName;
   }
 
   SmallString<16> FilePath;
-  uint64_t IncludeDirIndex = Entry.DirIdx;
-  const char *IncludeDir = "";
-  // Be defensive about the contents of Entry.
-  if (IncludeDirIndex > 0 &&
-      IncludeDirIndex <= Prologue.IncludeDirectories.size())
-    IncludeDir = Prologue.IncludeDirectories[IncludeDirIndex - 1];
 
   // We may still need to append compilation directory of compile unit.
   // We know that FileName is not absolute, the only way to have an
   // absolute path at this point would be if IncludeDir is absolute.
-  if (CompDir && Kind == FileLineInfoKind::AbsoluteFilePath &&
-      sys::path::is_relative(IncludeDir))
+  if (CompDir && sys::path::is_relative(IncludeDir))
     sys::path::append(FilePath, CompDir);
 
   // sys::path::append skips empty strings.
   sys::path::append(FilePath, IncludeDir, FileName);
-  Result = FilePath.str();
+  return FilePath.str();
+}
+
+bool DWARFDebugLine::LineTable::getFileNameByIndex(
+    uint64_t FileIndex, const char *&IncludeDir, const char *&FileName) const {
+  if (!hasFileAtIndex(FileIndex))
+    return false;
+  const FileNameEntry &Entry = Prologue.FileNames[FileIndex - 1];
+  FileName = Entry.Name;
+  if (Entry.DirIdx > 0 && Entry.DirIdx <= Prologue.IncludeDirectories.size())
+    IncludeDir = Prologue.IncludeDirectories[Entry.DirIdx - 1];
+  else
+    IncludeDir = "";
+  return true;
+}
+
+bool DWARFDebugLine::LineTable::getFileNameByIndex(uint64_t FileIndex,
+                                                   const char *CompDir,
+                                                   FileLineInfoKind Kind,
+                                                   std::string &Result) const {
+  if (Kind == FileLineInfoKind::None)
+    return false;
+  const char *FileName = nullptr;
+  const char *IncludeDir = nullptr;
+  if (!getFileNameByIndex(FileIndex, IncludeDir, FileName))
+    return false;
+  Result = getFilePath(CompDir, IncludeDir, FileName, Kind);
   return true;
 }
 
 bool DWARFDebugLine::LineTable::getFileLineInfoForAddress(
-    uint64_t Address, const char *CompDir, FileLineInfoKind Kind,
-    DILineInfo &Result) const {
+    uint64_t Address, const char *&IncludeDir, const char *&FileName,
+    uint32_t &Line, uint32_t &Column, uint32_t &Discriminator) const {
   // Get the index of row we're looking for in the line table.
   uint32_t RowIndex = lookupAddress(Address);
   if (RowIndex == -1U)
     return false;
   // Take file number and line/column from the row.
   const auto &Row = Rows[RowIndex];
-  if (!getFileNameByIndex(Row.File, CompDir, Kind, Result.FileName))
+  Line = Row.Line;
+  Column = Row.Column;
+  Discriminator = Row.Discriminator;
+  return getFileNameByIndex(Row.File, IncludeDir, FileName);
+}
+
+bool DWARFDebugLine::LineTable::getFileLineInfoForAddress(
+    uint64_t Address, const char *CompDir, FileLineInfoKind Kind,
+    DILineInfo &Result) const {
+  const char *FileName = nullptr;
+  const char *IncludeDir = nullptr;
+  if (!getFileLineInfoForAddress(Address, IncludeDir, FileName, Result.Line,
+                                 Result.Column, Result.Discriminator))
     return false;
-  Result.Line = Row.Line;
-  Result.Column = Row.Column;
-  Result.Discriminator = Row.Discriminator;
+  Result.FileName = getFilePath(CompDir, IncludeDir, FileName, Kind);
   return true;
 }