[PATCH] [dwarfdump] Implement extraction of file information referenced in .debug_info.

[David Blaikie]

================
Comment at: lib/DebugInfo/DWARFContext.cpp:458
@@ -498,2 +457,3 @@
   if (Spec.FLIKind != FileLineInfoKind::None) {
     const DWARFLineTable *LineTable = getLineTableForUnit(CU);
+    if (LineTable)
----------------
you could roll this declaration into the if condition below (though I'm still trying to figure out if this is the changes below are the right API direction)

================
Comment at: lib/DebugInfo/DWARFDebugInfoEntry.cpp:94
@@ -90,3 +93,3 @@
   const char *Name = nullptr;
   if (Optional<uint64_t> Val = formValue.getAsUnsignedConstant())
     Name = AttributeValueString(attr, *Val);
----------------
Presumably this could be an "else" to the new if you're introducing at 97 - since they are mutually exclusive (a decl_file/decl_line will never be decoded by AttributeValueString - so you might as well not call it & avoid all the switching, etc)

================
Comment at: lib/DebugInfo/DWARFDebugInfoEntry.cpp:98
@@ +97,3 @@
+  if (attr == DW_AT_decl_file || attr == DW_AT_call_file) {
+    typedef DILineInfoSpecifier::FileLineInfoKind FileLineInfoKind;
+    const DWARFDebugLine::LineTable *LT;
----------------
I probably wouldn't worry about the typedef for just one usage

================
Comment at: lib/DebugInfo/DWARFDebugInfoEntry.cpp:99
@@ +98,3 @@
+    typedef DILineInfoSpecifier::FileLineInfoKind FileLineInfoKind;
+    const DWARFDebugLine::LineTable *LT;
+    std::string File;
----------------
This variable could be rolled into an if condition down on line 103:

if (auto *LT = ... getLineTableForUnit(...))
  if (LT->getFileNameByIndex(...))
    File = '"' + ...

================
Comment at: lib/DebugInfo/DWARFDebugInfoEntry.cpp:104
@@ +103,3 @@
+    if (LT &&
+        LT->getFileNameByIndex(formValue.getAsUnsignedConstant().getValue(),
+                               FileLineInfoKind::AbsoluteFilePath, File, u)) {
----------------
Do you have test coverage for the situation where the file name is not found in the line table? (testing the fallback to the old behavior)

================
Comment at: lib/DebugInfo/DWARFDebugInfoEntry.cpp:106
@@ +105,3 @@
+                               FileLineInfoKind::AbsoluteFilePath, File, u)) {
+      File = '"' + File + '"';
+      Name = File.c_str();
----------------
Just for fun you could use std::move on the RHS here:

  File = '"' + std::move(File) + '"';

to possibly reuse the storage.

================
Comment at: lib/DebugInfo/DWARFDebugInfoEntry.cpp:107
@@ +106,3 @@
+      File = '"' + File + '"';
+      Name = File.c_str();
+    }
----------------
this pointer will be dangling when used on line 111, outside the scope of the 'File' variable

================
Comment at: lib/DebugInfo/DWARFDebugLine.cpp:674
@@ +673,3 @@
+
+  if (U && Kind == FileLineInfoKind::AbsoluteFilePath &&
+      sys::path::is_relative(FilePath.str())) {
----------------
test coverage for this? Some absolute and relative paths? (hopefully you can do this with just some hardcoded dwarf (not sure is is_relative will properly return false for "/foo/bar" on a Windows machine, though)

================
Comment at: lib/DebugInfo/DWARFDebugLine.h:186
@@ -183,2 +185,3 @@
                             DILineInfoSpecifier::FileLineInfoKind Kind,
-                            std::string &Result) const;
+                            std::string &Result, DWARFUnit *U = nullptr) const;
+
----------------
Moving these two functions may be better as a separate patch.

http://reviews.llvm.org/D5192