[lld] r344377 - Better support for POSIX paths in PDBs.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 12 10:26:20 PDT 2018


Author: zturner
Date: Fri Oct 12 10:26:19 2018
New Revision: 344377

URL: http://llvm.org/viewvc/llvm-project?rev=344377&view=rev
Log:
Better support for POSIX paths in PDBs.

This a resubmission of a patch which was previously reverted
due to breaking several lld tests.  The issues causing those
failures have been fixed, so the patch is now resubmitted.

---Original Commit Message---

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.

Furthermore, the behavior of LLD can now be summarized by a much
simpler rule than before: Unless you specify /pdbsourcepath and
/pdbaltpath, the PDB ends up with paths that are valid within
the context of the machine that the link is performed on.

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

Modified:
    lld/trunk/COFF/PDB.cpp
    lld/trunk/test/COFF/pdb-relative-source-lines.test
    lld/trunk/test/COFF/pdb-same-name.test

Modified: lld/trunk/COFF/PDB.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/PDB.cpp?rev=344377&r1=344376&r2=344377&view=diff
==============================================================================
--- lld/trunk/COFF/PDB.cpp (original)
+++ lld/trunk/COFF/PDB.cpp Fri Oct 12 10:26:19 2018
@@ -222,26 +222,32 @@ public:
 // 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))
+  // The default behavior is to produce paths that are valid within the context
+  // of the machine that you perform the link on.  If the linker is running on
+  // a POSIX system, we will output absolute POSIX paths.  If the linker is
+  // running on a Windows system, we will output absolute Windows paths.  If the
+  // user desires any other kind of behavior, they should explicitly pass
+  // /pdbsourcepath, in which case we will treat the exact string the user
+  // passed in as the gospel and not normalize, canonicalize it.
+  if (sys::path::is_absolute(FileName, sys::path::Style::windows) ||
+      sys::path::is_absolute(FileName, sys::path::Style::posix))
     return;
+
+  // It's not absolute in any path syntax.  Relative paths necessarily refer to
+  // the local file system, so we can make it native without ending up with a
+  // nonsensical path.
+  sys::path::native(FileName);
   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.
+  // Only apply native and dot removal to the relative file path.  We want to
+  // leave the path the user specified untouched since we assume they specified
+  // it for a reason.
+  sys::path::remove_dots(FileName, /*remove_dot_dots=*/true);
+
   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);
   FileName = std::move(AbsoluteFileName);
 }
 
@@ -444,6 +450,8 @@ PDBLinker::maybeMergeTypeServerPDB(ObjFi
         StringRef LocalPath =
             !File->ParentName.empty() ? File->ParentName : File->getName();
         SmallString<128> Path = sys::path::parent_path(LocalPath);
+        // Currently, type server PDBs are only created by cl, which only runs
+        // on Windows, so we can assume type server paths are Windows style.
         sys::path::append(
             Path, sys::path::filename(TSPath, sys::path::Style::windows));
         return tryToLoadPDB(TSId, Path);
@@ -1027,7 +1035,6 @@ void PDBLinker::addObjFile(ObjFile *File
   bool InArchive = !File->ParentName.empty();
   SmallString<128> Path = InArchive ? File->ParentName : File->getName();
   pdbMakeAbsolute(Path);
-  sys::path::native(Path, sys::path::Style::windows);
   StringRef Name = InArchive ? File->getName() : StringRef(Path);
 
   pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
@@ -1312,7 +1319,6 @@ void PDBLinker::addSections(ArrayRef<Out
   pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
   NativePath = Config->PDBPath;
   pdbMakeAbsolute(NativePath);
-  sys::path::native(NativePath, sys::path::Style::windows);
   uint32_t PdbFilePathNI = DbiBuilder.addECName(NativePath);
   auto &LinkerModule = ExitOnErr(DbiBuilder.addModuleInfo("* Linker *"));
   LinkerModule.setPdbFilePathNI(PdbFilePathNI);

Modified: lld/trunk/test/COFF/pdb-relative-source-lines.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/pdb-relative-source-lines.test?rev=344377&r1=344376&r2=344377&view=diff
==============================================================================
--- lld/trunk/test/COFF/pdb-relative-source-lines.test (original)
+++ lld/trunk/test/COFF/pdb-relative-source-lines.test Fri Oct 12 10:26:19 2018
@@ -34,27 +34,29 @@ RUN: yaml2obj %S/Inputs/pdb_lines_1_rela
 RUN: yaml2obj %S/Inputs/pdb_lines_2_relative.yaml -o %t/pdb_lines_2_relative.obj
 RUN: ./lld-link -debug "-pdbsourcepath:c:\src" -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj
 RUN: llvm-pdbutil pdb2yaml -modules -module-files -module-syms -subsections=lines,fc %t/out.pdb | FileCheck %s
+RUN: ./lld-link -debug "-pdbsourcepath:/usr/src" -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj
+RUN: llvm-pdbutil pdb2yaml -modules -module-files -module-syms -subsections=lines,fc %t/out.pdb | FileCheck --check-prefix=POSIX %s
 
-CHECK-LABEL:  - Module:          'c:\src\pdb_lines_1_relative.obj'
-CHECK-NEXT:     ObjFile:         'c:\src\pdb_lines_1_relative.obj'
+CHECK-LABEL:  - Module:          'c:\src{{[\\/]}}pdb_lines_1_relative.obj'
+CHECK-NEXT:     ObjFile:         'c:\src{{[\\/]}}pdb_lines_1_relative.obj'
 CHECK:          SourceFiles:
-CHECK-NEXT:       - 'c:{{[\\/]}}src{{[\\/]}}pdb_lines_1.c'
-CHECK-NEXT:       - 'c:{{[\\/]}}src{{[\\/]}}foo.h'
+CHECK-NEXT:       - 'c:\src{{[\\/]}}pdb_lines_1.c'
+CHECK-NEXT:       - 'c:\src{{[\\/]}}foo.h'
 CHECK:          Subsections:
-CHECK:                - FileName:        'c:{{[\\/]}}src{{[\\/]}}pdb_lines_1.c'
-CHECK:                - FileName:        'c:{{[\\/]}}src{{[\\/]}}foo.h'
+CHECK:                - FileName:        'c:\src{{[\\/]}}pdb_lines_1.c'
+CHECK:                - FileName:        'c:\src{{[\\/]}}foo.h'
 CHECK:            - !FileChecksums
-CHECK:                - FileName:        'c:{{[\\/]}}src{{[\\/]}}pdb_lines_1.c'
-CHECK:                - FileName:        'c:{{[\\/]}}src{{[\\/]}}foo.h'
+CHECK:                - FileName:        'c:\src{{[\\/]}}pdb_lines_1.c'
+CHECK:                - FileName:        'c:\src{{[\\/]}}foo.h'
 
-CHECK-LABEL:  - Module:          'c:\src\pdb_lines_2_relative.obj'
-CHECK-NEXT:     ObjFile:         'c:\src\pdb_lines_2_relative.obj'
+CHECK-LABEL:  - Module:          'c:\src{{[\\/]}}pdb_lines_2_relative.obj'
+CHECK-NEXT:     ObjFile:         'c:\src{{[\\/]}}pdb_lines_2_relative.obj'
 CHECK:          SourceFiles:
-CHECK-NEXT:       - 'c:{{[\\/]}}src{{[\\/]}}pdb_lines_2.c'
+CHECK-NEXT:       - 'c:\src{{[\\/]}}pdb_lines_2.c'
 CHECK:          Subsections:
-CHECK:                - FileName:        'c:{{[\\/]}}src{{[\\/]}}pdb_lines_2.c'
+CHECK:                - FileName:        'c:\src{{[\\/]}}pdb_lines_2.c'
 CHECK:            - !FileChecksums
-CHECK:                - FileName:        'c:{{[\\/]}}src{{[\\/]}}pdb_lines_2.c'
+CHECK:                - FileName:        'c:\src{{[\\/]}}pdb_lines_2.c'
 
 CHECK-LABEL:  - Kind:            S_ENVBLOCK
 CHECK-NEXT:     EnvBlockSym:     
@@ -62,8 +64,42 @@ CHECK-NEXT:       Entries:
 CHECK-NEXT:         - cwd
 CHECK-NEXT:         - 'c:\src'
 CHECK-NEXT:         - exe
-CHECK-NEXT:         - 'c:\src\lld-link'
+CHECK-NEXT:         - 'c:\src{{[\\/]}}lld-link'
 CHECK-NEXT:         - pdb 
-CHECK-NEXT:         - 'c:\src\out.pdb'
+CHECK-NEXT:         - 'c:\src{{[\\/]}}out.pdb'
 CHECK-NEXT:         - cmd
 CHECK-NEXT:         - '-debug -pdbsourcepath:c:\src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj'
+
+
+POSIX-LABEL:  - Module:          '/usr/src{{[\\/]}}pdb_lines_1_relative.obj'
+POSIX-NEXT:     ObjFile:         '/usr/src{{[\\/]}}pdb_lines_1_relative.obj'
+POSIX:          SourceFiles:
+POSIX-NEXT:       - '/usr/src{{[\\/]}}pdb_lines_1.c'
+POSIX-NEXT:       - '/usr/src{{[\\/]}}foo.h'
+POSIX:          Subsections:
+POSIX:                - FileName:        '/usr/src{{[\\/]}}pdb_lines_1.c'
+POSIX:                - FileName:        '/usr/src{{[\\/]}}foo.h'
+POSIX:            - !FileChecksums
+POSIX:                - FileName:        '/usr/src{{[\\/]}}pdb_lines_1.c'
+POSIX:                - FileName:        '/usr/src{{[\\/]}}foo.h'
+
+POSIX-LABEL:  - Module:          '/usr/src{{[\\/]}}pdb_lines_2_relative.obj'
+POSIX-NEXT:     ObjFile:         '/usr/src{{[\\/]}}pdb_lines_2_relative.obj'
+POSIX:          SourceFiles:
+POSIX-NEXT:       - '/usr/src{{[\\/]}}pdb_lines_2.c'
+POSIX:          Subsections:
+POSIX:                - FileName:        '/usr/src{{[\\/]}}pdb_lines_2.c'
+POSIX:            - !FileChecksums
+POSIX:                - FileName:        '/usr/src{{[\\/]}}pdb_lines_2.c'
+
+POSIX-LABEL:  - Kind:            S_ENVBLOCK
+POSIX-NEXT:     EnvBlockSym:     
+POSIX-NEXT:       Entries:     
+POSIX-NEXT:         - cwd
+POSIX-NEXT:         - '/usr/src'
+POSIX-NEXT:         - exe
+POSIX-NEXT:         - '/usr/src{{[\\/]}}lld-link'
+POSIX-NEXT:         - pdb 
+POSIX-NEXT:         - '/usr/src{{[\\/]}}out.pdb'
+POSIX-NEXT:         - cmd
+POSIX-NEXT:         - '-debug -pdbsourcepath:/usr/src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj'

Modified: lld/trunk/test/COFF/pdb-same-name.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/pdb-same-name.test?rev=344377&r1=344376&r2=344377&view=diff
==============================================================================
--- lld/trunk/test/COFF/pdb-same-name.test (original)
+++ lld/trunk/test/COFF/pdb-same-name.test Fri Oct 12 10:26:19 2018
@@ -14,10 +14,10 @@
 RAW:                               Modules
 RAW-NEXT: ============================================================
 RAW-NEXT:   Mod 0000 | `foo.obj`:
-RAW-NEXT:              Obj: `{{.*}}1\foo.lib`:
+RAW-NEXT:              Obj: `{{.*}}1{{[\\/]}}foo.lib`:
 RAW-NEXT:              debug stream: 11, # files: 1, has ec info: false
 RAW-NEXT:              pdb file ni: 0 ``, src file ni: 0 ``
 RAW-NEXT:   Mod 0001 | `foo.obj`:
-RAW-NEXT:              Obj: `{{.*}}2\foo.lib`:
+RAW-NEXT:              Obj: `{{.*}}2{{[\\/]}}foo.lib`:
 RAW-NEXT:              debug stream: 12, # files: 1, has ec info: false
 RAW-NEXT:              pdb file ni: 0 ``, src file ni: 0 ``




More information about the llvm-commits mailing list