Author: gclayton Date: Fri Jul 24 18:38:01 2015 New Revision: 243180 URL: http://llvm.org/viewvc/llvm-project?rev=243180&view=rev Log: Use double-checked locking to avoid locking the Module mutex if we don't need to. This avoid a deadlock we were seeing in Xcode.
<rdar://problem/21512067> Modified: lldb/trunk/include/lldb/Core/Module.h lldb/trunk/source/Core/Module.cpp Modified: lldb/trunk/include/lldb/Core/Module.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=243180&r1=243179&r2=243180&view=diff ============================================================================== --- lldb/trunk/include/lldb/Core/Module.h (original) +++ lldb/trunk/include/lldb/Core/Module.h Fri Jul 24 18:38:01 2015 @@ -10,6 +10,7 @@ #ifndef liblldb_Module_h_ #define liblldb_Module_h_ +#include <atomic> #include "lldb/lldb-forward.h" #include "lldb/Core/ArchSpec.h" #include "lldb/Core/UUID.h" @@ -1119,13 +1120,13 @@ protected: 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 lldb::SectionListUP m_sections_ap; ///< Unified section list for module that is used by the ObjectFile and and ObjectFile instances for the debug info - bool m_did_load_objfile:1, - m_did_load_symbol_vendor:1, - m_did_parse_uuid:1, - m_did_init_ast:1; + std::atomic<bool> m_did_load_objfile; + std::atomic<bool> m_did_load_symbol_vendor; + std::atomic<bool> m_did_parse_uuid; + std::atomic<bool> m_did_init_ast; mutable bool m_file_has_changed:1, m_first_file_changed_log:1; /// See if the module was modified after it was initially opened. - + //------------------------------------------------------------------ /// Resolve a file or load virtual address. /// Modified: lldb/trunk/source/Core/Module.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=243180&r1=243179&r2=243180&view=diff ============================================================================== --- lldb/trunk/source/Core/Module.cpp (original) +++ lldb/trunk/source/Core/Module.cpp Fri Jul 24 18:38:01 2015 @@ -399,15 +399,18 @@ Module::GetMemoryObjectFile (const lldb: const lldb_private::UUID& Module::GetUUID() { - Mutex::Locker locker (m_mutex); - if (m_did_parse_uuid == false) + if (m_did_parse_uuid.load() == false) { - ObjectFile * obj_file = GetObjectFile (); - - if (obj_file != NULL) + Mutex::Locker locker (m_mutex); + if (m_did_parse_uuid.load() == false) { - obj_file->GetUUID(&m_uuid); - m_did_parse_uuid = true; + ObjectFile * obj_file = GetObjectFile (); + + if (obj_file != NULL) + { + obj_file->GetUUID(&m_uuid); + m_did_parse_uuid = true; + } } } return m_uuid; @@ -416,32 +419,35 @@ Module::GetUUID() ClangASTContext & Module::GetClangASTContext () { - Mutex::Locker locker (m_mutex); - if (m_did_init_ast == false) + if (m_did_init_ast.load() == false) { - ObjectFile * objfile = GetObjectFile(); - ArchSpec object_arch; - if (objfile && objfile->GetArchitecture(object_arch)) - { - m_did_init_ast = true; - - // LLVM wants this to be set to iOS or MacOSX; if we're working on - // a bare-boards type image, change the triple for llvm's benefit. - if (object_arch.GetTriple().getVendor() == llvm::Triple::Apple - && object_arch.GetTriple().getOS() == llvm::Triple::UnknownOS) - { - if (object_arch.GetTriple().getArch() == llvm::Triple::arm || - object_arch.GetTriple().getArch() == llvm::Triple::aarch64 || - object_arch.GetTriple().getArch() == llvm::Triple::thumb) - { - object_arch.GetTriple().setOS(llvm::Triple::IOS); - } - else + Mutex::Locker locker (m_mutex); + if (m_did_init_ast.load() == false) + { + ObjectFile * objfile = GetObjectFile(); + ArchSpec object_arch; + if (objfile && objfile->GetArchitecture(object_arch)) + { + m_did_init_ast = true; + + // LLVM wants this to be set to iOS or MacOSX; if we're working on + // a bare-boards type image, change the triple for llvm's benefit. + if (object_arch.GetTriple().getVendor() == llvm::Triple::Apple + && object_arch.GetTriple().getOS() == llvm::Triple::UnknownOS) { - object_arch.GetTriple().setOS(llvm::Triple::MacOSX); + if (object_arch.GetTriple().getArch() == llvm::Triple::arm || + object_arch.GetTriple().getArch() == llvm::Triple::aarch64 || + object_arch.GetTriple().getArch() == llvm::Triple::thumb) + { + object_arch.GetTriple().setOS(llvm::Triple::IOS); + } + else + { + object_arch.GetTriple().setOS(llvm::Triple::MacOSX); + } } + m_ast->SetArchitecture (object_arch); } - m_ast->SetArchitecture (object_arch); } } return *m_ast; @@ -1039,15 +1045,18 @@ Module::FindTypes (const SymbolContext& SymbolVendor* Module::GetSymbolVendor (bool can_create, lldb_private::Stream *feedback_strm) { - Mutex::Locker locker (m_mutex); - if (m_did_load_symbol_vendor == false && can_create) + if (m_did_load_symbol_vendor.load() == false) { - ObjectFile *obj_file = GetObjectFile (); - if (obj_file != NULL) + Mutex::Locker locker (m_mutex); + if (m_did_load_symbol_vendor.load() == false && can_create) { - Timer scoped_timer(__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); - m_symfile_ap.reset(SymbolVendor::FindPlugin(shared_from_this(), feedback_strm)); - m_did_load_symbol_vendor = true; + ObjectFile *obj_file = GetObjectFile (); + if (obj_file != NULL) + { + Timer scoped_timer(__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); + m_symfile_ap.reset(SymbolVendor::FindPlugin(shared_from_this(), feedback_strm)); + m_did_load_symbol_vendor = true; + } } } return m_symfile_ap.get(); @@ -1288,37 +1297,40 @@ Module::GetObjectName() const ObjectFile * Module::GetObjectFile() { - Mutex::Locker locker (m_mutex); - if (m_did_load_objfile == false) + if (m_did_load_objfile.load() == false) { - Timer scoped_timer(__PRETTY_FUNCTION__, - "Module::GetObjectFile () module = %s", GetFileSpec().GetFilename().AsCString("")); - DataBufferSP data_sp; - lldb::offset_t data_offset = 0; - const lldb::offset_t file_size = m_file.GetByteSize(); - if (file_size > m_object_offset) - { - m_did_load_objfile = true; - m_objfile_sp = ObjectFile::FindPlugin (shared_from_this(), - &m_file, - m_object_offset, - file_size - m_object_offset, - data_sp, - data_offset); - if (m_objfile_sp) - { - // Once we get the object file, update our module with the object file's - // architecture since it might differ in vendor/os if some parts were - // unknown. But since the matching arch might already be more specific - // than the generic COFF architecture, only merge in those values that - // overwrite unspecified unknown values. - ArchSpec new_arch; - m_objfile_sp->GetArchitecture(new_arch); - m_arch.MergeFrom(new_arch); - } - else - { - ReportError ("failed to load objfile for %s", GetFileSpec().GetPath().c_str()); + Mutex::Locker locker (m_mutex); + if (m_did_load_objfile.load() == false) + { + Timer scoped_timer(__PRETTY_FUNCTION__, + "Module::GetObjectFile () module = %s", GetFileSpec().GetFilename().AsCString("")); + DataBufferSP data_sp; + lldb::offset_t data_offset = 0; + const lldb::offset_t file_size = m_file.GetByteSize(); + if (file_size > m_object_offset) + { + m_did_load_objfile = true; + m_objfile_sp = ObjectFile::FindPlugin (shared_from_this(), + &m_file, + m_object_offset, + file_size - m_object_offset, + data_sp, + data_offset); + if (m_objfile_sp) + { + // Once we get the object file, update our module with the object file's + // architecture since it might differ in vendor/os if some parts were + // unknown. But since the matching arch might already be more specific + // than the generic COFF architecture, only merge in those values that + // overwrite unspecified unknown values. + ArchSpec new_arch; + m_objfile_sp->GetArchitecture(new_arch); + m_arch.MergeFrom(new_arch); + } + else + { + ReportError ("failed to load objfile for %s", GetFileSpec().GetPath().c_str()); + } } } } _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits