This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Refactor Symbols::DownloadObjectAndSymbolFile
ClosedPublic

Authored by JDevlieghere on Aug 5 2022, 3:21 PM.

Details

Summary
  • Reduce indentation
  • Extract caching of the DbgShellCommand and the dsymForUUID executable (or equivalent)
  • Check the DBGShellCommands before falling back to /usr/local/bin/dsymForUUID
  • Don't check ~rc/bin/dsymForUUID
  • Improve error reporting
  • Don't cache the value of LLDB_APPLE_DSYMFORUUID_EXECUTABLE

Diff Detail

Event Timeline

JDevlieghere created this revision.Aug 5 2022, 3:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2022, 3:21 PM
JDevlieghere requested review of this revision.Aug 5 2022, 3:21 PM
JDevlieghere edited the summary of this revision. (Show Details)
  • Don't cache the value of LLDB_APPLE_DSYMFORUUID_EXECUTABLE
  • Fix typo
  • Still resolve the LLDB_APPLE_DSYMFORUUID_EXECUTABLE in case it's a relative path
jasonmolenda accepted this revision.Aug 6 2022, 11:22 PM

LGTM, I was looking at this method last week and had the same idea -- I started on a patch similar to this, but hadn't finished yet.

lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
566

I had this conditional also test if LLDB_APPLE_DSYMFORUUID_EXECUTABLE was currently set in the environment, which is equivalent to if the DBGShellCommand was set in the com.apple.DebugSymbol preferences. But as I think through it, I think it's fine -- code is calling Symbol::DownloadObjectAndSymbolFile() when it is wiling to do a potentially expensive search for the debug information almost always - force==true is the default, and all of the important ones we're testing in the testsuite.

This revision is now accepted and ready to land.Aug 6 2022, 11:22 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2022, 8:14 PM