[Lldb-commits] [lldb] da83e96 - Fix a regression in macOS-style path remapping.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-11-11T12:21:38-08:00
New Revision: da83e96273527a137f2ebd77cedb920180eab621

URL: https://github.com/llvm/llvm-project/commit/da83e96273527a137f2ebd77cedb920180eab621
DIFF: https://github.com/llvm/llvm-project/commit/da83e96273527a137f2ebd77cedb920180eab621.diff

LOG: Fix a regression in macOS-style path remapping.

When we switched to the LLVM .debug_line parser, the .dSYM-style path
remapping logic stopped working for relative paths because of how
RemapSourceFile silently fails for relative paths. This patch both
makes the code more readable and fixes this particular bug.

One interesting thing I learned is that Module::RemapSourceFile() is a
macOS-only code path that operates on on the lldb::Module level and is
completely separate from target.source-map, which operates on a
per-Target level.

Differential Revision: https://reviews.llvm.org/D70037

rdar://problem/56924558

Added: 
    lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c

Modified: 
    lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
    lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
    lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
index 556bda3c17d1..41a6a46c9261 100644
--- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
+++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
@@ -1,8 +1,8 @@
-void stop() {}
+void relative();
 
 int main()
 {
-  stop();
-  // Hello World!
+  relative();
+  // Hello Absolute!
   return 0;
 }

diff  --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c
new file mode 100644
index 000000000000..02331834cf21
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c
@@ -0,0 +1,5 @@
+void stop() {}
+void relative() {
+  stop();
+  // Hello Relative!
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
index f36a8dc1e671..8c82c73b13fc 100644
--- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
+++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
@@ -1,5 +1,10 @@
 BOTDIR = $(BUILDDIR)/buildbot
 USERDIR = $(BUILDDIR)/user
 C_SOURCES = $(BOTDIR)/main.c
+LD_EXTRAS = $(BOTDIR)/relative.o
 
 include Makefile.rules
+
+$(EXE): relative.o
+relative.o: $(BOTDIR)/relative.c
+	cd $(BOTDIR) && $(CC) -c $(CFLAGS) -o $@ relative.c

diff  --git a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
index d13a04748672..2ee379150392 100644
--- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
+++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
@@ -17,7 +17,7 @@ def build(self):
         lldbutil.mkdir_p(botdir)
         lldbutil.mkdir_p(userdir)
         import shutil
-        for f in ['main.c']:
+        for f in ['main.c', 'relative.c']:
             shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f))
             shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, f))
 
@@ -52,5 +52,10 @@ def build(self):
     @skipIf(debug_info=no_match("dsym"))
     def test(self):
         self.build()
-        lldbutil.run_to_name_breakpoint(self, 'main')
-        self.expect("source list", substrs=["Hello World"])
+        
+        target, process, _, _ = lldbutil.run_to_name_breakpoint(
+            self, 'main')
+        self.expect("source list -n main", substrs=["Hello Absolute"])
+        bkpt = target.BreakpointCreateByName('relative')
+        lldbutil.continue_to_breakpoint(process, bkpt)
+        self.expect("source list -n relative", substrs=["Hello Relative"])

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index c982d59c2830..f43537d20200 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -179,6 +179,23 @@ ParseLLVMLineTable(lldb_private::DWARFContext &context,
   return *line_table;
 }
 
+static llvm::Optional<std::string>
+GetFileByIndex(const llvm::DWARFDebugLine::Prologue &prologue, size_t idx,
+               llvm::StringRef compile_dir, FileSpec::Style style) {
+  // Try to get an absolute path first.
+  std::string abs_path;
+  auto absolute = llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath;
+  if (prologue.getFileNameByIndex(idx, compile_dir, absolute, abs_path, style))
+    return std::move(abs_path);
+
+  // Otherwise ask for a relative path.
+  std::string rel_path;
+  auto relative = llvm::DILineInfoSpecifier::FileLineInfoKind::Default;
+  if (!prologue.getFileNameByIndex(idx, compile_dir, relative, rel_path, style))
+    return {};
+  return std::move(rel_path);
+}
+
 static FileSpecList ParseSupportFilesFromPrologue(
     const lldb::ModuleSP &module,
     const llvm::DWARFDebugLine::Prologue &prologue, FileSpec::Style style,
@@ -188,27 +205,12 @@ static FileSpecList ParseSupportFilesFromPrologue(
 
   const size_t number_of_files = prologue.FileNames.size();
   for (size_t idx = 1; idx <= number_of_files; ++idx) {
-    std::string original_file;
-    if (!prologue.getFileNameByIndex(
-            idx, compile_dir,
-            llvm::DILineInfoSpecifier::FileLineInfoKind::Default, original_file,
-            style)) {
-      // Always add an entry so the indexes remain correct.
-      support_files.EmplaceBack();
-      continue;
-    }
-
     std::string remapped_file;
-    if (!prologue.getFileNameByIndex(
-            idx, compile_dir,
-            llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath,
-            remapped_file, style)) {
-      // Always add an entry so the indexes remain correct.
-      support_files.EmplaceBack(original_file, style);
-      continue;
-    }
+    if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style))
+      if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file))
+        remapped_file = std::move(*file_path);
 
-    module->RemapSourceFile(llvm::StringRef(original_file), remapped_file);
+    // Unconditionally add an entry, so the indices match up.
     support_files.EmplaceBack(remapped_file, style);
   }