[Lldb-commits] [lldb] r359028 - Lock accesses to OptionValueFileSpecList objects
Frederic Riss via lldb-commits
lldb-commits at lists.llvm.org
Tue Apr 23 13:17:04 PDT 2019
Author: friss
Date: Tue Apr 23 13:17:04 2019
New Revision: 359028
URL: http://llvm.org/viewvc/llvm-project?rev=359028&view=rev
Log:
Lock accesses to OptionValueFileSpecList objects
Before a Debugger gets a Target, target settings are routed to a global set
of settings. Even without this, some part of the LLDB which exist independently
of the Debugger object (the Module cache, the Symbol vendors, ...) access
directly the global default store for those settings.
Of course, if you modify one of those global settings while they are being read,
bad things happen. We see this quite a bit with FileSpecList settings. In
particular, we see many cases where one debug session changes
target.exec-search-paths while another session starts up and it crashes when
one of those accesses invalid FileSpecs.
This patch addresses the specific FileSpecList issue by adding locking to
OptionValueFileSpecList and never returning by reference.
Reviewers: clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D60468
Modified:
lldb/trunk/include/lldb/Interpreter/OptionValueFileSpecList.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/include/lldb/Target/Thread.h
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Target/Target.cpp
lldb/trunk/source/Target/TargetList.cpp
lldb/trunk/source/Target/Thread.cpp
Modified: lldb/trunk/include/lldb/Interpreter/OptionValueFileSpecList.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/OptionValueFileSpecList.h?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Interpreter/OptionValueFileSpecList.h (original)
+++ lldb/trunk/include/lldb/Interpreter/OptionValueFileSpecList.h Tue Apr 23 13:17:04 2019
@@ -9,6 +9,8 @@
#ifndef liblldb_OptionValueFileSpecList_h_
#define liblldb_OptionValueFileSpecList_h_
+#include <mutex>
+
#include "lldb/Core/FileSpecList.h"
#include "lldb/Interpreter/OptionValue.h"
@@ -49,13 +51,23 @@ public:
// Subclass specific functions
- FileSpecList &GetCurrentValue() { return m_current_value; }
+ FileSpecList GetCurrentValue() const {
+ std::lock_guard<std::mutex> lock(m_mutex);
+ return m_current_value;
+ }
- const FileSpecList &GetCurrentValue() const { return m_current_value; }
+ void SetCurrentValue(const FileSpecList &value) {
+ std::lock_guard<std::mutex> lock(m_mutex);
+ m_current_value = value;
+ }
- void SetCurrentValue(const FileSpecList &value) { m_current_value = value; }
+ void AppendCurrentValue(const FileSpec &value) {
+ std::lock_guard<std::mutex> lock(m_mutex);
+ m_current_value.Append(value);
+ }
protected:
+ mutable std::mutex m_mutex;
FileSpecList m_current_value;
};
Modified: lldb/trunk/include/lldb/Target/Target.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Tue Apr 23 13:17:04 2019
@@ -119,11 +119,13 @@ public:
PathMappingList &GetSourcePathMap() const;
- FileSpecList &GetExecutableSearchPaths();
+ FileSpecList GetExecutableSearchPaths();
- FileSpecList &GetDebugFileSearchPaths();
+ void AppendExecutableSearchPaths(const FileSpec&);
- FileSpecList &GetClangModuleSearchPaths();
+ FileSpecList GetDebugFileSearchPaths();
+
+ FileSpecList GetClangModuleSearchPaths();
bool GetEnableAutoImportClangModules() const;
Modified: lldb/trunk/include/lldb/Target/Thread.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Thread.h?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Thread.h (original)
+++ lldb/trunk/include/lldb/Target/Thread.h Tue Apr 23 13:17:04 2019
@@ -43,7 +43,7 @@ public:
///
const RegularExpression *GetSymbolsToAvoidRegexp();
- FileSpecList &GetLibrariesToAvoid() const;
+ FileSpecList GetLibrariesToAvoid() const;
bool GetTraceEnabledState() const;
Modified: lldb/trunk/source/Commands/CommandObjectTarget.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectTarget.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectTarget.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectTarget.cpp Tue Apr 23 13:17:04 2019
@@ -410,7 +410,7 @@ protected:
}
FileSpec core_file_dir;
core_file_dir.GetDirectory() = core_file.GetDirectory();
- target_sp->GetExecutableSearchPaths().Append(core_file_dir);
+ target_sp->AppendExecutableSearchPaths(core_file_dir);
ProcessSP process_sp(target_sp->CreateProcess(
m_interpreter.GetDebugger().GetListener(), llvm::StringRef(),
Modified: lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp (original)
+++ lldb/trunk/source/Interpreter/OptionValueFileSpecLIst.cpp Tue Apr 23 13:17:04 2019
@@ -163,5 +163,6 @@ Status OptionValueFileSpecList::SetValue
}
lldb::OptionValueSP OptionValueFileSpecList::DeepCopy() const {
- return OptionValueSP(new OptionValueFileSpecList(*this));
+ std::lock_guard<std::mutex> lock(m_mutex);
+ return OptionValueSP(new OptionValueFileSpecList(m_current_value));
}
Modified: lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp (original)
+++ lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp Tue Apr 23 13:17:04 2019
@@ -804,10 +804,11 @@ bool DynamicLoaderDarwinKernel::KextImag
if (platform_name == g_platform_name) {
ModuleSpec kext_bundle_module_spec(module_spec);
FileSpec kext_filespec(m_name.c_str());
+ FileSpecList search_paths = target.GetExecutableSearchPaths();
kext_bundle_module_spec.GetFileSpec() = kext_filespec;
platform_sp->GetSharedModule(
kext_bundle_module_spec, process, m_module_sp,
- &target.GetExecutableSearchPaths(), NULL, NULL);
+ &search_paths, NULL, NULL);
}
}
Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp Tue Apr 23 13:17:04 2019
@@ -608,7 +608,7 @@ ClangModulesDeclVendor::Create(Target &t
compiler_invocation_arguments.push_back(module_cache_argument);
}
- FileSpecList &module_search_paths = target.GetClangModuleSearchPaths();
+ FileSpecList module_search_paths = target.GetClangModuleSearchPaths();
for (size_t spi = 0, spe = module_search_paths.GetSize(); spi < spe; ++spi) {
const FileSpec &search_path = module_search_paths.GetFileSpecAtIndex(spi);
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp Tue Apr 23 13:17:04 2019
@@ -208,9 +208,9 @@ public:
NULL, idx, g_properties[idx].default_uint_value != 0);
}
- FileSpecList &GetKextDirectories() const {
+ FileSpecList GetKextDirectories() const {
const uint32_t idx = ePropertyKextDirectories;
- OptionValueFileSpecList *option_value =
+ const OptionValueFileSpecList *option_value =
m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(
NULL, false, idx);
assert(option_value);
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Tue Apr 23 13:17:04 2019
@@ -136,8 +136,8 @@ public:
m_collection_sp->Initialize(g_properties);
}
- FileSpecList &GetSymLinkPaths() {
- OptionValueFileSpecList *option_value =
+ FileSpecList GetSymLinkPaths() {
+ const OptionValueFileSpecList *option_value =
m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(
nullptr, true, ePropertySymLinkPaths);
assert(option_value);
@@ -159,7 +159,7 @@ static const SymbolFileDWARFPropertiesSP
} // anonymous namespace end
-const FileSpecList &SymbolFileDWARF::GetSymlinkPaths() {
+FileSpecList SymbolFileDWARF::GetSymlinkPaths() {
return GetGlobalPluginProperties()->GetSymLinkPaths();
}
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h Tue Apr 23 13:17:04 2019
@@ -80,7 +80,7 @@ public:
static lldb_private::SymbolFile *
CreateInstance(lldb_private::ObjectFile *obj_file);
- static const lldb_private::FileSpecList &GetSymlinkPaths();
+ static lldb_private::FileSpecList GetSymlinkPaths();
// Constructors and Destructors
Modified: lldb/trunk/source/Target/Target.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Tue Apr 23 13:17:04 2019
@@ -1574,8 +1574,9 @@ bool Target::SetArchitecture(const ArchS
arch_spec.GetArchitectureName(),
arch_spec.GetTriple().getTriple().c_str());
ModuleSpec module_spec(executable_sp->GetFileSpec(), other);
+ FileSpecList search_paths = GetExecutableSearchPaths();
Status error = ModuleList::GetSharedModule(module_spec, executable_sp,
- &GetExecutableSearchPaths(),
+ &search_paths,
nullptr, nullptr);
if (!error.Fail() && executable_sp) {
@@ -2015,7 +2016,7 @@ ModuleSP Target::GetOrCreateModule(const
ModuleSP old_module_sp; // This will get filled in if we have a new version
// of the library
bool did_create_module = false;
-
+ FileSpecList search_paths = GetExecutableSearchPaths();
// If there are image search path entries, try to use them first to acquire
// a suitable image.
if (m_image_search_paths.GetSize()) {
@@ -2026,7 +2027,7 @@ ModuleSP Target::GetOrCreateModule(const
transformed_spec.GetFileSpec().GetFilename() =
module_spec.GetFileSpec().GetFilename();
error = ModuleList::GetSharedModule(transformed_spec, module_sp,
- &GetExecutableSearchPaths(),
+ &search_paths,
&old_module_sp, &did_create_module);
}
}
@@ -2043,7 +2044,7 @@ ModuleSP Target::GetOrCreateModule(const
if (module_spec.GetUUID().IsValid()) {
// We have a UUID, it is OK to check the global module list...
error = ModuleList::GetSharedModule(module_spec, module_sp,
- &GetExecutableSearchPaths(),
+ &search_paths,
&old_module_sp, &did_create_module);
}
@@ -2053,7 +2054,7 @@ ModuleSP Target::GetOrCreateModule(const
if (m_platform_sp) {
error = m_platform_sp->GetSharedModule(
module_spec, m_process_sp.get(), module_sp,
- &GetExecutableSearchPaths(), &old_module_sp, &did_create_module);
+ &search_paths, &old_module_sp, &did_create_module);
} else {
error.SetErrorString("no platform is currently set");
}
@@ -3865,27 +3866,36 @@ PathMappingList &TargetProperties::GetSo
return option_value->GetCurrentValue();
}
-FileSpecList &TargetProperties::GetExecutableSearchPaths() {
+void TargetProperties::AppendExecutableSearchPaths(const FileSpec& dir) {
const uint32_t idx = ePropertyExecutableSearchPaths;
OptionValueFileSpecList *option_value =
m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr,
false, idx);
assert(option_value);
+ option_value->AppendCurrentValue(dir);
+}
+
+FileSpecList TargetProperties::GetExecutableSearchPaths() {
+ const uint32_t idx = ePropertyExecutableSearchPaths;
+ const OptionValueFileSpecList *option_value =
+ m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr,
+ false, idx);
+ assert(option_value);
return option_value->GetCurrentValue();
}
-FileSpecList &TargetProperties::GetDebugFileSearchPaths() {
+FileSpecList TargetProperties::GetDebugFileSearchPaths() {
const uint32_t idx = ePropertyDebugFileSearchPaths;
- OptionValueFileSpecList *option_value =
+ const OptionValueFileSpecList *option_value =
m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr,
false, idx);
assert(option_value);
return option_value->GetCurrentValue();
}
-FileSpecList &TargetProperties::GetClangModuleSearchPaths() {
+FileSpecList TargetProperties::GetClangModuleSearchPaths() {
const uint32_t idx = ePropertyClangModuleSearchPaths;
- OptionValueFileSpecList *option_value =
+ const OptionValueFileSpecList *option_value =
m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr,
false, idx);
assert(option_value);
Modified: lldb/trunk/source/Target/TargetList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/TargetList.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Target/TargetList.cpp (original)
+++ lldb/trunk/source/Target/TargetList.cpp Tue Apr 23 13:17:04 2019
@@ -422,7 +422,7 @@ Status TargetList::CreateTargetInternal(
if (file.GetDirectory()) {
FileSpec file_dir;
file_dir.GetDirectory() = file.GetDirectory();
- target_sp->GetExecutableSearchPaths().Append(file_dir);
+ target_sp->AppendExecutableSearchPaths(file_dir);
}
// Don't put the dummy target in the target list, it's held separately.
Modified: lldb/trunk/source/Target/Thread.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cpp?rev=359028&r1=359027&r2=359028&view=diff
==============================================================================
--- lldb/trunk/source/Target/Thread.cpp (original)
+++ lldb/trunk/source/Target/Thread.cpp Tue Apr 23 13:17:04 2019
@@ -138,9 +138,9 @@ const RegularExpression *ThreadPropertie
return m_collection_sp->GetPropertyAtIndexAsOptionValueRegex(nullptr, idx);
}
-FileSpecList &ThreadProperties::GetLibrariesToAvoid() const {
+FileSpecList ThreadProperties::GetLibrariesToAvoid() const {
const uint32_t idx = ePropertyStepAvoidLibraries;
- OptionValueFileSpecList *option_value =
+ const OptionValueFileSpecList *option_value =
m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(nullptr,
false, idx);
assert(option_value);
More information about the lldb-commits
mailing list