This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rG27df2d9f556c: [lldb] Don't process symlinks deep inside 
DWARFUnit (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D71770?vs=237076&id=239067#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71770/new/

https://reviews.llvm.org/D71770

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleList.h
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/ModuleList.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td
+++ 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,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -90,8 +90,6 @@
   static lldb_private::SymbolFile *
   CreateInstance(lldb::ObjectFileSP objfile_sp);
 
-  static lldb_private::FileSpecList GetSymlinkPaths();
-
   // Constructors and Destructors
 
   SymbolFileDWARF(lldb::ObjectFileSP objfile_sp,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -135,14 +135,6 @@
     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 @@
   return support_files;
 }
 
-FileSpecList SymbolFileDWARF::GetSymlinkPaths() {
-  return GetGlobalPluginProperties()->GetSymLinkPaths();
-}
-
 void SymbolFileDWARF::Initialize() {
   LogChannelDWARF::Initialize();
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -738,25 +738,6 @@
   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 @@
   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.
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -601,7 +601,7 @@
 
   {
     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());
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -253,7 +253,7 @@
   }
 
   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());
Index: lldb/source/Core/ModuleList.cpp
===================================================================
--- lldb/source/Core/ModuleList.cpp
+++ 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 @@
   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 @@
       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) {}
 
Index: lldb/source/Core/CoreProperties.td
===================================================================
--- lldb/source/Core/CoreProperties.td
+++ lldb/source/Core/CoreProperties.td
@@ -9,6 +9,10 @@
     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 {
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
@@ -12,7 +12,7 @@
 
 _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 @@
     @skipIf(hostoslist=["windows"])
     def test_symlink_paths_set(self):
         pwd_symlink = self.create_src_symlink()
-        self.doBuild(pwd_symlink)
-        self.runCmd(
-            "settings set %s %s" %
-            (_COMP_DIR_SYM_LINK_PROP, pwd_symlink))
+        self.doBuild(pwd_symlink, 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 @@
     def test_symlink_paths_set_procselfcwd(self):
         os.chdir(self.getBuildDir())
         pwd_symlink = '/proc/self/cwd'
-        self.doBuild(pwd_symlink)
-        self.runCmd(
-            "settings set %s %s" %
-            (_COMP_DIR_SYM_LINK_PROP, pwd_symlink))
+        self.doBuild(pwd_symlink, 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 @@
     @skipIf(hostoslist=["windows"])
     def test_symlink_paths_unset(self):
         pwd_symlink = self.create_src_symlink()
-        self.doBuild(pwd_symlink)
-        self.runCmd('settings clear ' + _COMP_DIR_SYM_LINK_PROP)
+        self.doBuild(pwd_symlink, "")
         src_path = self.getBuildArtifact(_SRC_FILE)
         self.assertRaises(
             AssertionError,
@@ -71,8 +64,12 @@
         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)
Index: lldb/include/lldb/Core/ModuleList.h
===================================================================
--- lldb/include/lldb/Core/ModuleList.h
+++ 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 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 @@
   bool SetClangModulesCachePath(llvm::StringRef path);
   bool GetEnableExternalLookup() const;
   bool SetEnableExternalLookup(bool new_value);
+
+  PathMappingList GetSymlinkMappings() const;
 };
 
 /// \class ModuleList ModuleList.h "lldb/Core/ModuleList.h"
Index: lldb/include/lldb/Core/Module.h
===================================================================
--- lldb/include/lldb/Core/Module.h
+++ 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 @@
   ///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
-                                     ///when you have debug info for a module
-                                     ///that doesn't match where the sources
-                                     ///currently are
+  /// Module specific source remappings for when you have debug info for a
+  /// module that doesn't match where the sources currently are.
+  PathMappingList m_source_mappings =
+      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-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to