[Lldb-commits] [lldb] r270891 - Make sure that we succeed in starting a definition before we complete it and emit an error if we fail to start the definition.

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Thu May 26 12:24:02 PDT 2016


Author: gclayton
Date: Thu May 26 14:24:02 2016
New Revision: 270891

URL: http://llvm.org/viewvc/llvm-project?rev=270891&view=rev
Log:
Make sure that we succeed in starting a definition before we complete it and emit an error if we fail to start the definition.

ClangASTContext::StartTagDeclarationDefinition(...) was starting definitions for any TagType instances that have TagDecl, but ClangASTContext::CompleteTagDeclarationDefinition(...) was getting the type to a CXXRecordDecl with:

    clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
    
The problem is that getAsCXXRecordDecl() might dig a bit deeper into a type and dig out a different decl, which means we might call ClangASTContext::StartTagDeclarationDefinition(...), but it might not do anything, and then we might call ClangASTContext::CompleteTagDeclarationDefinition(...) and it might try to complete something that didn't have its definition started and this will crash.

This change fixes that, and also makes sure that starting a definition succeeds before any calls to ClangASTContext::CompleteTagDeclarationDefinition().
                                                    
<rdar://problem/24091798>


Modified:
    lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
    lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=270891&r1=270890&r2=270891&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu May 26 14:24:02 2016
@@ -896,8 +896,16 @@ DWARFASTParserClang::ParseTypeFromDWARF
                         if (die.HasChildren() == false)
                         {
                             // No children for this struct/union/class, lets finish it
-                            ClangASTContext::StartTagDeclarationDefinition (clang_type);
-                            ClangASTContext::CompleteTagDeclarationDefinition (clang_type);
+                            if (ClangASTContext::StartTagDeclarationDefinition (clang_type))
+                            {
+                                ClangASTContext::CompleteTagDeclarationDefinition (clang_type);
+                            }
+                            else
+                            {
+                                dwarf->GetObjectFile()->GetModule()->ReportError("DWARF DIE at 0x%8.8x named \"%s\" was not able to start its definition.\nPlease file a bug and attach the file at the start of this error message",
+                                                                                 die.GetOffset(),
+                                                                                 type_name_cstr);
+                            }
 
                             if (tag == DW_TAG_structure_type) // this only applies in C
                             {
@@ -1085,15 +1093,23 @@ DWARFASTParserClang::ParseTypeFromDWARF
                                                  clang_type,
                                                  Type::eResolveStateForward));
 
-                        ClangASTContext::StartTagDeclarationDefinition (clang_type);
-                        if (die.HasChildren())
+                        if (ClangASTContext::StartTagDeclarationDefinition (clang_type))
+                        {
+                            if (die.HasChildren())
+                            {
+                                SymbolContext cu_sc(die.GetLLDBCompileUnit());
+                                bool is_signed = false;
+                                enumerator_clang_type.IsIntegerType(is_signed);
+                                ParseChildEnumerators(cu_sc, clang_type, is_signed, type_sp->GetByteSize(), die);
+                            }
+                            ClangASTContext::CompleteTagDeclarationDefinition (clang_type);
+                        }
+                        else
                         {
-                            SymbolContext cu_sc(die.GetLLDBCompileUnit());
-                            bool is_signed = false;
-                            enumerator_clang_type.IsIntegerType(is_signed);
-                            ParseChildEnumerators(cu_sc, clang_type, is_signed, type_sp->GetByteSize(), die);
+                            dwarf->GetObjectFile()->GetModule()->ReportError("DWARF DIE at 0x%8.8x named \"%s\" was not able to start its definition.\nPlease file a bug and attach the file at the start of this error message",
+                                                                             die.GetOffset(),
+                                                                             type_name_cstr);
                         }
-                        ClangASTContext::CompleteTagDeclarationDefinition (clang_type);
                     }
                 }
                     break;
@@ -1735,8 +1751,16 @@ DWARFASTParserClang::ParseTypeFromDWARF
                                 // to layout the class. Since we provide layout assistance, all
                                 // ivars in this class and other classes will be fine, this is
                                 // the best we can do short of crashing.
-                                ClangASTContext::StartTagDeclarationDefinition(array_element_type);
-                                ClangASTContext::CompleteTagDeclarationDefinition(array_element_type);
+                                if (ClangASTContext::StartTagDeclarationDefinition(array_element_type))
+                                {
+                                    ClangASTContext::CompleteTagDeclarationDefinition(array_element_type);
+                                }
+                                else
+                                {
+                                    module_sp->ReportError ("DWARF DIE at 0x%8.8x was not able to start its definition.\nPlease file a bug and attach the file at the start of this error message",
+                                                            type_die_ref.die_offset,
+                                                            type_name_cstr);
+                                }
                             }
 
                             uint64_t array_element_bit_stride = byte_stride * 8 + bit_stride;
@@ -2242,8 +2266,10 @@ DWARFASTParserClang::CompleteTypeFromDWA
                                     // below. Since we provide layout assistance, all ivars in this
                                     // class and other classes will be fine, this is the best we can do
                                     // short of crashing.
-                                    ClangASTContext::StartTagDeclarationDefinition (base_class_type);
-                                    ClangASTContext::CompleteTagDeclarationDefinition (base_class_type);
+                                    if (ClangASTContext::StartTagDeclarationDefinition (base_class_type))
+                                    {
+                                        ClangASTContext::CompleteTagDeclarationDefinition (base_class_type);
+                                    }
                                 }
                             }
                         }
@@ -2339,15 +2365,17 @@ DWARFASTParserClang::CompleteTypeFromDWA
             return (bool)clang_type;
 
         case DW_TAG_enumeration_type:
-            ClangASTContext::StartTagDeclarationDefinition (clang_type);
-            if (die.HasChildren())
+            if (ClangASTContext::StartTagDeclarationDefinition (clang_type))
             {
-                SymbolContext sc(die.GetLLDBCompileUnit());
-                bool is_signed = false;
-                clang_type.IsIntegerType(is_signed);
-                ParseChildEnumerators(sc, clang_type, is_signed, type->GetByteSize(), die);
+                if (die.HasChildren())
+                {
+                    SymbolContext sc(die.GetLLDBCompileUnit());
+                    bool is_signed = false;
+                    clang_type.IsIntegerType(is_signed);
+                    ParseChildEnumerators(sc, clang_type, is_signed, type->GetByteSize(), die);
+                }
+                ClangASTContext::CompleteTagDeclarationDefinition (clang_type);
             }
-            ClangASTContext::CompleteTagDeclarationDefinition (clang_type);
             return (bool)clang_type;
 
         default:
@@ -3089,8 +3117,18 @@ DWARFASTParserClang::ParseChildMembers(c
                                     // to layout the class. Since we provide layout assistance, all
                                     // ivars in this class and other classes will be fine, this is
                                     // the best we can do short of crashing.
-                                    ClangASTContext::StartTagDeclarationDefinition(member_clang_type);
-                                    ClangASTContext::CompleteTagDeclarationDefinition(member_clang_type);
+                                    if (ClangASTContext::StartTagDeclarationDefinition(member_clang_type))
+                                    {
+                                        ClangASTContext::CompleteTagDeclarationDefinition(member_clang_type);
+                                    }
+                                    else
+                                    {
+                                        module_sp->ReportError ("DWARF DIE at 0x%8.8x (class %s) has a member variable 0x%8.8x (%s) whose type claims to be a C++ class but we were not able to start its definition.\nPlease file a bug and attach the file at the start of this error message",
+                                                                parent_die.GetOffset(),
+                                                                parent_die.GetName(),
+                                                                die.GetOffset(),
+                                                                name);
+                                    }
                                 }
 
                                 field_decl = ClangASTContext::AddFieldToRecordType (class_clang_type,

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=270891&r1=270890&r2=270891&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Thu May 26 14:24:02 2016
@@ -8594,18 +8594,28 @@ ClangASTContext::CompleteTagDeclarationD
     clang::QualType qual_type(ClangUtil::GetQualType(type));
     if (!qual_type.isNull())
     {
-        clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-        
-        if (cxx_record_decl)
+        // Make sure we use the same methodology as ClangASTContext::StartTagDeclarationDefinition()
+        // as to how we start/end the definition. Previously we were calling 
+        const clang::TagType *tag_type = qual_type->getAs<clang::TagType>();
+        if (tag_type)
         {
-            if (!cxx_record_decl->isCompleteDefinition())
-                cxx_record_decl->completeDefinition();
-            cxx_record_decl->setHasLoadedFieldsFromExternalStorage(true);
-            cxx_record_decl->setHasExternalLexicalStorage (false);
-            cxx_record_decl->setHasExternalVisibleStorage (false);
-            return true;
+            clang::TagDecl *tag_decl = tag_type->getDecl();
+            if (tag_decl)
+            {
+                clang::CXXRecordDecl *cxx_record_decl = llvm::dyn_cast_or_null<clang::CXXRecordDecl>(tag_decl);
+                
+                if (cxx_record_decl)
+                {
+                    if (!cxx_record_decl->isCompleteDefinition())
+                        cxx_record_decl->completeDefinition();
+                    cxx_record_decl->setHasLoadedFieldsFromExternalStorage(true);
+                    cxx_record_decl->setHasExternalLexicalStorage (false);
+                    cxx_record_decl->setHasExternalVisibleStorage (false);
+                    return true;
+                }
+            }
         }
-        
+
         const clang::EnumType *enutype = qual_type->getAs<clang::EnumType>();
         
         if (enutype)




More information about the lldb-commits mailing list