[LLDB] Improve PDB discovery

[amccarth]

Summary

When loading a PE/COFF target, the associated PDB file often wasn't found. The executable module contains a path for the associated PDB file, but people often debug from a different directory than the one their build system uses. (This is especially common in post-mortem and cross platform debugging.)

Suppose the COFF executable being debugged is ~/proj/foo.exe, but it was built elsewhere and refers to D:\remote\build\env\foobar.pdb, LLDB wouldn't find it.

With this change, if no file exists at the PDB path, LLDB will look in the executable directory for a PDB file that matches the name of the one it expected (e.g., ~/proj/foobar.pdb). If found, the PDB is subject to the same matching criteria (GUIDs and age) as would have been used had it been in the original location.

This same-directory-as-the-binary rule is commonly used by debuggers on Windows.

[amccarth]

Fixed typos in comments.

[labath]

The idea is fine -- the same logic is used in Symbols::LocateExecutableSymbolFile. In fact, one of the problems of our pdb support is that it does not use that function to locate symbol files (that way it would also respect the target.debug-file-search-paths setting, which it looks like it doesn't do right now). But it can't do that right now, because we don't treat pdbs as ObjectFiles, etc..

This doesn't seem particularly intrusive, so I don't think we need to teach introduce an ObjectFilePDB just yet, but I think a time will come when we will need to do that...

This could use a test though. It sounds like it shouldn't be hard to construct one (?)

[amccarth]

Thanks. Working on a test.

I found a description of how the Windows debuggers look for PDBs, and it's different than what I expected:

1. The directory that contains the binary
2. The build path embedded in the binary
3. (if enabled) The symbol server cache
4. (if enabled) That symbol server

LLDB currently has only step 2. This patch adds step 1, but it adds it as the _second_ step. And that's it. I figured it might also look locally on the PATH or some "well-known" locations, but nope.

(It's buried in a paragraph in this long article by John Robbins: https://www.wintellect.com/pdb-files-what-every-developer-must-know/)

[amccarth]

Added test to check locating the PDB either in the original build directory or adjacent to the executable.

I tried but failed to make a negative test. LLDB sends the errors message to stderr when the target modules dump symfile command fails. I tried using redirects to combine stdout and stderr, but lit's built-in shell told me that was unsupported. I couldn't find a good way to arrange FileCheck to confirm the absence case.

[labath]

In D84815#2203802, @amccarth wrote:

I tried but failed to make a negative test. LLDB sends the errors message to stderr when the target modules dump symfile command fails. I tried using redirects to combine stdout and stderr, but lit's built-in shell told me that was unsupported. I couldn't find a good way to arrange FileCheck to confirm the absence case.

We have a bunch of lldb tests that do %lldb ... 2>&1 | FileCheck and they don't seem to be disabled on windows. What makes this test special?

LGTM, because I don't think a negative test is strictly required for this, though it would definitely be nice to have.

[amccarth]

Landed on Tuesday. I must have messed up the Differential Revision: line.

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

loadMatchingPDBFile(std::string exe_path, llvm::BumpPtrAllocator &allocator) {
 
   // If it doesn't have a debug directory, fail.
   llvm::StringRef pdb_file;
   if (llvm::Error e = obj->getDebugPDBInfo(pdb_info, pdb_file)) {
     consumeError(std::move(e));
     return nullptr;
   }
 
-  // if the file doesn't exist, is not a pdb, or doesn't have a matching guid,
-  // fail.
+  // If the file doesn't exist, perhaps the path specified at build time
+  // doesn't match the PDB's current location, so check the location of the
+  // executable.
+  if (!FileSystem::Instance().Exists(pdb_file)) {
+    const auto exe_dir = FileSpec(exe_path).CopyByRemovingLastPathComponent();
+    const auto pdb_name = FileSpec(pdb_file).GetFilename().GetCString();
+    pdb_file = exe_dir.CopyByAppendingPathComponent(pdb_name).GetCString();
+  }
+
+  // If the file is not a PDB or if it doesn't have a matching GUID, fail.
   llvm::file_magic magic;
   auto ec = llvm::identify_magic(pdb_file, magic);
   if (ec || magic != llvm::file_magic::pdb)
     return nullptr;
   std::unique_ptr<PDBFile> pdb = loadPDBFile(std::string(pdb_file), allocator);
   if (!pdb)
     return nullptr;
 

lldb/test/Shell/SymbolFile/NativePDB/Inputs/locate-pdb.lldbinit

+target modules dump symfile
+quit

lldb/test/Shell/SymbolFile/NativePDB/locate-pdb.cpp

+// clang-format off
+// REQUIRES: lld, x86
+
+// Test that lldb can find the PDB file that corresponds to the executable.  The linker
+// writes a path to the PDB in the executable.  If the PDB is not there, lldb should
+// check the directory that contains the executable.  We'll generate the PDB file in
+// a subdirectory and then move it into the directory with the executable.  That will
+// ensure the PDB path stored in the executable is wrong.
+
+// Build an EXE and PDB in different directories
+// RUN: mkdir -p %t/executable
+// RUN: rm -f %t/executable/foo.exe %t/executable/bar.pdb
+// RUN: mkdir -p %t/symbols
+// RUN: rm -f %t/symbols/bar.pdb
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t/executable/foo.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t/executable/foo.obj \
+// RUN:     -out:%t/executable/foo.exe -pdb:%t/symbols/bar.pdb
+
+// Find the PDB in its build location
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t/executable/foo.exe -s \
+// RUN:     %p/Inputs/locate-pdb.lldbinit | FileCheck %s
+
+// Also find the PDB when it's adjacent to the executable
+// RUN: mv -f %t/symbols/bar.pdb %t/executable/bar.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t/executable/foo.exe -s \
+// RUN:     %p/Inputs/locate-pdb.lldbinit | FileCheck %s
+
+int main(int argc, char** argv) {
+  return 0;
+}
+
+// CHECK: (lldb) target modules dump symfile
+// CHECK: Dumping debug symbols for 1 modules.
+// CHECK: SymbolFile pdb