[Lldb-commits] [lldb] r243180 - 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.

Greg Clayton gclayton at apple.com
Fri Jul 24 16:38:02 PDT 2015


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());
+                }
             }
         }
     }





More information about the lldb-commits mailing list