[Lldb-commits] [lldb] 27df2d9 - [lldb] Don't process symlinks deep inside DWARFUnit
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Jan 20 04:06:09 PST 2020
Author: Pavel Labath
Date: 2020-01-20T13:05:00+01:00
New Revision: 27df2d9f556c3199601ecd1f15c1b37cd49ed9df
URL: https://github.com/llvm/llvm-project/commit/27df2d9f556c3199601ecd1f15c1b37cd49ed9df
DIFF: https://github.com/llvm/llvm-project/commit/27df2d9f556c3199601ecd1f15c1b37cd49ed9df.diff
LOG: [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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 2af18c83f23a..08bc569d0db1 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 @@ class Module : public std::enable_shared_from_this<Module>,
///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 a6e80ed75c76..bdcef3061964 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 <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 @@ class ModuleListProperties : public Properties {
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 0e372c732494..006c0bff0838 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 @@
_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 @@ def setUp(self):
@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(self):
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 @@ def test_symlink_paths_set_procselfcwd(self):
@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 @@ def create_src_symlink(self):
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 014927c65c6f..bfb8291ba4b2 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 07100bb81dca..4850dbdf5b85 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<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) {}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 8abd14942885..8dfdd762e931 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 0696c669f2e2..40bf740935a6 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 22e3e40dac93..4ac07515a21c 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 493a3160376f..a07297414e05 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -135,14 +135,6 @@ class PluginProperties : public Properties {
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 1336c8cb0220..5183ad33f3a9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -90,8 +90,6 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
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 ef6ae3498588..2f1ce88808b7 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,
More information about the lldb-commits
mailing list