[Lldb-commits] [lldb] r253618 - Fix a crasher in SymbolContext::SortTypeList() where something that was iterating over a std::multimap was actually mutating the list.

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 19 15:10:45 PST 2015


Author: gclayton
Date: Thu Nov 19 17:10:45 2015
New Revision: 253618

URL: http://llvm.org/viewvc/llvm-project?rev=253618&view=rev
Log:
Fix a crasher in SymbolContext::SortTypeList() where something that was iterating over a std::multimap was actually mutating the list.

<rdar://problem/23605600>


Modified:
    lldb/trunk/include/lldb/Symbol/TypeMap.h
    lldb/trunk/source/Symbol/SymbolContext.cpp
    lldb/trunk/source/Symbol/TypeMap.cpp

Modified: lldb/trunk/include/lldb/Symbol/TypeMap.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/TypeMap.h?rev=253618&r1=253617&r2=253618&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/TypeMap.h (original)
+++ lldb/trunk/include/lldb/Symbol/TypeMap.h Thu Nov 19 17:10:45 2015
@@ -35,9 +35,6 @@ public:
     void
     Dump(Stream *s, bool show_context);
 
-//    lldb::TypeSP
-//    FindType(lldb::user_id_t uid);
-
     TypeMap
     FindTypes(const ConstString &name);
 
@@ -72,7 +69,7 @@ public:
     ForEach (std::function <bool(lldb::TypeSP &type_sp)> const &callback);
 
     bool
-    RemoveTypeWithUID (lldb::user_id_t uid);
+    Remove (const lldb::TypeSP &type_sp);
 
     void
     RemoveMismatchedTypes (const char *qualified_typename,

Modified: lldb/trunk/source/Symbol/SymbolContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/SymbolContext.cpp?rev=253618&r1=253617&r2=253618&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/SymbolContext.cpp (original)
+++ lldb/trunk/source/Symbol/SymbolContext.cpp Thu Nov 19 17:10:45 2015
@@ -659,172 +659,114 @@ SymbolContext::GetFunctionMethodInfo (ll
     return false;
 }
 
-class TypeMoveMatchingBlock
-{
-public:
-    TypeMoveMatchingBlock(Block * block, TypeMap &typem, TypeList &typel) :
-        curr_block(block),type_map(typem),type_list(typel)
-    {
-    }
-
-    bool
-    operator() (const lldb::TypeSP& type)
-    {
-        if (type && type->GetSymbolContextScope() != nullptr && curr_block == type->GetSymbolContextScope()->CalculateSymbolContextBlock())
-        {
-            type_list.Insert(type);
-            type_map.RemoveTypeWithUID(type->GetID());
-            return false;
-        }
-        return true;
-    }
-
-private:
-    const Block * const curr_block;
-    TypeMap &type_map;
-    TypeList &type_list;
-};
-
-class TypeMoveMatchingFunction
-{
-public:
-    TypeMoveMatchingFunction(Function * block, TypeMap &typem, TypeList &typel) :
-        func(block),type_map(typem),type_list(typel)
-    {
-    }
-
-    bool
-    operator() (const lldb::TypeSP& type)
-    {
-        if (type && type->GetSymbolContextScope() != nullptr && func == type->GetSymbolContextScope()->CalculateSymbolContextFunction())
-        {
-            type_list.Insert(type);
-            type_map.RemoveTypeWithUID(type->GetID());
-            return false;
-        }
-        return true;
-    }
-
-private:
-    const Function * const func;
-    TypeMap &type_map;
-    TypeList &type_list;
-};
-
-class TypeMoveMatchingCompileUnit
-{
-public:
-    TypeMoveMatchingCompileUnit(CompileUnit * cunit, TypeMap &typem, TypeList &typel) :
-        comp_unit(cunit),type_map(typem),type_list(typel)
-    {
-    }
-
-    bool
-    operator() (const lldb::TypeSP& type)
-    {
-        if (type && type->GetSymbolContextScope() != nullptr && comp_unit == type->GetSymbolContextScope()->CalculateSymbolContextCompileUnit())
-        {
-            type_list.Insert(type);
-            type_map.RemoveTypeWithUID(type->GetID());
-            return false;
-        }
-        return true;
-    }
-
-private:
-    const CompileUnit * const comp_unit;
-    TypeMap &type_map;
-    TypeList &type_list;
-};
-
-class TypeMoveMatchingModule
-{
-public:
-    TypeMoveMatchingModule(lldb::ModuleSP modsp, TypeMap &typem, TypeList &typel) :
-        modulesp(modsp),type_map(typem),type_list(typel)
-    {
-    }
-
-    bool
-    operator() (const lldb::TypeSP& type)
-    {
-        if (type && type->GetSymbolContextScope() != nullptr && modulesp.get() == type->GetSymbolContextScope()->CalculateSymbolContextModule().get())
-        {
-            type_list.Insert(type);
-            type_map.RemoveTypeWithUID(type->GetID());
-            return false;
-        }
-        return true;
-    }
-
-private:
-    lldb::ModuleSP modulesp;
-    TypeMap &type_map;
-    TypeList &type_list;
-};
-
-class TypeMaptoList
-{
-public:
-    TypeMaptoList(TypeMap &typem, TypeList &typel) :
-        type_map(typem),type_list(typel)
-    {
-    }
-
-    bool
-    operator() (const lldb::TypeSP& type)
-    {
-        if(type)
-        {
-            type_list.Insert(type);
-            type_map.RemoveTypeWithUID(type->GetID());
-            if (type_map.Empty())
-                return false;
-        }
-        return true;
-    }
-
-private:
-    TypeMap &type_map;
-    TypeList &type_list;
-};
-
 void
-SymbolContext::SortTypeList(TypeMap &type_map, TypeList &type_list ) const
+SymbolContext::SortTypeList(TypeMap &type_map, TypeList &type_list) const
 {
     Block * curr_block = block;
     bool isInlinedblock = false;
     if (curr_block != nullptr && curr_block->GetContainingInlinedBlock() != nullptr)
         isInlinedblock = true;
 
+    //----------------------------------------------------------------------
+    // Find all types that match the current block if we have one and put
+    // them first in the list. Keep iterating up through all blocks.
+    //----------------------------------------------------------------------
     while (curr_block != nullptr && !isInlinedblock)
     {
-        TypeMoveMatchingBlock callbackBlock (curr_block, type_map, type_list);
-        type_map.ForEach(callbackBlock);
+        type_map.ForEach([curr_block, &type_list](const lldb::TypeSP& type_sp) -> bool {
+            SymbolContextScope *scs = type_sp->GetSymbolContextScope();
+            if (scs && curr_block == scs->CalculateSymbolContextBlock())
+                type_list.Insert(type_sp);
+            return true; // Keep iterating
+        });
+
+        // Remove any entries that are now in "type_list" from "type_map"
+        // since we can't remove from type_map while iterating
+        type_list.ForEach([&type_map](const lldb::TypeSP& type_sp) -> bool {
+            type_map.Remove(type_sp);
+            return true; // Keep iterating
+        });
         curr_block = curr_block->GetParent();
     }
-    if(function != nullptr && type_map.GetSize() > 0)
-    {
-        TypeMoveMatchingFunction callbackFunction (function, type_map, type_list);
-        type_map.ForEach(callbackFunction);
+    //----------------------------------------------------------------------
+    // Find all types that match the current function, if we have onem, and
+    // put them next in the list.
+    //----------------------------------------------------------------------
+    if (function != nullptr && !type_map.Empty())
+    {
+        const size_t old_type_list_size = type_list.GetSize();
+        type_map.ForEach([this, &type_list](const lldb::TypeSP& type_sp) -> bool {
+            SymbolContextScope *scs = type_sp->GetSymbolContextScope();
+            if (scs && function == scs->CalculateSymbolContextFunction())
+                type_list.Insert(type_sp);
+            return true; // Keep iterating
+        });
+
+        // Remove any entries that are now in "type_list" from "type_map"
+        // since we can't remove from type_map while iterating
+        const size_t new_type_list_size = type_list.GetSize();
+        if (new_type_list_size > old_type_list_size)
+        {
+            for (size_t i=old_type_list_size; i<new_type_list_size; ++i)
+                type_map.Remove(type_list.GetTypeAtIndex(i));
+        }
     }
-    if(comp_unit != nullptr && type_map.GetSize() > 0)
-    {
-        TypeMoveMatchingCompileUnit callbackCompileUnit (comp_unit, type_map, type_list);
-        type_map.ForEach(callbackCompileUnit);
+    //----------------------------------------------------------------------
+    // Find all types that match the current compile unit, if we have one,
+    // and put them next in the list.
+    //----------------------------------------------------------------------
+    if (comp_unit != nullptr && !type_map.Empty())
+    {
+        const size_t old_type_list_size = type_list.GetSize();
+
+        type_map.ForEach([this, &type_list](const lldb::TypeSP& type_sp) -> bool {
+            SymbolContextScope *scs = type_sp->GetSymbolContextScope();
+            if (scs && comp_unit == scs->CalculateSymbolContextCompileUnit())
+                type_list.Insert(type_sp);
+            return true; // Keep iterating
+        });
+
+        // Remove any entries that are now in "type_list" from "type_map"
+        // since we can't remove from type_map while iterating
+        const size_t new_type_list_size = type_list.GetSize();
+        if (new_type_list_size > old_type_list_size)
+        {
+            for (size_t i=old_type_list_size; i<new_type_list_size; ++i)
+                type_map.Remove(type_list.GetTypeAtIndex(i));
+        }
     }
-    if(module_sp && type_map.GetSize() > 0)
-    {
-        TypeMoveMatchingModule callbackModule (module_sp, type_map, type_list);
-        type_map.ForEach(callbackModule);
+    //----------------------------------------------------------------------
+    // Find all types that match the current module, if we have one, and put
+    // them next in the list.
+    //----------------------------------------------------------------------
+    if (module_sp && !type_map.Empty())
+    {
+        const size_t old_type_list_size = type_list.GetSize();
+        type_map.ForEach([this, &type_list](const lldb::TypeSP& type_sp) -> bool {
+            SymbolContextScope *scs = type_sp->GetSymbolContextScope();
+            if (scs &&  module_sp == scs->CalculateSymbolContextModule())
+                type_list.Insert(type_sp);
+            return true; // Keep iterating
+        });
+        // Remove any entries that are now in "type_list" from "type_map"
+        // since we can't remove from type_map while iterating
+        const size_t new_type_list_size = type_list.GetSize();
+        if (new_type_list_size > old_type_list_size)
+        {
+            for (size_t i=old_type_list_size; i<new_type_list_size; ++i)
+                type_map.Remove(type_list.GetTypeAtIndex(i));
+        }
     }
-    if(type_map.GetSize() > 0)
-    {
-        TypeMaptoList callbackM2L (type_map, type_list);
-        type_map.ForEach(callbackM2L);
-
+    //----------------------------------------------------------------------
+    // Any types that are left get copied into the list an any order.
+    //----------------------------------------------------------------------
+    if (!type_map.Empty())
+    {
+        type_map.ForEach([&type_list](const lldb::TypeSP& type_sp) -> bool {
+            type_list.Insert(type_sp);
+            return true; // Keep iterating
+        });
     }
-	return ;
 }
 
 ConstString

Modified: lldb/trunk/source/Symbol/TypeMap.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/TypeMap.cpp?rev=253618&r1=253617&r2=253618&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/TypeMap.cpp (original)
+++ lldb/trunk/source/Symbol/TypeMap.cpp Thu Nov 19 17:10:45 2015
@@ -72,8 +72,8 @@ TypeMap::InsertUnique (const TypeSP& typ
             if (pos->second.get() == type_sp.get())
                 return false;
         }
+        Insert (type_sp);
     }
-    Insert (type_sp);
     return true;
 }
 
@@ -161,21 +161,24 @@ TypeMap::ForEach (std::function <bool(ll
     }
 }
 
-
 bool
-TypeMap::RemoveTypeWithUID (user_id_t uid)
+TypeMap::Remove (const lldb::TypeSP &type_sp)
 {
-    iterator pos = m_types.find(uid);
-    
-    if (pos != m_types.end())
+    if (type_sp)
     {
-        m_types.erase(pos);
-        return true;
+        lldb::user_id_t uid = type_sp->GetID();
+        for (iterator pos = m_types.find(uid), end = m_types.end(); pos != end && pos->first == uid; ++pos)
+        {
+            if (pos->second == type_sp)
+            {
+                m_types.erase(pos);
+                return true;
+            }
+        }
     }
     return false;
 }
 
-
 void
 TypeMap::Dump(Stream *s, bool show_context)
 {




More information about the lldb-commits mailing list