[PDB] Better support for posix style paths in PDBs.

[zturner]

Summary

While it doesn't make a *ton* of sense for POSIX paths to be
in PDBs, it's possible to occur in real scenarios involving
cross compilation.

The tools need to be able to handle this, because certain types
of debugging scenarios are possible without a running process
and so don't necessarily require you to be on a Windows system.
These include post-mortem debugging and binary forensics (e.g.
using a debugger to disassemble functions and examine symbols
without running the process).

There's changes in clang, LLD, and lldb in this patch.  After
this the cross-platform disassembly and source-list tests pass
on Linux.

[stella.stamenova]

A couple of comments, but looks good otherwise.

lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp

156

This looks unused

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

533

Is it possible that it would start with a letter? Like D:\ etc...

[rnk]

I agree with the direction, but please update the commit message to make it clear what the new expected behavior of our tools is. Basically, they should generate native paths unless we have command line flags telling us to do something else.

lld/COFF/PDB.cpp

221–239

Please clean up these comments to make it clear that we have a new, simple rule for what we're doing: Creating absolute paths that work on the linker's host filesystem. If you want any other kind of behavior, the user should use /pdbsourcepath:.

lld/COFF/PDB.cpp

public:
   DebugSHandler(PDBLinker &Linker, ObjFile &File, const CVIndexMap &IndexMap)
       : Linker(Linker), File(File), IndexMap(IndexMap) {}
 
   void handleDebugS(lld::coff::SectionChunk &DebugS);
   void finish();
 };
 }
 
 // Visual Studio's debugger requires absolute paths in various places in the
 // PDB to work without additional configuration:
 // https://docs.microsoft.com/en-us/visualstudio/debugger/debug-source-files-common-properties-solution-property-pages-dialog-box
 static void pdbMakeAbsolute(SmallVectorImpl<char> &FileName) {
-  if (sys::path::is_absolute(FileName, sys::path::Style::windows))
+  if (sys::path::is_absolute(FileName))
     return;
   if (Config->PDBSourcePath.empty()) {
     // Debuggers generally want that PDB files contain absolute, Windows-style
     // paths. On POSIX hosts, this here will produce an absolute POSIX-style
     // path, which is weird -- but it's not clear what else to do.
     // People doing cross builds should probably just always pass
     // /pbdsourcepath: and make sure paths to input obj files and to lld-link
     // itself are relative.
     sys::fs::make_absolute(FileName);
     return;
   }
   // Using /pdbsourcepath: with absolute POSIX paths will prepend
   // PDBSourcePath to the absolute POSIX path. Since absolute POSIX paths
   // don't make sense in PDB files anyways, this is gargabe-in-garbage-out.
   SmallString<128> AbsoluteFileName = Config->PDBSourcePath;
-  sys::path::append(AbsoluteFileName, sys::path::Style::windows, FileName);
-  sys::path::native(AbsoluteFileName, sys::path::Style::windows);
-  sys::path::remove_dots(AbsoluteFileName, /*remove_dot_dots=*/true,
-                         sys::path::Style::windows);
+  sys::path::append(AbsoluteFileName, FileName);
+  sys::path::native(AbsoluteFileName);
+  sys::path::remove_dots(AbsoluteFileName, /*remove_dot_dots=*/true);
   FileName = std::move(AbsoluteFileName);
 }
 
 static SectionChunk *findByName(ArrayRef<SectionChunk *> Sections,
                                 StringRef Name) {
   for (SectionChunk *C : Sections)
     if (C->getSectionName() == Name)
       return C;
PDBLinker::maybeMergeTypeServerPDB(ObjFile *File, TypeServer2Record &TS) {
   // 1. The given file path
   // 2. Next to the object file or archive file
   auto ExpectedSession = handleExpected(
       tryToLoadPDB(TSId, TSPath),
       [&]() {
         StringRef LocalPath =
             !File->ParentName.empty() ? File->ParentName : File->getName();
         SmallString<128> Path = sys::path::parent_path(LocalPath);
-        sys::path::append(
-            Path, sys::path::filename(TSPath, sys::path::Style::windows));
+        sys::path::append(Path, sys::path::filename(TSPath));
         return tryToLoadPDB(TSId, Path);
       },
       [&](std::unique_ptr<ECError> EC) -> Error {
         auto SysErr = EC->convertToErrorCode();
         // Only re-try loading if the previous error was "No such file or
         // directory"
         if (SysErr.category() == std::generic_category() &&
             SysErr.value() == ENOENT)

 void PDBLinker::addObjFile(ObjFile *File) {
   // Add a module descriptor for every object file. We need to put an absolute
   // path to the object into the PDB. If this is a plain object, we make its
   // path absolute. If it's an object in an archive, we make the archive path
   // absolute.
   bool InArchive = !File->ParentName.empty();
   SmallString<128> Path = InArchive ? File->ParentName : File->getName();
   pdbMakeAbsolute(Path);
-  sys::path::native(Path, sys::path::Style::windows);
+  sys::path::native(Path);
   StringRef Name = InArchive ? File->getName() : StringRef(Path);
 
   pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
   File->ModuleDBI = &ExitOnErr(DbiBuilder.addModuleInfo(Name));
   File->ModuleDBI->setObjFileName(Path);
 
   auto Chunks = File->getChunks();
   uint32_t Modi = File->ModuleDBI->getModuleIndex();

 }
 
 void PDBLinker::addSections(ArrayRef<OutputSection *> OutputSections,
                             ArrayRef<uint8_t> SectionTable) {
   // It's not entirely clear what this is, but the * Linker * module uses it.
   pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
   NativePath = Config->PDBPath;
   pdbMakeAbsolute(NativePath);
-  sys::path::native(NativePath, sys::path::Style::windows);
+  sys::path::native(NativePath);
   uint32_t PdbFilePathNI = DbiBuilder.addECName(NativePath);
   auto &LinkerModule = ExitOnErr(DbiBuilder.addModuleInfo("* Linker *"));
   LinkerModule.setPdbFilePathNI(PdbFilePathNI);
   addCommonLinkerModuleSymbols(NativePath, LinkerModule, Alloc);
 
   // Add section contributions. They must be ordered by ascending RVA.
   for (OutputSection *OS : OutputSections) {
     addLinkerModuleSectionSymbol(LinkerModule, *OS, Alloc);

lldb/lit/SymbolFile/NativePDB/source-list.cpp

 
 // Some context lines after
 // the function.
 
 // check lines go at the end so that line numbers stay stable when
 // changing this file.
 
 // CHECK: (lldb) source list -n main
-// CHECK: File: D:\src\llvm-mono\lldb\lit\SymbolFile\NativePDB\source-list.cpp
+// CHECK: File: {{.*}}source-list.cpp
 // CHECK:    10
 // CHECK:    11    // Some context lines before
 // CHECK:    12   // the function.
 // CHECK:    13
 // CHECK:    14
 // CHECK:    15   int main(int argc, char **argv) {
 // CHECK:    16     // Here are some comments.
 // CHECK:    17     // That we should print when listing source.
 // CHECK:    18     return 0;
 // CHECK:    19   }
 // CHECK:    20
 // CHECK:    21   // Some context lines after
 // CHECK:    22   // the function.
 // CHECK:    23

lldb/source/Plugins/SymbolFile/NativePDB/CompileUnitIndex.cpp

if (main == other)
     return true;
 
   // If the files refer to the local file system, we can just ask the file
   // system if they're equivalent.  But if the source isn't present on disk
   // then we still want to try.
   if (llvm::sys::fs::equivalent(main, other))
     return true;
 
-  // FIXME: If we ever want to support PDB debug info for non-Windows systems
-  // the following check will be wrong, but we need a way to store the host
-  // information in the PDB.
   llvm::SmallString<64> normalized(other);
-  llvm::sys::path::native(normalized, llvm::sys::path::Style::windows);
+  llvm::sys::path::native(normalized);
   return main.equals_lower(normalized);
 }
 
 static void ParseCompile3(const CVSymbol &sym, CompilandIndexItem &cci) {
   cci.m_compile_opts.emplace();
   llvm::cantFail(
       SymbolDeserializer::deserializeAs<Compile3Sym>(sym, *cci.m_compile_opts));
 }
CompileUnitIndex::GetOrCreateCompiland(PdbSymUid compiland_uid) {
 
   // We want the main source file to always comes first.  Note that we can't
   // just push_back the main file onto the front because `GetMainSourceFile`
   // computes it in such a way that it doesn't own the resulting memory.  So we
   // have to iterate the module file list comparing each one to the main file
   // name until we find it, and we can cache that one since the memory is backed
   // by a contiguous chunk inside the mapped PDB.
   llvm::SmallString<64> main_file = GetMainSourceFile(*cci);
-  llvm::sys::path::native(main_file, llvm::sys::path::Style::windows);
+  std::string s = main_file.str();
+  llvm::sys::path::native(main_file);
 
   uint32_t file_count = modules.getSourceFileCount(modi);
   cci->m_file_list.reserve(file_count);
   bool found_main_file = false;
   for (llvm::StringRef file : modules.source_files(modi)) {
     if (!found_main_file && IsMainFile(main_file, file)) {
       cci->m_file_list.insert(cci->m_file_list.begin(), file);
       found_main_file = true;

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

bool SymbolFileNativePDB::ParseCompileUnitSupportFiles(
 
   PdbSymUid comp_uid = PdbSymUid::fromOpaqueId(sc.comp_unit->GetID());
   lldbassert(comp_uid.tag() == PDB_SymType::Compiland);
 
   const CompilandIndexItem *cci = m_index->compilands().GetCompiland(comp_uid);
   lldbassert(cci);
 
   for (llvm::StringRef f : cci->m_file_list) {
-    FileSpec spec(f, false, FileSpec::Style::windows);
+    FileSpec::Style style =
+        f.startswith("/") ? FileSpec::Style::posix : FileSpec::Style::windows;
+    FileSpec spec(f, false, style);
     support_files.Append(spec);
   }
 
   return true;
 }
 
 bool SymbolFileNativePDB::ParseImportedModules(
     const lldb_private::SymbolContext &sc,

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Target/TargetLoweringObjectFile.h"
 #include "llvm/Target/TargetMachine.h"
 #include <algorithm>
 #include <cassert>
 #include <cctype>
 #include <cstddef>
StringRef CodeViewDebug::getFullFilepath(const DIFile *File) {
   std::string &Filepath = FileToFilepathMap[File];
   if (!Filepath.empty())
     return Filepath;
 
   StringRef Dir = File->getDirectory(), Filename = File->getFilename();
 
   // If this is a Unix-style path, just use it as is. Don't try to canonicalize
   // it textually because one of the path components could be a symlink.
-  if (!Dir.empty() && Dir[0] == '/') {
+  if (Dir.startswith("/") || Filename.startswith("/")) {
+    if (llvm::sys::path::is_absolute(Filename, llvm::sys::path::Style::posix))
+      return Filename;
     Filepath = Dir;
     if (Dir.back() != '/')
       Filepath += '/';
     Filepath += Filename;
     return Filepath;
   }
 
   // Clang emits directory and relative filename info into the IR, but CodeView