[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

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

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

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

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

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

llvm/include/llvm/DebugInfo/Symbolize/Symbolize.h

 public:
   struct Options {
     FunctionNameKind PrintFunctions;
     bool UseSymbolTable : 1;
     bool Demangle : 1;
     bool RelativeAddresses : 1;
     std::string DefaultArch;
     std::vector<std::string> DsymHints;
+    std::string FallbackDebugPath;
 
     Options(FunctionNameKind PrintFunctions = FunctionNameKind::LinkageName,
             bool UseSymbolTable = true, bool Demangle = true,
-            bool RelativeAddresses = false, std::string DefaultArch = "")
+            bool RelativeAddresses = false, std::string DefaultArch = "",
+            std::string FallbackDebugPath = "")
         : PrintFunctions(PrintFunctions), UseSymbolTable(UseSymbolTable),
           Demangle(Demangle), RelativeAddresses(RelativeAddresses),
-          DefaultArch(std::move(DefaultArch)) {}
+          DefaultArch(std::move(DefaultArch)),
+          FallbackDebugPath(std::move(FallbackDebugPath)) {}
   };
 
   LLVMSymbolizer(const Options &Opts = Options()) : Opts(Opts) {}
 
   ~LLVMSymbolizer() {
     flush();
   }
 

llvm/lib/DebugInfo/Symbolize/Symbolize.cpp

ErrorOr<std::unique_ptr<MemoryBuffer>> MB =
       MemoryBuffer::getFileOrSTDIN(Path);
   if (!MB)
     return false;
   return !zlib::isAvailable() || CRCHash == zlib::crc32(MB.get()->getBuffer());
 }
 
 bool findDebugBinary(const std::string &OrigPath,
                      const std::string &DebuglinkName, uint32_t CRCHash,
+                     const std::string &FallbackDebugPath,
                      std::string &Result) {
   SmallString<16> OrigDir(OrigPath);
   llvm::sys::path::remove_filename(OrigDir);
   SmallString<16> DebugPath = OrigDir;
-  // Try /path/to/original_binary/debuglink_name
+  // Try relative/path/to/original_binary/debuglink_name
   llvm::sys::path::append(DebugPath, DebuglinkName);
   if (checkFileCRC(DebugPath, CRCHash)) {
     Result = DebugPath.str();
     return true;
   }
-  // Try /path/to/original_binary/.debug/debuglink_name
+  // Try relative/path/to/original_binary/.debug/debuglink_name
   DebugPath = OrigDir;
   llvm::sys::path::append(DebugPath, ".debug", DebuglinkName);
   if (checkFileCRC(DebugPath, CRCHash)) {
     Result = DebugPath.str();
     return true;
   }
+  // 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);
+  if (!FallbackDebugPath.empty()) {
+    // Try <FallbackDebugPath>/absolute/path/to/original_binary/debuglink_name
+    DebugPath = FallbackDebugPath;
+  } else {
 #if defined(__NetBSD__)
-  // Try /usr/libdata/debug/path/to/original_binary/debuglink_name
+    // Try /usr/libdata/debug/absolute/path/to/original_binary/debuglink_name
     DebugPath = "/usr/libdata/debug";
 #else
-  // Try /usr/lib/debug/path/to/original_binary/debuglink_name
+    // Try /usr/lib/debug/absolute/path/to/original_binary/debuglink_name
     DebugPath = "/usr/lib/debug";
 #endif
+  }
   llvm::sys::path::append(DebugPath, llvm::sys::path::relative_path(OrigDir),
                           DebuglinkName);
   if (checkFileCRC(DebugPath, CRCHash)) {
     Result = DebugPath.str();
     return true;
   }
   return false;
 }

 ObjectFile *LLVMSymbolizer::lookUpDebuglinkObject(const std::string &Path,
                                                   const ObjectFile *Obj,
                                                   const std::string &ArchName) {
   std::string DebuglinkName;
   uint32_t CRCHash;
   std::string DebugBinaryPath;
   if (!getGNUDebuglinkContents(Obj, DebuglinkName, CRCHash))
     return nullptr;
-  if (!findDebugBinary(Path, DebuglinkName, CRCHash, DebugBinaryPath))
+  if (!findDebugBinary(Path, DebuglinkName, CRCHash, Opts.FallbackDebugPath,
+                       DebugBinaryPath))
     return nullptr;
   auto DbgObjOrErr = getOrCreateObject(DebugBinaryPath, ArchName);
   if (!DbgObjOrErr) {
     // Ignore errors, the file might not exist.
     consumeError(DbgObjOrErr.takeError());
     return nullptr;
   }
   return DbgObjOrErr.get();

llvm/test/DebugInfo/symbolize-gnu-debuglink-fallback.test

+# REQUIRES: shell
+# Ensures that .debuglink can fallback to a separate location. This is normally
+# /usr/lib/debug (or /usr/libdata/debug for NetBSD), but can be configured on
+# the command line (mainly for testing).
+
+RUN: rm -rf %t/foo %t/bar
+RUN: mkdir -p %t/foo %t/bar/%t/foo
+
+RUN: cp %p/Inputs/dwarfdump-test.elf-x86-64.debuglink %t/foo
+
+RUN: llvm-symbolizer --obj=%t/foo/dwarfdump-test.elf-x86-64.debuglink 0x40113f \
+RUN:   --fallback-debug-path=%t/bar | FileCheck %s --check-prefix=UNKNOWN
+
+UNKNOWN:      main
+UNKNOWN-NEXT: ??:0:0
+
+RUN: cp %p/Inputs/dwarfdump-test.elf-x86-64 %t/bar/%t/foo
+RUN: llvm-symbolizer --obj=%t/foo/dwarfdump-test.elf-x86-64.debuglink 0x40113f \
+RUN:   --fallback-debug-path=%t/bar | FileCheck %s --check-prefix=FOUND
+
+FOUND:      main
+FOUND-NEXT: /tmp/dbginfo{{[/\\]}}dwarfdump-test.cc:16

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

 static cl::opt<unsigned long long>
     ClAdjustVMA("adjust-vma", cl::init(0), cl::value_desc("offset"),
                 cl::desc("Add specified offset to object file addresses"));
 
 static cl::list<std::string> ClInputAddresses(cl::Positional,
                                               cl::desc("<input addresses>..."),
                                               cl::ZeroOrMore);
 
+static cl::opt<std::string> ClFallbackDebugPath(
+    "fallback-debug-path", cl::init(""),
+    cl::desc("Fallback path for debug binaries. If empty, defaults to "
+#if defined(__NetBSD__)
+             "/usr/libdata/debug"
+#else
+             "/usr/lib/debug"
+#endif
+             ));
+
 template<typename T>
 static bool error(Expected<T> &ResOrErr) {
   if (ResOrErr)
     return false;
   logAllUnhandledErrors(ResOrErr.takeError(), errs(),
                         "LLVMSymbolizer: error reading file: ");
   return true;
 }
int main(int argc, char **argv) {
   llvm::sys::InitializeCOMRAII COM(llvm::sys::COMThreadingMode::MultiThreaded);
   cl::ParseCommandLineOptions(argc, argv, "llvm-symbolizer\n");
 
   // If both --demangle and --no-demangle are specified then pick the last one.
   if (ClNoDemangle.getPosition() > ClDemangle.getPosition())
     ClDemangle = !ClNoDemangle;
 
   LLVMSymbolizer::Options Opts(ClPrintFunctions, ClUseSymbolTable, ClDemangle,
-                               ClUseRelativeAddress, ClDefaultArch);
+                               ClUseRelativeAddress, ClDefaultArch,
+                               ClFallbackDebugPath);
 
   for (const auto &hint : ClDsymHint) {
     if (sys::path::extension(hint) == ".dSYM") {
       Opts.DsymHints.push_back(hint);
     } else {
       errs() << "Warning: invalid dSYM hint: \"" << hint <<
                 "\" (must have the '.dSYM' extension).\n";
     }