[Lldb-commits] [lldb] r153713 - in /lldb/trunk: include/lldb/Symbol/ClangASTContext.h lldb.xcodeproj/xcshareddata/xcschemes/lldb-tool.xcscheme source/Expression/ClangExpressionDeclMap.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp source/Symbol/ClangASTContext.cpp source/Symbol/ClangASTImporter.cpp source/Symbol/ClangASTType.cpp

Greg Clayton gclayton at apple.com
Thu Mar 29 17:51:14 PDT 2012


Author: gclayton
Date: Thu Mar 29 19:51:13 2012
New Revision: 153713

URL: http://llvm.org/viewvc/llvm-project?rev=153713&view=rev
Log:
<rdar://problem/11082392>

Fixed an issue that could cause circular type parsing that will assert and kill LLDB.

Prior to this fix the DWARF parser would always create class types and not start their definitions (for both C++ and ObjC classes) until we were asked to complete the class later. When we had cases like:

class A
{
    class B
    {
    };
};

We would alway try to complete A before specifying "A" as the decl context for B. Turns out we can just start the definition and still not complete the class since we can check the TagDecl::isCompleteDefinition() function. This only works for C++ types. This means we will not be pulling in the full definition of parent classes all the time and should help with our memory consumption and also reduce the amount of debug info we have to parse.

I also reduced redundant code that was checking in a lldb::clang_type_t was a possible C++ dynamic type since it was still completing the type, just to see if it was dynamic. This was fixed in another function that was checking for a type being dynamic as an ObjC or a C++ type, but there was dedicated fucntion for C++ that we missed.



Modified:
    lldb/trunk/include/lldb/Symbol/ClangASTContext.h
    lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/lldb-tool.xcscheme
    lldb/trunk/source/Expression/ClangExpressionDeclMap.cpp
    lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/trunk/source/Symbol/ClangASTContext.cpp
    lldb/trunk/source/Symbol/ClangASTImporter.cpp
    lldb/trunk/source/Symbol/ClangASTType.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=153713&r1=153712&r2=153713&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Thu Mar 29 19:51:13 2012
@@ -829,7 +829,9 @@
     static bool
     IsPossibleDynamicType (clang::ASTContext *ast, 
                            lldb::clang_type_t clang_type, 
-                           lldb::clang_type_t *dynamic_pointee_type = NULL);
+                           lldb::clang_type_t *dynamic_pointee_type = NULL,
+                           bool cplusplus = true,
+                           bool objc = true);
 
     static bool
     IsCStringType (lldb::clang_type_t clang_type, uint32_t &length);

Modified: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/lldb-tool.xcscheme
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/lldb-tool.xcscheme?rev=153713&r1=153712&r2=153713&view=diff
==============================================================================
--- lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/lldb-tool.xcscheme (original)
+++ lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/lldb-tool.xcscheme Thu Mar 29 19:51:13 2012
@@ -84,15 +84,17 @@
    <LaunchAction
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      debugProcessAsUID = "4294967295"
-      launchStyle = "0"
+      launchStyle = "1"
       useCustomWorkingDirectory = "NO"
       customWorkingDirectory = "/Volumes/work/gclayton/Documents/devb/attach"
       buildConfiguration = "Debug"
       ignoresPersistentStateOnLaunch = "YES"
       debugDocumentVersioning = "YES"
       allowLocationSimulation = "YES">
-      <BuildableProductRunnable>
+      <PathRunnable
+         FilePath = "/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python">
+      </PathRunnable>
+      <MacroExpansion>
          <BuildableReference
             BuildableIdentifier = "primary"
             BlueprintIdentifier = "26F5C26910F3D9A4009D5894"
@@ -100,7 +102,7 @@
             BlueprintName = "lldb-tool"
             ReferencedContainer = "container:lldb.xcodeproj">
          </BuildableReference>
-      </BuildableProductRunnable>
+      </MacroExpansion>
       <EnvironmentVariables>
          <EnvironmentVariable
             key = "LLDB_LAUNCH_FLAG_DISABLE_ASLR"

Modified: lldb/trunk/source/Expression/ClangExpressionDeclMap.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/ClangExpressionDeclMap.cpp?rev=153713&r1=153712&r2=153713&view=diff
==============================================================================
--- lldb/trunk/source/Expression/ClangExpressionDeclMap.cpp (original)
+++ lldb/trunk/source/Expression/ClangExpressionDeclMap.cpp Thu Mar 29 19:51:13 2012
@@ -3267,7 +3267,7 @@
         return;
     }
      
-    if (add_method && ClangASTContext::IsAggregateType(copied_type))
+    if (add_method && ClangASTContext::IsAggregateType(copied_type) && ClangASTContext::GetCompleteType (parser_ast_context, copied_type))
     {
         void *args[1];
         

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=153713&r1=153712&r2=153713&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Thu Mar 29 19:51:13 2012
@@ -79,23 +79,23 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static inline bool
-child_requires_parent_class_union_or_struct_to_be_completed (dw_tag_t tag)
-{
-    switch (tag)
-    {
-    default:
-        break;
-    case DW_TAG_subprogram:
-    case DW_TAG_inlined_subroutine:
-    case DW_TAG_class_type:
-    case DW_TAG_structure_type:
-    case DW_TAG_union_type:
-        return true;
-    }
-    return false;
-}
-
+//static inline bool
+//child_requires_parent_class_union_or_struct_to_be_completed (dw_tag_t tag)
+//{
+//    switch (tag)
+//    {
+//    default:
+//        break;
+//    case DW_TAG_subprogram:
+//    case DW_TAG_inlined_subroutine:
+//    case DW_TAG_class_type:
+//    case DW_TAG_structure_type:
+//    case DW_TAG_union_type:
+//        return true;
+//    }
+//    return false;
+//}
+//
 static AccessType
 DW_ACCESS_to_AccessType (uint32_t dwarf_accessibility)
 {
@@ -1874,23 +1874,23 @@
                                                               DW_TAG_value_to_name(die->Tag()), 
                                                               die->GetName(this, cu), 
                                                               decl_ctx_die->GetOffset());
-
-                Type *parent_type = ResolveTypeUID (cu, decl_ctx_die, assert_not_being_parsed);
-                if (child_requires_parent_class_union_or_struct_to_be_completed(die->Tag()))
-                {
-                    if (log)
-                        GetObjectFile()->GetModule()->LogMessage (log.get(), 
-                                                                  "SymbolFileDWARF::ResolveTypeUID (die = 0x%8.8x) %s '%s' resolve parent full type for 0x%8.8x since die is a function", 
-                                                                  die->GetOffset(), 
-                                                                  DW_TAG_value_to_name(die->Tag()), 
-                                                                  die->GetName(this, cu), 
-                                                                  decl_ctx_die->GetOffset());
-                    // Ask the type to complete itself if it already hasn't since if we
-                    // want a function (method or static) from a class, the class must 
-                    // create itself and add it's own methods and class functions.
-                    if (parent_type)
-                        parent_type->GetClangFullType();
-                }
+//
+//                Type *parent_type = ResolveTypeUID (cu, decl_ctx_die, assert_not_being_parsed);
+//                if (child_requires_parent_class_union_or_struct_to_be_completed(die->Tag()))
+//                {
+//                    if (log)
+//                        GetObjectFile()->GetModule()->LogMessage (log.get(), 
+//                                                                  "SymbolFileDWARF::ResolveTypeUID (die = 0x%8.8x) %s '%s' resolve parent full type for 0x%8.8x since die is a function", 
+//                                                                  die->GetOffset(), 
+//                                                                  DW_TAG_value_to_name(die->Tag()), 
+//                                                                  die->GetName(this, cu), 
+//                                                                  decl_ctx_die->GetOffset());
+//                    // Ask the type to complete itself if it already hasn't since if we
+//                    // want a function (method or static) from a class, the class must 
+//                    // create itself and add it's own methods and class functions.
+//                    if (parent_type)
+//                        parent_type->GetClangFullType();
+//                }
             }
             break;
 
@@ -1963,7 +1963,6 @@
         {
             LayoutInfo layout_info;
             
-            ast.StartTagDeclarationDefinition (clang_type);
             {
                 if (die->HasChildren())
                 {
@@ -1971,7 +1970,12 @@
                     LanguageType class_language = eLanguageTypeUnknown;
                     bool is_objc_class = ClangASTContext::IsObjCClassType (clang_type);
                     if (is_objc_class)
+                    {
                         class_language = eLanguageTypeObjC;
+                        // For objective C we don't start the definition when
+                        // the class is created.
+                        ast.StartTagDeclarationDefinition (clang_type);
+                    }
                     
                     int tag_decl_kind = -1;
                     AccessType default_accessibility = eAccessNone;
@@ -5076,7 +5080,11 @@
                                                        unique_ast_entry);
                     
                     if (!is_forward_declaration)
-                    {                    
+                    {
+                        // Always start the definition for a class type so that
+                        // if the class has child classes or types that require
+                        // the class to be created for use as their decl contexts
+                        // the class will be ready to accept these child definitions.
                         if (die->HasChildren() == false)
                         {
                             // No children for this struct/union/class, lets finish it
@@ -5085,6 +5093,17 @@
                         }
                         else if (clang_type_was_created)
                         {
+                            // Start the definition if the class is not objective C since
+                            // the underlying decls respond to isCompleteDefinition(). Objective
+                            // C decls dont' respond to isCompleteDefinition() so we can't
+                            // start the declaration definition right away. For C++ classs/union/structs
+                            // we want to start the definition in case the class is needed as the
+                            // declaration context for a contained class or type without the need
+                            // to complete that type..
+                            
+                            if (class_language != eLanguageTypeObjC)
+                                ast.StartTagDeclarationDefinition (clang_type);
+
                             // Leave this as a forward declaration until we need
                             // to know the details of the type. lldb_private::Type
                             // will automatically call the SymbolFile virtual function

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=153713&r1=153712&r2=153713&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Thu Mar 29 19:51:13 2012
@@ -101,7 +101,7 @@
                 clang::TagDecl *tag_decl = tag_type->getDecl();
                 if (tag_decl)
                 {
-                    if (tag_decl->getDefinition())
+                    if (tag_decl->isCompleteDefinition())
                         return true;
                     
                     if (!allow_completion)
@@ -5413,7 +5413,11 @@
 }
 
 bool
-ClangASTContext::IsPossibleDynamicType (clang::ASTContext *ast, clang_type_t clang_type, clang_type_t *dynamic_pointee_type)
+ClangASTContext::IsPossibleDynamicType (clang::ASTContext *ast,
+                                        clang_type_t clang_type,
+                                        clang_type_t *dynamic_pointee_type,
+                                        bool check_cplusplus,
+                                        bool check_objc)
 {
     QualType pointee_qual_type;
     if (clang_type)
@@ -5424,7 +5428,7 @@
         switch (type_class)
         {
             case clang::Type::Builtin:
-                if (cast<clang::BuiltinType>(qual_type)->getKind() == clang::BuiltinType::ObjCId)
+                if (check_objc && cast<clang::BuiltinType>(qual_type)->getKind() == clang::BuiltinType::ObjCId)
                 {
                     if (dynamic_pointee_type)
                         *dynamic_pointee_type = clang_type;
@@ -5433,9 +5437,13 @@
                 break;
 
             case clang::Type::ObjCObjectPointer:
-                if (dynamic_pointee_type)
-                    *dynamic_pointee_type = cast<ObjCObjectPointerType>(qual_type)->getPointeeType().getAsOpaquePtr();
-                return true;
+                if (check_objc)
+                {
+                    if (dynamic_pointee_type)
+                        *dynamic_pointee_type = cast<ObjCObjectPointerType>(qual_type)->getPointeeType().getAsOpaquePtr();
+                    return true;
+                }
+                break;
 
             case clang::Type::Pointer:
                 pointee_qual_type = cast<PointerType>(qual_type)->getPointeeType();
@@ -5512,6 +5520,7 @@
                     break;
 
                 case clang::Type::Record:
+                    if (check_cplusplus)
                     {
                         CXXRecordDecl *cxx_record_decl = pointee_qual_type->getAsCXXRecordDecl();
                         if (cxx_record_decl)
@@ -5522,7 +5531,7 @@
                             // time, memory and slow down debugging. If we have a complete
                             // type, then answer the question definitively, else we
                             // just say that a C++ class can possibly be dynamic...
-                            if (cxx_record_decl->getDefinition())
+                            if (cxx_record_decl->isCompleteDefinition())
                             {
                                 success = cxx_record_decl->isDynamicClass();
                             }
@@ -5545,9 +5554,13 @@
                     
                 case clang::Type::ObjCObject:
                 case clang::Type::ObjCInterface:
-                    if (dynamic_pointee_type)
-                        *dynamic_pointee_type = pointee_qual_type.getAsOpaquePtr();
-                    return true;
+                    if (check_objc)
+                    {
+                        if (dynamic_pointee_type)
+                            *dynamic_pointee_type = pointee_qual_type.getAsOpaquePtr();
+                        return true;
+                    }
+                    break;
 
                 default:
                     break;
@@ -5563,121 +5576,11 @@
 bool
 ClangASTContext::IsPossibleCPlusPlusDynamicType (clang::ASTContext *ast, clang_type_t clang_type, clang_type_t *dynamic_pointee_type)
 {
-    QualType pointee_qual_type;
-    if (clang_type)
-    {
-        QualType qual_type (QualType::getFromOpaquePtr(clang_type));
-        const clang::Type::TypeClass type_class = qual_type->getTypeClass();
-        bool success = false;
-        switch (type_class)
-        {
-            case clang::Type::Pointer:
-                pointee_qual_type = cast<PointerType>(qual_type)->getPointeeType();
-                success = true;
-                break;
-
-            case clang::Type::LValueReference:
-            case clang::Type::RValueReference:
-                pointee_qual_type = cast<ReferenceType>(qual_type)->getPointeeType();
-                success = true;
-                break;
-
-            case clang::Type::Typedef:
-                return ClangASTContext::IsPossibleCPlusPlusDynamicType (ast, cast<TypedefType>(qual_type)->getDecl()->getUnderlyingType().getAsOpaquePtr(), dynamic_pointee_type);
-
-            case clang::Type::Elaborated:
-                return ClangASTContext::IsPossibleCPlusPlusDynamicType (ast, cast<ElaboratedType>(qual_type)->getNamedType().getAsOpaquePtr());
-
-            default:
-                break;
-        }
-        
-        if (success)
-        {
-            // Check to make sure what we are pointing too is a possible dynamic C++ type
-            // We currently accept any "void *" (in case we have a class that has been
-            // watered down to an opaque pointer) and virtual C++ classes.
-            const clang::Type::TypeClass pointee_type_class = pointee_qual_type->getTypeClass();
-            switch (pointee_type_class)
-            {
-            case clang::Type::Builtin:
-                switch (cast<clang::BuiltinType>(pointee_qual_type)->getKind())
-                {
-                    case clang::BuiltinType::UnknownAny:
-                    case clang::BuiltinType::Void:
-                        if (dynamic_pointee_type)
-                            *dynamic_pointee_type = pointee_qual_type.getAsOpaquePtr();
-                        return true;
-
-                    case clang::BuiltinType::NullPtr:  
-                    case clang::BuiltinType::Bool:
-                    case clang::BuiltinType::Char_U:
-                    case clang::BuiltinType::UChar:
-                    case clang::BuiltinType::WChar_U:
-                    case clang::BuiltinType::Char16:
-                    case clang::BuiltinType::Char32:
-                    case clang::BuiltinType::UShort:
-                    case clang::BuiltinType::UInt:
-                    case clang::BuiltinType::ULong:
-                    case clang::BuiltinType::ULongLong:
-                    case clang::BuiltinType::UInt128:
-                    case clang::BuiltinType::Char_S:
-                    case clang::BuiltinType::SChar:
-                    case clang::BuiltinType::WChar_S:
-                    case clang::BuiltinType::Short:
-                    case clang::BuiltinType::Int:
-                    case clang::BuiltinType::Long:
-                    case clang::BuiltinType::LongLong:
-                    case clang::BuiltinType::Int128:
-                    case clang::BuiltinType::Float:
-                    case clang::BuiltinType::Double:
-                    case clang::BuiltinType::LongDouble:
-                    case clang::BuiltinType::Dependent:
-                    case clang::BuiltinType::Overload:
-                    case clang::BuiltinType::ObjCId:
-                    case clang::BuiltinType::ObjCClass:
-                    case clang::BuiltinType::ObjCSel:
-                    case clang::BuiltinType::BoundMember:
-                    case clang::BuiltinType::Half:          
-                    case clang::BuiltinType::ARCUnbridgedCast:          
-                    case clang::BuiltinType::PseudoObject:
-                        break;
-                }
-                break;
-            case clang::Type::Record:
-                {
-                    CXXRecordDecl *cxx_record_decl = pointee_qual_type->getAsCXXRecordDecl();
-                    if (cxx_record_decl)
-                    {
-                        if (GetCompleteQualType (ast, pointee_qual_type))
-                        {
-                            success = cxx_record_decl->isDynamicClass();
-                        }
-                        else
-                        {
-                            // We failed to get the complete type, so we have to 
-                            // treat this as a void * which we might possibly be
-                            // able to complete
-                            success = true;
-                        }
-                        if (success)
-                        {
-                            if (dynamic_pointee_type)
-                                *dynamic_pointee_type = pointee_qual_type.getAsOpaquePtr();
-                            return true;
-                        }
-                    }
-                }
-                break;
-
-            default:
-                break;
-            }
-        }
-    }
-    if (dynamic_pointee_type)
-        *dynamic_pointee_type = NULL;
-    return false;
+    return IsPossibleDynamicType (ast,
+                                  clang_type,
+                                  dynamic_pointee_type,
+                                  true,     // Check for dynamic C++ types
+                                  false);   // Check for dynamic ObjC types
 }
 
 bool
@@ -6371,7 +6274,7 @@
         
     if (clang::TagDecl *tag_decl = llvm::dyn_cast<clang::TagDecl>(decl))
     {
-        if (tag_decl->getDefinition())
+        if (tag_decl->isCompleteDefinition())
             return true;
         
         if (!tag_decl->hasExternalLexicalStorage())

Modified: lldb/trunk/source/Symbol/ClangASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTImporter.cpp?rev=153713&r1=153712&r2=153713&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp Thu Mar 29 19:51:13 2012
@@ -169,7 +169,8 @@
             tag_decl->setCompleteDefinition(true);
         }
     }
-    else {
+    else
+    {
         assert (0 && "CompleteDecl called on a Decl that can't be completed");
     }
 }

Modified: lldb/trunk/source/Symbol/ClangASTType.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTType.cpp?rev=153713&r1=153712&r2=153713&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTType.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTType.cpp Thu Mar 29 19:51:13 2012
@@ -1202,7 +1202,7 @@
     {
         clang::TagDecl *tag_decl = tag_type->getDecl();
         if (tag_decl)
-            return tag_decl->getDefinition() != NULL;
+            return tag_decl->isCompleteDefinition();
         return false;
     }
     else





More information about the lldb-commits mailing list