[Lldb-commits] [lldb] a842950 - [lldb] Add a SymbolFileProvider to record and replay calls to dsymForUUID

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Mon Aug 24 15:09:16 PDT 2020


Author: Jonas Devlieghere
Date: 2020-08-24T15:09:08-07:00
New Revision: a842950b62b6d029a392c3c312c6495d6368c2a4

URL: https://github.com/llvm/llvm-project/commit/a842950b62b6d029a392c3c312c6495d6368c2a4
DIFF: https://github.com/llvm/llvm-project/commit/a842950b62b6d029a392c3c312c6495d6368c2a4.diff

LOG: [lldb] Add a SymbolFileProvider to record and replay calls to dsymForUUID

When replaying a reproducer captured from a core file, we always use
dsymForUUID for the kernel binary. When enabled, we also use it to find
kexts. Since these files are already contained in the reproducer,
there's no reason to call out to an external tool. If the tool returns a
different result, e.g. because the dSYM got garbage collected, it will
break reproducer replay. The SymbolFileProvider solves the issue by
mapping UUIDs to module and symbol paths in the reproducer.

Differential revision: https://reviews.llvm.org/D86389

Added: 
    lldb/test/Shell/Reproducer/Inputs/core
    lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh
    lldb/test/Shell/Reproducer/TestDebugSymbols.test

Modified: 
    lldb/include/lldb/Utility/Reproducer.h
    lldb/include/lldb/Utility/ReproducerProvider.h
    lldb/source/Commands/CommandObjectReproducer.cpp
    lldb/source/Symbol/LocateSymbolFile.cpp
    lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
    lldb/source/Utility/ReproducerProvider.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h
index 4dc6ddd51394..d3d6589a7ef8 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -22,6 +22,7 @@
 #include <vector>
 
 namespace lldb_private {
+class UUID;
 namespace repro {
 
 class Reproducer;

diff  --git a/lldb/include/lldb/Utility/ReproducerProvider.h b/lldb/include/lldb/Utility/ReproducerProvider.h
index b84b8a67c4ca..abb13f0edd43 100644
--- a/lldb/include/lldb/Utility/ReproducerProvider.h
+++ b/lldb/include/lldb/Utility/ReproducerProvider.h
@@ -12,6 +12,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/ProcessInfo.h"
 #include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/UUID.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileCollector.h"
@@ -205,6 +206,41 @@ class HomeDirectoryProvider : public DirectoryProvider<HomeDirectoryProvider> {
   static char ID;
 };
 
+/// Provider for mapping UUIDs to symbol and executable files.
+class SymbolFileProvider : public Provider<SymbolFileProvider> {
+public:
+  SymbolFileProvider(const FileSpec &directory)
+      : Provider(directory), m_symbol_files() {}
+
+  void AddSymbolFile(const UUID *uuid, const FileSpec &module_path,
+                     const FileSpec &symbol_path);
+  void Keep() override;
+
+  struct Entry {
+    Entry() = default;
+    Entry(std::string uuid) : uuid(std::move(uuid)) {}
+    Entry(std::string uuid, std::string module_path, std::string symbol_path)
+        : uuid(std::move(uuid)), module_path(std::move(module_path)),
+          symbol_path(std::move(symbol_path)) {}
+
+    bool operator==(const Entry &rhs) const { return uuid == rhs.uuid; }
+    bool operator<(const Entry &rhs) const { return uuid < rhs.uuid; }
+
+    std::string uuid;
+    std::string module_path;
+    std::string symbol_path;
+  };
+
+  struct Info {
+    static const char *name;
+    static const char *file;
+  };
+  static char ID;
+
+private:
+  std::vector<Entry> m_symbol_files;
+};
+
 /// The MultiProvider is a provider that hands out recorder which can be used
 /// to capture data for 
diff erent instances of the same object. The recorders
 /// can be passed around or stored as an instance member.
@@ -345,6 +381,16 @@ template <typename T> class MultiLoader {
   unsigned m_index = 0;
 };
 
+class SymbolFileLoader {
+public:
+  SymbolFileLoader(Loader *loader);
+  std::pair<FileSpec, FileSpec> GetPaths(const UUID *uuid) const;
+
+private:
+  // Sorted list of UUID to path mappings.
+  std::vector<SymbolFileProvider::Entry> m_symbol_files;
+};
+
 /// Helper to read directories written by the DirectoryProvider.
 template <typename T>
 llvm::Expected<std::string> GetDirectoryFrom(repro::Loader *loader) {
@@ -357,4 +403,20 @@ llvm::Expected<std::string> GetDirectoryFrom(repro::Loader *loader) {
 } // namespace repro
 } // namespace lldb_private
 
+LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::repro::SymbolFileProvider::Entry)
+
+namespace llvm {
+namespace yaml {
+template <>
+struct MappingTraits<lldb_private::repro::SymbolFileProvider::Entry> {
+  static void mapping(IO &io,
+                      lldb_private::repro::SymbolFileProvider::Entry &entry) {
+    io.mapRequired("uuid", entry.uuid);
+    io.mapRequired("module-path", entry.module_path);
+    io.mapRequired("symbol-path", entry.symbol_path);
+  }
+};
+} // namespace yaml
+} // namespace llvm
+
 #endif // LLDB_UTILITY_REPRODUCER_PROVIDER_H

diff  --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp
index 9add2df52985..da2d9ca5a901 100644
--- a/lldb/source/Commands/CommandObjectReproducer.cpp
+++ b/lldb/source/Commands/CommandObjectReproducer.cpp
@@ -27,6 +27,7 @@ using namespace lldb_private::repro;
 enum ReproducerProvider {
   eReproducerProviderCommands,
   eReproducerProviderFiles,
+  eReproducerProviderSymbolFiles,
   eReproducerProviderGDB,
   eReproducerProviderProcessInfo,
   eReproducerProviderVersion,
@@ -46,6 +47,11 @@ static constexpr OptionEnumValueElement g_reproducer_provider_type[] = {
         "files",
         "Files",
     },
+    {
+        eReproducerProviderSymbolFiles,
+        "symbol-files",
+        "Symbol Files",
+    },
     {
         eReproducerProviderGDB,
         "gdb",
@@ -427,6 +433,29 @@ class CommandObjectReproducerDump : public CommandObjectParsed {
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
     }
+    case eReproducerProviderSymbolFiles: {
+      Expected<std::string> symbol_files =
+          loader->LoadBuffer<SymbolFileProvider>();
+      if (!symbol_files) {
+        SetError(result, symbol_files.takeError());
+        return false;
+      }
+
+      std::vector<SymbolFileProvider::Entry> entries;
+      llvm::yaml::Input yin(*symbol_files);
+      yin >> entries;
+
+      for (const auto &entry : entries) {
+        result.AppendMessageWithFormat("- uuid:        %s\n",
+                                       entry.uuid.c_str());
+        result.AppendMessageWithFormat("  module path: %s\n",
+                                       entry.module_path.c_str());
+        result.AppendMessageWithFormat("  symbol path: %s\n",
+                                       entry.symbol_path.c_str());
+      }
+      result.SetStatus(eReturnStatusSuccessFinishResult);
+      return true;
+    }
     case eReproducerProviderVersion: {
       Expected<std::string> version = loader->LoadBuffer<VersionProvider>();
       if (!version) {

diff  --git a/lldb/source/Symbol/LocateSymbolFile.cpp b/lldb/source/Symbol/LocateSymbolFile.cpp
index 95ae2ca7917a..af4bbb6e5360 100644
--- a/lldb/source/Symbol/LocateSymbolFile.cpp
+++ b/lldb/source/Symbol/LocateSymbolFile.cpp
@@ -16,6 +16,7 @@
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/UUID.h"
@@ -225,6 +226,7 @@ static FileSpec LocateExecutableSymbolFileDsym(const ModuleSpec &module_spec) {
   } else {
     dsym_module_spec.GetSymbolFileSpec() = symbol_fspec;
   }
+
   return dsym_module_spec.GetSymbolFileSpec();
 }
 
@@ -248,6 +250,7 @@ ModuleSpec Symbols::LocateExecutableObjectFile(const ModuleSpec &module_spec) {
   } else {
     LocateMacOSXFilesUsingDebugSymbols(module_spec, result);
   }
+
   return result;
 }
 

diff  --git a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
index 251605085c58..fe30ceeabcb2 100644
--- a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -27,6 +27,7 @@
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/ReproducerProvider.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/UUID.h"
@@ -53,6 +54,17 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
   return_module_spec.GetFileSpec().Clear();
   return_module_spec.GetSymbolFileSpec().Clear();
 
+  const UUID *uuid = module_spec.GetUUIDPtr();
+  const ArchSpec *arch = module_spec.GetArchitecturePtr();
+
+  if (repro::Loader *l = repro::Reproducer::Instance().GetLoader()) {
+    static repro::SymbolFileLoader symbol_file_loader(l);
+    std::pair<FileSpec, FileSpec> paths = symbol_file_loader.GetPaths(uuid);
+    return_module_spec.GetFileSpec() = paths.first;
+    return_module_spec.GetSymbolFileSpec() = paths.second;
+    return 1;
+  }
+
   int items_found = 0;
 
   if (g_dlsym_DBGCopyFullDSYMURLForUUID == nullptr ||
@@ -69,9 +81,6 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
     return items_found;
   }
 
-  const UUID *uuid = module_spec.GetUUIDPtr();
-  const ArchSpec *arch = module_spec.GetArchitecturePtr();
-
   if (uuid && uuid->IsValid()) {
     // Try and locate the dSYM file using DebugSymbols first
     llvm::ArrayRef<uint8_t> module_uuid = uuid->GetBytes();
@@ -247,6 +256,12 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
     }
   }
 
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) {
+    g->GetOrCreate<repro::SymbolFileProvider>().AddSymbolFile(
+        uuid, return_module_spec.GetFileSpec(),
+        return_module_spec.GetSymbolFileSpec());
+  }
+
   return items_found;
 }
 
@@ -464,6 +479,25 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
   const UUID *uuid_ptr = module_spec.GetUUIDPtr();
   const FileSpec *file_spec_ptr = module_spec.GetFileSpecPtr();
 
+  if (repro::Loader *l = repro::Reproducer::Instance().GetLoader()) {
+    static repro::SymbolFileLoader symbol_file_loader(l);
+    std::pair<FileSpec, FileSpec> paths = symbol_file_loader.GetPaths(uuid_ptr);
+    if (paths.first)
+      module_spec.GetFileSpec() = paths.first;
+    if (paths.second)
+      module_spec.GetSymbolFileSpec() = paths.second;
+    return true;
+  }
+
+  // Lambda to capture the state of module_spec before returning from this
+  // function.
+  auto RecordResult = [&]() {
+    if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) {
+      g->GetOrCreate<repro::SymbolFileProvider>().AddSymbolFile(
+          uuid_ptr, module_spec.GetFileSpec(), module_spec.GetSymbolFileSpec());
+    }
+  };
+
   // It's expensive to check for the DBGShellCommands defaults setting, only do
   // it once per lldb run and cache the result.
   static bool g_have_checked_for_dbgshell_command = false;
@@ -489,6 +523,7 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
   // When g_dbgshell_command is NULL, the user has not enabled the use of an
   // external program to find the symbols, don't run it for them.
   if (!force_lookup && g_dbgshell_command == NULL) {
+    RecordResult();
     return false;
   }
 
@@ -613,8 +648,10 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
                 ::CFDictionaryGetKeysAndValues(plist.get(), NULL,
                                                (const void **)&values[0]);
                 if (num_values == 1) {
-                  return GetModuleSpecInfoFromUUIDDictionary(values[0],
-                                                             module_spec);
+                  success = GetModuleSpecInfoFromUUIDDictionary(values[0],
+                                                                module_spec);
+                  RecordResult();
+                  return success;
                 } else {
                   for (CFIndex i = 0; i < num_values; ++i) {
                     ModuleSpec curr_module_spec;
@@ -623,6 +660,7 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
                       if (module_spec.GetArchitecture().IsCompatibleMatch(
                               curr_module_spec.GetArchitecture())) {
                         module_spec = curr_module_spec;
+                        RecordResult();
                         return true;
                       }
                     }
@@ -644,5 +682,6 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
       }
     }
   }
+  RecordResult();
   return success;
 }

diff  --git a/lldb/source/Utility/ReproducerProvider.cpp b/lldb/source/Utility/ReproducerProvider.cpp
index 54f3a870b7dd..f5556659390b 100644
--- a/lldb/source/Utility/ReproducerProvider.cpp
+++ b/lldb/source/Utility/ReproducerProvider.cpp
@@ -105,6 +105,61 @@ void ProcessInfoRecorder::Record(const ProcessInstanceInfoList &process_infos) {
   m_os.flush();
 }
 
+void SymbolFileProvider::AddSymbolFile(const UUID *uuid,
+                                       const FileSpec &module_file,
+                                       const FileSpec &symbol_file) {
+  if (!uuid || (!module_file && !symbol_file))
+    return;
+  m_symbol_files.emplace_back(uuid->GetAsString(), module_file.GetPath(),
+                              symbol_file.GetPath());
+}
+
+void SymbolFileProvider::Keep() {
+  FileSpec file = this->GetRoot().CopyByAppendingPathComponent(Info::file);
+  std::error_code ec;
+  llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
+  if (ec)
+    return;
+
+  // Remove duplicates.
+  llvm::sort(m_symbol_files.begin(), m_symbol_files.end());
+  m_symbol_files.erase(
+      std::unique(m_symbol_files.begin(), m_symbol_files.end()),
+      m_symbol_files.end());
+
+  llvm::yaml::Output yout(os);
+  yout << m_symbol_files;
+}
+
+SymbolFileLoader::SymbolFileLoader(Loader *loader) {
+  if (!loader)
+    return;
+
+  FileSpec file = loader->GetFile<SymbolFileProvider::Info>();
+  if (!file)
+    return;
+
+  auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+  if (auto err = error_or_file.getError())
+    return;
+
+  llvm::yaml::Input yin((*error_or_file)->getBuffer());
+  yin >> m_symbol_files;
+}
+
+std::pair<FileSpec, FileSpec>
+SymbolFileLoader::GetPaths(const UUID *uuid) const {
+  if (!uuid)
+    return {};
+
+  auto it = std::lower_bound(m_symbol_files.begin(), m_symbol_files.end(),
+                             SymbolFileProvider::Entry(uuid->GetAsString()));
+  if (it == m_symbol_files.end())
+    return {};
+  return std::make_pair<FileSpec, FileSpec>(FileSpec(it->module_path),
+                                            FileSpec(it->symbol_path));
+}
+
 void ProviderBase::anchor() {}
 char CommandProvider::ID = 0;
 char FileProvider::ID = 0;
@@ -113,6 +168,7 @@ char VersionProvider::ID = 0;
 char WorkingDirectoryProvider::ID = 0;
 char HomeDirectoryProvider::ID = 0;
 char ProcessInfoProvider::ID = 0;
+char SymbolFileProvider::ID = 0;
 const char *CommandProvider::Info::file = "command-interpreter.yaml";
 const char *CommandProvider::Info::name = "command-interpreter";
 const char *FileProvider::Info::file = "files.yaml";
@@ -125,3 +181,5 @@ const char *HomeDirectoryProvider::Info::file = "home.txt";
 const char *HomeDirectoryProvider::Info::name = "home";
 const char *ProcessInfoProvider::Info::file = "process-info.yaml";
 const char *ProcessInfoProvider::Info::name = "process-info";
+const char *SymbolFileProvider::Info::file = "symbol-files.yaml";
+const char *SymbolFileProvider::Info::name = "symbol-files";

diff  --git a/lldb/test/Shell/Reproducer/Inputs/core b/lldb/test/Shell/Reproducer/Inputs/core
new file mode 100644
index 000000000000..4b8aabe65a8b
Binary files /dev/null and b/lldb/test/Shell/Reproducer/Inputs/core 
diff er

diff  --git a/lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh b/lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh
new file mode 100755
index 000000000000..ce5ade741ed6
--- /dev/null
+++ b/lldb/test/Shell/Reproducer/Inputs/dsymforuuid.sh
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+
+echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+echo "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//ENhttp://www.apple.com/DTDs/PropertyList-1.0.dtd\">"
+echo "<plist version=\"1.0\">"
+echo "<dict>"
+echo "    <key>AD52358C-94F8-3796-ADD6-B20FFAC00E5C</key>"
+echo "    <dict>"
+echo "        <key>DBGArchitecture</key>"
+echo "        <string>x86_64</string>"
+echo "        <key>DBGBuildSourcePath</key>"
+echo "        <string>/path/to/build/sources</string>"
+echo "        <key>DBGSourcePath</key>"
+echo "        <string>/path/to/actual/sources</string>"
+echo "        <key>DBGDSYMPath</key>"
+echo "        <string>/path/to/foo.dSYM/Contents/Resources/DWARF/foo</string>"
+echo "        <key>DBGSymbolRichExecutable</key>"
+echo "        <string>/path/to/unstripped/executable</string>"
+echo "    </dict>"
+echo "</dict>"
+echo "</plist>"

diff  --git a/lldb/test/Shell/Reproducer/TestDebugSymbols.test b/lldb/test/Shell/Reproducer/TestDebugSymbols.test
new file mode 100644
index 000000000000..6a3cc1249cbd
--- /dev/null
+++ b/lldb/test/Shell/Reproducer/TestDebugSymbols.test
@@ -0,0 +1,14 @@
+# REQUIRES: system-darwin
+
+# RUN: rm -rf %t.repro
+# RUN: env LLDB_APPLE_DSYMFORUUID_EXECUTABLE=%S/Inputs/dsymforuuid.sh %lldb --capture --capture-path %t.repro -c %S/Inputs/core -o 'reproducer generate'
+
+# RUN: cat %t.repro/symbol-files.yaml | FileCheck %s --check-prefix YAML
+# YAML: AD52358C-94F8-3796-ADD6-B20FFAC00E5C
+# YAML: /path/to/unstripped/executable
+# YAML: /path/to/foo.dSYM/Contents/Resources/DWARF/foo
+
+# RUN: %lldb -b -o 'reproducer dump -p symbol-files -f %t.repro' | FileCheck %s --check-prefix DUMP
+# DUMP: uuid: AD52358C-94F8-3796-ADD6-B20FFAC00E5C
+# DUMP-NEXT: module path: /path/to/unstripped/executable
+# DUMP-NEXT: symbol path: /path/to/foo.dSYM/Contents/Resources/DWARF/foo


        


More information about the lldb-commits mailing list