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); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
