From 27df2d9f556c3199601ecd1f15c1b37cd49ed9df Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Fri, 20 Dec 2019 16:34:55 +0100 Subject: [PATCH] [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 | 9 ++++---- lldb/include/lldb/Core/ModuleList.h | 8 +++++++ .../comp_dir_symlink/TestCompDirSymLink.py | 21 ++++++++---------- lldb/source/Core/CoreProperties.td | 4 ++++ lldb/source/Core/ModuleList.cpp | 25 ++++++++++++++++++++++ .../Clang/ClangExpressionParser.cpp | 2 +- .../Clang/ClangModulesDeclVendor.cpp | 2 +- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp | 21 +----------------- .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 12 ----------- .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.h | 2 -- .../SymbolFile/DWARF/SymbolFileDWARFProperties.td | 4 ---- 11 files changed, 54 insertions(+), 56 deletions(-) diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h index 2af18c83f23..08bc569d0db 100644 --- a/lldb/include/lldb/Core/Module.h +++ b/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 - ///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 diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h index a6e80ed75c7..bdcef306196 100644 --- a/lldb/include/lldb/Core/ModuleList.h +++ b/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 #include @@ -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" diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py index 0e372c73249..006c0bff083 100644 --- a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py +++ b/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.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 @@ class CompDirSymLinkTestCase(TestBase): 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 @@ class CompDirSymLinkTestCase(TestBase): @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 @@ 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) diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td index 014927c65c6..bfb8291ba4b 100644 --- a/lldb/source/Core/CoreProperties.td +++ b/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 { diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp index 07100bb81dc..4850dbdf5b8 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/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(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) {} diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 8abd1494288..8dfdd762e93 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/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()); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp index 0696c669f2e..40bf740935a 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp +++ b/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()); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp index 22e3e40dac9..4ac07515a21 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp +++ b/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. diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 493a3160376..a07297414e0 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/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(), diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 1336c8cb022..5183ad33f3a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/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, diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td index ef6ae349858..2f1ce88808b 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td +++ b/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, -- 2.11.0