[DebugInfo] Fix /usr/lib/debug llvm-symbolizer lookup with relative paths

[rupprecht]

Summary

rL189250 added a realpath call, and rL352916 because realpath breaks assumptions with some build systems. However, the /usr/lib/debug case has been clarified, falling back to /usr/lib/debug is currently broken if the obj passed in is a relative path. Adding a call to use absolute paths when falling back to /usr/lib/debug fixes that while still not making any realpath assumptions.

This also adds a --fallback-debug-path command line flag for testing (since we probably can't write to /usr/lib/debug from buildbot environments), but was also verified manually:

$ rm -f path/to/dwarfdump-test.elf-x86-64
$ strace llvm-symbolizer --obj=relative/path/to/dwarfdump-test.elf-x86-64.debuglink 0x40113f |& grep dwarfdump

Lookups went to relative/path/to/dwarfdump-test.elf-x86-64, relative/path/to/.debug/dwarfdump-test.elf-x86-64, and then finally /usr/lib/debug/absolute/path/to/dwarfdump-test.elf-x86-64.

[dblaikie]

One option for testing would be to move DebugPath into a user-configurable value (something that could be passed on the command line), for testing purposes. Not sure if it's worth it, though.

[rupprecht]

• Add --fallback-debug-path flag for testing /usr/lib/debug fallback

[dblaikie]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

146–154 ↗

(On Diff #186092)

If these default values are going to be encoded here - perhaps both versions (for diagnostic printing and for the implementation) should be here, using the cl::init value to propagate the default from here & remove it from Symbolize.cpp?

(alternatively, perhaps it's not important enough to specify what the default is in the diagnostic? Dunno)

Just a thought.

[rupprecht]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

146–154 ↗

(On Diff #186092)

There's a lot more paths through findDebugBinary, so removing the default from there would mean adding it to all call sites (there might be ~10 or so based on a quick check)

Including the default fallback in --help doesn't seem too important, so I'll just remove it.

[dblaikie]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

146–154 ↗

(On Diff #186092)

Oh, I didn't mean having to pass it around separately - I meant using the cl::init value, something like this I would think:

#if defined(__NetBSD__)
#define DEFAULT_DEBUG_PATH "/usr/libdata/debug"
#else
#define DEFAULT_DEBUG_PATH "/usr/lib/debug"
#endif
static cl::opt<std::string> ClFallbackDebugPath(
    "fallback-debug-path", cl::init(DEFAULT_DEBUG_PATH),
    cl::desc("Fallback path for debug binaries. If empty, defaults to "
          DEFAULT_DEBUG_PATH));

At least I think that'd work, probably?

That way the parameter support and defaults are all here - the code later on uses whatever value is passed to it & doesn't have to worry about it being empty & falling back to values hardcoded over there, etc. Does that make sense?

(but no worries either way)

[rupprecht]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

146–154 ↗

(On Diff #186092)

Yeah, that should work. But the flip side is if some new platform has a different default, we'd have to update both llvm-symbolizer and Symbolizer.cpp to handle it; or more likely, we'd only update Symbolizer.cpp and then wonder why we weren't getting the new default path w/ llvm-symbolizer.

Probably the best thing would be expose the default value in Symbolizer.h, but that's just more plumbing than I think it's worth, especially since this probably won't be used outside this unit test :)

[dblaikie]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

146–154 ↗

(On Diff #186092)

Oh, sorry, I get what you mean now - makes sense. Thanks!

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

bool findDebugBinary(const std::string &OrigPath,
   }
 #if defined(__NetBSD__)
   // Try /usr/libdata/debug/path/to/original_binary/debuglink_name
   DebugPath = "/usr/libdata/debug";
 #else
   // Try /usr/lib/debug/path/to/original_binary/debuglink_name
   DebugPath = "/usr/lib/debug";
 #endif
+  // Make the path absolute so that lookups will go to
+  // "/usr/lib/debug/full/path/to/debug", not
+  // "/usr/lib/debug/to/debug"
+  llvm::sys::fs::make_absolute(OrigDir);
   llvm::sys::path::append(DebugPath, llvm::sys::path::relative_path(OrigDir),
                           DebuglinkName);
   if (checkFileCRC(DebugPath, CRCHash)) {
     Result = DebugPath.str();
     return true;
   }
   return false;
 }