[lldb] Don't process symlinks deep inside DWARFUnit

Summary:
This code is handling debug info paths starting with /proc/self/cwd,
which is one of the mechanisms people use to obtain "relocatable" debug
info (the idea being that one starts the debugger with an appropriate
cwd and things "just work").

Instead of resolving the symlinks inside DWARFUnit, we can do the same
thing more elegantly by hooking into the existing Module path remapping
code. Since llvm::DWARFUnit does not support any similar functionality,
doing things this way is also a step towards unifying llvm and lldb
dwarf parsers.

Reviewers: JDevlieghere, aprantl, clayborg, jdoerfert

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D71770

lldb/include/lldb/Core/Module.h

@@ -10,6 +10,7 @@
 #define liblldb_Module_h_
 
 #include "lldb/Core/Address.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContextScope.h"
@@ -966,10 +967,10 @@ protected:
   ///references to them
   TypeSystemMap m_type_system_map;   ///< A map of any type systems associated
                                      ///with this module
-  PathMappingList m_source_mappings; ///< Module specific source remappings for
+  /// Module specific source remappings for when you have debug info for a
-                                     ///when you have debug info for a module
+  /// module that doesn't match where the sources currently are.
-                                     ///that doesn't match where the sources
+  PathMappingList m_source_mappings =
-                                     ///currently are
+      ModuleList::GetGlobalModuleListProperties().GetSymlinkMappings();
   lldb::SectionListUP m_sections_up; ///< Unified section list for module that
                                      /// is used by the ObjectFile and and
                                      /// ObjectFile instances for the debug info

lldb/include/lldb/Core/ModuleList.h

@@ -20,6 +20,7 @@
 #include "lldb/lldb-types.h"
 
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/RWMutex.h"
 
 #include <functional>
 #include <list>
@@ -46,6 +47,11 @@ class UUID;
 class VariableList;
 
 class ModuleListProperties : public Properties {
+  mutable llvm::sys::RWMutex m_symlink_paths_mutex;
+  PathMappingList m_symlink_paths;
+
+  void UpdateSymlinkMappings();
+
 public:
   ModuleListProperties();
 
@@ -53,6 +59,8 @@ public:
   bool SetClangModulesCachePath(llvm::StringRef path);
   bool GetEnableExternalLookup() const;
   bool SetEnableExternalLookup(bool new_value);
+
+  PathMappingList GetSymlinkMappings() const;
 };
 
 /// \class ModuleList ModuleList.h "lldb/Core/ModuleList.h"

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py

@@ -12,7 +12,7 @@ from lldbsuite.test import lldbutil
 
 _EXE_NAME = 'CompDirSymLink'  # Must match Makefile
 _SRC_FILE = 'relative.cpp'
-_COMP_DIR_SYM_LINK_PROP = 'plugin.symbol-file.dwarf.comp-dir-symlink-paths'
+_COMP_DIR_SYM_LINK_PROP = 'symbols.debug-info-symlink-paths'
 
 
 class CompDirSymLinkTestCase(TestBase):
@@ -30,10 +30,7 @@ class CompDirSymLinkTestCase(TestBase):
     @skipIf(hostoslist=["windows"])
     def test_symlink_paths_set(self):
         pwd_symlink = self.create_src_symlink()
-        self.doBuild(pwd_symlink)
+        self.doBuild(pwd_symlink, pwd_symlink)
-        self.runCmd(
-            "settings set %s %s" %
-            (_COMP_DIR_SYM_LINK_PROP, pwd_symlink))
         src_path = self.getBuildArtifact(_SRC_FILE)
         lldbutil.run_break_set_by_file_and_line(self, src_path, self.line)
 
@@ -41,10 +38,7 @@ class CompDirSymLinkTestCase(TestBase):
     def test_symlink_paths_set_procselfcwd(self):
         os.chdir(self.getBuildDir())
         pwd_symlink = '/proc/self/cwd'
-        self.doBuild(pwd_symlink)
+        self.doBuild(pwd_symlink, pwd_symlink)
-        self.runCmd(
-            "settings set %s %s" %
-            (_COMP_DIR_SYM_LINK_PROP, pwd_symlink))
         src_path = self.getBuildArtifact(_SRC_FILE)
         # /proc/self/cwd points to a realpath form of current directory.
         src_path = os.path.realpath(src_path)
@@ -53,8 +47,7 @@ class CompDirSymLinkTestCase(TestBase):
     @skipIf(hostoslist=["windows"])
     def test_symlink_paths_unset(self):
         pwd_symlink = self.create_src_symlink()
-        self.doBuild(pwd_symlink)
+        self.doBuild(pwd_symlink, "")
-        self.runCmd('settings clear ' + _COMP_DIR_SYM_LINK_PROP)
         src_path = self.getBuildArtifact(_SRC_FILE)
         self.assertRaises(
             AssertionError,
@@ -71,8 +64,12 @@ class CompDirSymLinkTestCase(TestBase):
         self.addTearDownHook(lambda: os.remove(pwd_symlink))
         return pwd_symlink
 
-    def doBuild(self, pwd_symlink):
+    def doBuild(self, pwd_symlink, setting_value):
         self.build(None, None, {'PWD': pwd_symlink})
 
+        self.runCmd(
+            "settings set %s '%s'" %
+            (_COMP_DIR_SYM_LINK_PROP, setting_value))
+
         exe = self.getBuildArtifact(_EXE_NAME)
         self.runCmd('file ' + exe, CURRENT_EXECUTABLE_SET)

lldb/source/Core/CoreProperties.td

@@ -9,6 +9,10 @@ let Definition = "modulelist" in {
     Global,
     DefaultStringValue<"">,
     Desc<"The path to the clang modules cache directory (-fmodules-cache-path).">;
+  def SymLinkPaths: Property<"debug-info-symlink-paths", "FileSpecList">,
+    Global,
+    DefaultStringValue<"">,
+    Desc<"Debug info path which should be resolved while parsing, relative to the host filesystem.">;
 }
 
 let Definition = "debugger" in {

lldb/source/Core/ModuleList.cpp

@@ -12,6 +12,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/OptionValueFileSpec.h"
+#include "lldb/Interpreter/OptionValueFileSpecList.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Symbol/LocateSymbolFile.h"
@@ -77,6 +78,8 @@ ModuleListProperties::ModuleListProperties() {
   m_collection_sp =
       std::make_shared<OptionValueProperties>(ConstString("symbols"));
   m_collection_sp->Initialize(g_modulelist_properties);
+  m_collection_sp->SetValueChangedCallback(ePropertySymLinkPaths,
+                                           [this] { UpdateSymlinkMappings(); });
 
   llvm::SmallString<128> path;
   clang::driver::Driver::getDefaultModuleCachePath(path);
@@ -106,6 +109,28 @@ bool ModuleListProperties::SetClangModulesCachePath(llvm::StringRef path) {
       nullptr, ePropertyClangModulesCachePath, path);
 }
 
+void ModuleListProperties::UpdateSymlinkMappings() {
+  FileSpecList list = m_collection_sp
+                          ->GetPropertyAtIndexAsOptionValueFileSpecList(
+                              nullptr, false, ePropertySymLinkPaths)
+                          ->GetCurrentValue();
+  llvm::sys::ScopedWriter lock(m_symlink_paths_mutex);
+  const bool notify = false;
+  m_symlink_paths.Clear(notify);
+  for (FileSpec symlink : list) {
+    FileSpec resolved;
+    Status status = FileSystem::Instance().Readlink(symlink, resolved);
+    if (status.Success())
+      m_symlink_paths.Append(ConstString(symlink.GetPath()),
+                             ConstString(resolved.GetPath()), notify);
+  }
+}
+
+PathMappingList ModuleListProperties::GetSymlinkMappings() const {
+  llvm::sys::ScopedReader lock(m_symlink_paths_mutex);
+  return m_symlink_paths;
+}
+
 ModuleList::ModuleList()
     : m_modules(), m_modules_mutex(), m_notifier(nullptr) {}
 

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

@@ -253,7 +253,7 @@ static void SetupModuleHeaderPaths(CompilerInstance *compiler,
   }
 
   llvm::SmallString<128> module_cache;
-  auto props = ModuleList::GetGlobalModuleListProperties();
+  const auto &props = ModuleList::GetGlobalModuleListProperties();
   props.GetClangModulesCachePath().GetPath(module_cache);
   search_opts.ModuleCachePath = module_cache.str();
   LLDB_LOG(log, "Using module cache path: {0}", module_cache.c_str());

lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

@@ -601,7 +601,7 @@ ClangModulesDeclVendor::Create(Target &target) {
 
   {
     llvm::SmallString<128> path;
-    auto props = ModuleList::GetGlobalModuleListProperties();
+    const auto &props = ModuleList::GetGlobalModuleListProperties();
     props.GetClangModulesCachePath().GetPath(path);
     std::string module_cache_argument("-fmodules-cache-path=");
     module_cache_argument.append(path.str());

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

@@ -738,25 +738,6 @@ removeHostnameFromPathname(llvm::StringRef path_from_dwarf) {
   return path;
 }
 
-static FileSpec resolveCompDir(const FileSpec &path) {
-  bool is_symlink = SymbolFileDWARF::GetSymlinkPaths().FindFileIndex(
-                        0, path, /*full*/ true) != UINT32_MAX;
-
-  if (!is_symlink)
-    return path;
-
-  namespace fs = llvm::sys::fs;
-  if (fs::get_file_type(path.GetPath(), false) != fs::file_type::symlink_file)
-    return path;
-
-  FileSpec resolved_symlink;
-  const auto error = FileSystem::Instance().Readlink(path, resolved_symlink);
-  if (error.Success())
-    return resolved_symlink;
-
-  return path;
-}
-
 void DWARFUnit::ComputeCompDirAndGuessPathStyle() {
   m_comp_dir = FileSpec();
   const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
@@ -768,7 +749,7 @@ void DWARFUnit::ComputeCompDirAndGuessPathStyle() {
   if (!comp_dir.empty()) {
     FileSpec::Style comp_dir_style =
         FileSpec::GuessPathStyle(comp_dir).getValueOr(FileSpec::Style::native);
-    m_comp_dir = resolveCompDir(FileSpec(comp_dir, comp_dir_style));
+    m_comp_dir = FileSpec(comp_dir, comp_dir_style);
   } else {
     // Try to detect the style based on the DW_AT_name attribute, but just store
     // the detected style in the m_comp_dir field.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

@@ -135,14 +135,6 @@ public:
     m_collection_sp->Initialize(g_symbolfiledwarf_properties);
   }
 
-  FileSpecList GetSymLinkPaths() {
-    const OptionValueFileSpecList *option_value =
-        m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(
-            nullptr, true, ePropertySymLinkPaths);
-    assert(option_value);
-    return option_value->GetCurrentValue();
-  }
-
   bool IgnoreFileIndexes() const {
     return m_collection_sp->GetPropertyAtIndexAsBoolean(
         nullptr, ePropertyIgnoreIndexes, false);
@@ -227,10 +219,6 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP &module,
   return support_files;
 }
 
-FileSpecList SymbolFileDWARF::GetSymlinkPaths() {
-  return GetGlobalPluginProperties()->GetSymLinkPaths();
-}
-
 void SymbolFileDWARF::Initialize() {
   LogChannelDWARF::Initialize();
   PluginManager::RegisterPlugin(GetPluginNameStatic(),

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

@@ -90,8 +90,6 @@ public:
   static lldb_private::SymbolFile *
   CreateInstance(lldb::ObjectFileSP objfile_sp);
 
-  static lldb_private::FileSpecList GetSymlinkPaths();
-
   // Constructors and Destructors
 
   SymbolFileDWARF(lldb::ObjectFileSP objfile_sp,

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td

@@ -1,10 +1,6 @@
 include "../../../../include/lldb/Core/PropertiesBase.td"
 
 let Definition = "symbolfiledwarf" in {
-  def SymLinkPaths: Property<"comp-dir-symlink-paths", "FileSpecList">,
-    Global,
-    DefaultStringValue<"">,
-    Desc<"If the DW_AT_comp_dir matches any of these paths the symbolic links will be resolved at DWARF parse time.">;
   def IgnoreIndexes: Property<"ignore-file-indexes", "Boolean">,
     Global,
     DefaultFalse,