Turn local DWARFContext helpers getFileNameForUnit() and getFileLineInfoForCompileUnit() into full-blowm DWARFDebugLine::LineTable methods.

[friss]

Summary

getFileNameForUnit() is basically a wrapper around LineTable::getFileNameByIndex().
Fold its additional functionality (adding the DWARFUnit compilation dir) into
LineTable::getFileNameByIndex().

getFileLineInfoForCompileUnit() is a wrapper around getFileNameForUnit(). As
a function to search the line information by address, it seems natural to put
it in the LineTable also.

Before this commit only the Context with its private helpers could do Linetable
lookups. This newly exposed feature will be used by the DIE dumping code to
get access to file information referenced in DIE attributes.

This commit has already been partly reviewed in D5192 and contained an
additional and a bit controversial 'realpath' call that is left out of this
patch. We can reinstate that realpath code later if it is desirable.

[samsonov]

Can you pass just a compilation dir instead of DWARFUnit* into a DWARFDebugLine methods? For now we don't need any other information from unit, and I'm not sure we will - line table should be more or less self-contained.

lib/DebugInfo/DWARFDebugLine.cpp

663

Looks like you can have a single SmallString<> to construct an absolute filepath.

693

It's not clear now why CU must be nonzero, even though getFileNameByIndex allows U to be nullptr.

lib/DebugInfo/DWARFDebugLine.h

184

It's somewhat confusing to have output parameter not the last parameter in the function. See my suggestion about passing compilation dir instead of DWARFUnit* - in this case you can just pass empty string if unit is unavailable.

[friss]

lib/DebugInfo/DWARFDebugLine.cpp

663

You mean removing the AbsolutePath variable in the is_relative code bellow right?

693

This is just how the original code is written, this patch is supposed to have no functional change. I agree with you that this looks strange, and maybe this check should be extracted and moved to the current callers.

lib/DebugInfo/DWARFDebugLine.h

184

Agreed. I mechanically add new parameters with a default value to the end to avoid having to modify the existing callers, but here it looks strange.
I'd rather pass a raw const char * to avoid creating unnecessary string objects though.

[samsonov]

lib/DebugInfo/DWARFDebugLine.cpp

663

Yes.

[friss]

As per review comments:

• Remove DWARFUnit * parameters to LineTable methods. Pass just the compilation dir instead.
• Remove useless temporaries in getFileNameByIndex
• Change the order of getFileNameByIndex arguments to be more natural

With above changes, the name getFileLineInfoForCompilationUnit didn't make any sense anymore (no more CU argument). Rename the function to getFileLineInfoForAddress instead.

[samsonov]

lib/DebugInfo/DWARFDebugLine.cpp

11

And you don't need these headers.

13

and this config.h file.

666

You don't need this extra variable.

679

Just call sys::part::append(FilePath, CompDir) here.

699

you may use "const auto& " here.

lib/DebugInfo/DWARFDebugLine.h

23

Now you don't need these forward declarations.

177

Looks like we can make this a private method now (in a separate change, after this one goes in).

193

Add a brief comment for this function.

[friss]

• remove now useless includes and forward declarations.
• add comment to new member function
• code changes as per review comments

[samsonov]

LGTM

[friss]

Closed by commit rL218125 (authored by @friss).

lib/DebugInfo/DWARFContext.cpp

 
 DWARFCompileUnit *DWARFContext::getCompileUnitForAddress(uint64_t Address) {
   // First, get the offset of the compile unit.
   uint32_t CUOffset = getDebugAranges()->findAddress(Address);
   // Retrieve the compile unit.
   return getCompileUnitForOffset(CUOffset);
 }
 
-static bool getFileNameForUnit(DWARFCompileUnit *U,
-                               const DWARFLineTable *LineTable,
-                               uint64_t FileIndex, FileLineInfoKind Kind,
-                               std::string &FileName) {
-  if (!U || !LineTable || Kind == FileLineInfoKind::None ||
-      !LineTable->getFileNameByIndex(FileIndex, Kind, FileName))
-    return false;
-  if (Kind == FileLineInfoKind::AbsoluteFilePath &&
-      sys::path::is_relative(FileName)) {
-    // We may still need to append compilation directory of compile unit.
-    SmallString<16> AbsolutePath;
-    if (const char *CompilationDir = U->getCompilationDir()) {
-      sys::path::append(AbsolutePath, CompilationDir);
-    }
-    sys::path::append(AbsolutePath, FileName);
-    FileName = AbsolutePath.str();
-  }
-  return true;
-}
-
-static bool getFileLineInfoForCompileUnit(DWARFCompileUnit *CU,
-                                          const DWARFLineTable *LineTable,
-                                          uint64_t Address,
-                                          FileLineInfoKind Kind,
-                                          DILineInfo &Result) {
-  if (!CU || !LineTable)
-    return false;
-  // Get the index of row we're looking for in the line table.
-  uint32_t RowIndex = LineTable->lookupAddress(Address);
-  if (RowIndex == -1U)
-    return false;
-  // Take file number and line/column from the row.
-  const DWARFDebugLine::Row &Row = LineTable->Rows[RowIndex];
-  if (!getFileNameForUnit(CU, LineTable, Row.File, Kind,
-                          Result.FileName))
-    return false;
-  Result.Line = Row.Line;
-  Result.Column = Row.Column;
-  return true;
-}
-
 static bool getFunctionNameForAddress(DWARFCompileUnit *CU, uint64_t Address,
                                       FunctionNameKind Kind,
                                       std::string &FunctionName) {
   if (Kind == FunctionNameKind::None)
     return false;
   // The address may correspond to instruction in some inlined function,
   // so we have to build the chain of inlined functions and take the
   // name of the topmost function in it.
DILineInfo DWARFContext::getLineInfoForAddress(uint64_t Address,
                                                DILineInfoSpecifier Spec) {
   DILineInfo Result;
 
   DWARFCompileUnit *CU = getCompileUnitForAddress(Address);
   if (!CU)
     return Result;
   getFunctionNameForAddress(CU, Address, Spec.FNKind, Result.FunctionName);
   if (Spec.FLIKind != FileLineInfoKind::None) {
-    const DWARFLineTable *LineTable = getLineTableForUnit(CU);
-    getFileLineInfoForCompileUnit(CU, LineTable, Address, Spec.FLIKind, Result);
+    if (const DWARFLineTable *LineTable = getLineTableForUnit(CU))
+      LineTable->getFileLineInfoForCompileUnit(CU, Address, Spec.FLIKind, Result);
   }
   return Result;
 }
 
 DILineInfoTable
 DWARFContext::getLineInfoForAddressRange(uint64_t Address, uint64_t Size,
                                          DILineInfoSpecifier Spec) {
   DILineInfoTable  Lines;
DWARFContext::getLineInfoForAddressRange(uint64_t Address, uint64_t Size,
   std::vector<uint32_t> RowVector;
   if (!LineTable->lookupAddressRange(Address, Size, RowVector))
     return Lines;
 
   for (uint32_t RowIndex : RowVector) {
     // Take file number and line/column from the row.
     const DWARFDebugLine::Row &Row = LineTable->Rows[RowIndex];
     DILineInfo Result;
-    getFileNameForUnit(CU, LineTable, Row.File, Spec.FLIKind,
-                       Result.FileName);
+    LineTable->getFileNameByIndex(Row.File, Spec.FLIKind, Result.FileName, CU);
     Result.FunctionName = FunctionName;
     Result.Line = Row.Line;
     Result.Column = Row.Column;
     Lines.push_back(std::make_pair(Row.Address, Result));
   }
 
   return Lines;
 }
DWARFContext::getInliningInfoForAddress(uint64_t Address,
   const DWARFDebugInfoEntryInlinedChain &InlinedChain =
       CU->getInlinedChainForAddress(Address);
   if (InlinedChain.DIEs.size() == 0) {
     // 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 (getFileLineInfoForCompileUnit(CU, LineTable, Address, Spec.FLIKind,
-                                        Frame)) {
+      if (LineTable &&
+          LineTable->getFileLineInfoForCompileUnit(CU, Address, Spec.FLIKind,
+                                                   Frame))
         InliningInfo.addFrame(Frame);
-      }
-    }
     return InliningInfo;
   }
 
   uint32_t CallFile = 0, CallLine = 0, CallColumn = 0;
   for (uint32_t i = 0, n = InlinedChain.DIEs.size(); i != n; i++) {
     const DWARFDebugInfoEntryMinimal &FunctionDIE = InlinedChain.DIEs[i];
     DILineInfo Frame;
     // Get function name if necessary.
     if (const char *Name =
             FunctionDIE.getSubroutineName(InlinedChain.U, Spec.FNKind))
       Frame.FunctionName = Name;
     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.
-        getFileLineInfoForCompileUnit(CU, LineTable, Address, Spec.FLIKind,
+        if (LineTable)
+          LineTable->getFileLineInfoForCompileUnit(CU, Address, Spec.FLIKind,
                                                    Frame);
       } else {
         // Otherwise, use call file, call line and call column from
         // previous DIE in inlined chain.
-        getFileNameForUnit(CU, LineTable, CallFile, Spec.FLIKind,
-                           Frame.FileName);
+        if (LineTable)
+          LineTable->getFileNameByIndex(CallFile, Spec.FLIKind,
+                                        Frame.FileName, CU);
         Frame.Line = CallLine;
         Frame.Column = CallColumn;
       }
       // Get call file/line/column of a current DIE.
       if (i + 1 < n) {
         FunctionDIE.getCallerFrame(InlinedChain.U, CallFile, CallLine,
                                    CallColumn);
       }

lib/DebugInfo/DWARFDebugLine.h

 #include "llvm/Support/DataExtractor.h"
 #include <map>
 #include <string>
 #include <vector>
 
 namespace llvm {
 
 class raw_ostream;
+class DWARFUnit;
+class DWARFCompileUnit;
 
 class DWARFDebugLine {
 public:
   DWARFDebugLine(const RelocAddrMap* LineInfoRelocMap) : RelocMap(LineInfoRelocMap) {}
   struct FileNameEntry {
     FileNameEntry() : Name(nullptr), DirIdx(0), ModTime(0), Length(0) {}
 
     const char *Name;
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 -1 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;
 
     // Extracts filename by its index in filename table in prologue.
     // Returns true on success.
     bool getFileNameByIndex(uint64_t FileIndex,
                             DILineInfoSpecifier::FileLineInfoKind Kind,
-                            std::string &Result) const;
+                            std::string &Result, DWARFUnit *U = nullptr) const;
+
+    bool
+    getFileLineInfoForCompileUnit(DWARFCompileUnit *CU, uint64_t Address,
+                                  DILineInfoSpecifier::FileLineInfoKind Kind,
+                                  DILineInfo &Result) const;
 
     void dump(raw_ostream &OS) const;
     void clear();
 
     /// Parse prologue and all rows.
     bool parse(DataExtractor debug_line_data, const RelocAddrMap *RMap,
                uint32_t *offset_ptr);
 
     struct Prologue Prologue;
     typedef std::vector<Row> RowVector;

lib/DebugInfo/DWARFDebugLine.cpp

 //===-- DWARFDebugLine.cpp ------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "DWARFDebugLine.h"
+#include "DWARFCompileUnit.h"
+#include "DWARFUnit.h"
+#include "llvm/Config/config.h"
 #include "llvm/Support/Dwarf.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 using namespace llvm;
 using namespace dwarf;
 typedef DILineInfoSpecifier::FileLineInfoKind FileLineInfoKind;
bool DWARFDebugLine::LineTable::lookupAddressRange(
   }
 
   return true;
 }
 
 bool
 DWARFDebugLine::LineTable::getFileNameByIndex(uint64_t FileIndex,
                                               FileLineInfoKind Kind,
-                                              std::string &Result) const {
+                                              std::string &Result,
+                                              DWARFUnit *U) const {
   if (FileIndex == 0 || FileIndex > Prologue.FileNames.size() ||
       Kind == FileLineInfoKind::None)
     return false;
   const FileNameEntry &Entry = Prologue.FileNames[FileIndex - 1];
   const char *FileName = Entry.Name;
   if (Kind != FileLineInfoKind::AbsoluteFilePath ||
       sys::path::is_absolute(FileName)) {
     Result = FileName;
     return true;
   }
   SmallString<16> FilePath;
   uint64_t IncludeDirIndex = Entry.DirIdx;
   // Be defensive about the contents of Entry.
   if (IncludeDirIndex > 0 &&
       IncludeDirIndex <= Prologue.IncludeDirectories.size()) {
     const char *IncludeDir = Prologue.IncludeDirectories[IncludeDirIndex - 1];
     sys::path::append(FilePath, IncludeDir);
   }
   sys::path::append(FilePath, FileName);
+
+  if (U && Kind == FileLineInfoKind::AbsoluteFilePath &&
+      sys::path::is_relative(FilePath.str())) {
+    // We may still need to append compilation directory of compile unit.
+    SmallString<16> AbsolutePath;
+    if (const char *CompilationDir = U->getCompilationDir()) {
+      sys::path::append(AbsolutePath, CompilationDir);
+    }
+    sys::path::append(AbsolutePath, FilePath.str());
+    FilePath = AbsolutePath.str();
+  }
+
   Result = FilePath.str();
   return true;
 }
+
+bool
+DWARFDebugLine::LineTable::getFileLineInfoForCompileUnit(DWARFCompileUnit *CU,
+                                                         uint64_t Address,
+                                                         FileLineInfoKind Kind,
+                                                         DILineInfo &Result) const {
+  if (!CU)
+    return false;
+  // 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 DWARFDebugLine::Row &Row = Rows[RowIndex];
+  if (!getFileNameByIndex(Row.File, Kind, Result.FileName, CU))
+    return false;
+  Result.Line = Row.Line;
+  Result.Column = Row.Column;
+  return true;
+}