Page MenuHomePhabricator

[LLDB] Improve PDB discovery
ClosedPublic

Authored by amccarth on Jul 28 2020, 6:14 PM.

Details

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.

Diff Detail

Event Timeline

amccarth requested review of this revision.Jul 28 2020, 6:14 PM
amccarth created this revision.
amccarth updated this revision to Diff 281445.Jul 28 2020, 6:32 PM

Fixed typos in comments.

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 (?)

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 updated this revision to Diff 284025.Aug 7 2020, 1:36 PM

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 accepted this revision.Aug 11 2020, 3:55 AM

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.

This revision is now accepted and ready to land.Aug 11 2020, 3:55 AM
amccarth closed this revision.Aug 13 2020, 4:17 PM

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