Fix a regression in macOS-style path remapping.

[aprantl]

Summary

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.

[JDevlieghere]

Sorry for breaking this functionality & thank you for fixing it!

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c

-void stop() {}
+void relative();
 
 int main()
 {
-  stop();
-  // Hello World!
+  relative();
+  // Hello Absolute!
   return 0;
 }

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

+void stop() {}
+void relative() {
+  stop();
+  // Hello Relative!
+}

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile

 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

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py

class TestDSYMSourcePathRemapping(lldbtest.TestBase):
 
     def build(self):
         botdir = self.getBuildArtifact('buildbot')
         userdir = self.getBuildArtifact('user')
         inputs = self.getSourcePath('Inputs')
         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))
 
         super(TestDSYMSourcePathRemapping, self).build()
 
         # Remove the build sources.
         self.assertTrue(os.path.isdir(botdir))
         shutil.rmtree(botdir)
with open(plist, 'w') as f:
       f.write('  </dict>\n')
       f.write('</dict>\n')
       f.write('</plist>\n')
 
 
     @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"])

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

ParseLLVMLineTable(lldb_private::DWARFContext &context,
   if (!line_table) {
     LLDB_LOG_ERROR(log, line_table.takeError(),
                    "SymbolFileDWARF::ParseLineTable failed to parse");
     return nullptr;
   }
   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,
     llvm::StringRef compile_dir = {}, FileSpec first_file = {}) {
   FileSpecList support_files;
   support_files.Append(first_file);
 
   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);
   }
 
   return support_files;
 }
 
 FileSpecList SymbolFileDWARF::GetSymlinkPaths() {
   return GetGlobalPluginProperties()->GetSymLinkPaths();