[Lldb-commits] [lldb] r241695 - Fixed a serious bug in DeportType where the types could retain DeclContexts that

[Sean Callanan]

Author: spyffe
Date: Wed Jul  8 13:03:41 2015
New Revision: 241695

URL: http://llvm.org/viewvc/llvm-project?rev=241695&view=rev
Log:
Fixed a serious bug in DeportType where the types could retain DeclContexts that
pointed into the artificial function constructed for the expression.  I now make
anything that pointed to the function as its DeclContext be global while the
copy occurs; afterward I restored the old DeclContext.

Added a testcase that make sure that this works properly and doesn't crash
anything.

<rdar://problem/21049838>

Modified:
    lldb/trunk/source/Symbol/ClangASTImporter.cpp
    lldb/trunk/test/expression_command/persistent_types/TestPersistentTypes.py

Modified: lldb/trunk/source/Symbol/ClangASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTImporter.cpp?rev=241695&r1=241694&r2=241695&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp Wed Jul  8 13:03:41 2015
@@ -17,6 +17,7 @@
 #include "lldb/Symbol/ClangASTImporter.h"
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
 #include "lldb/Symbol/ClangNamespaceDecl.h"
+#include "lldb/Utility/LLDBAssert.h"
 
 using namespace lldb_private;
 using namespace clang;
@@ -109,6 +110,134 @@ ClangASTImporter::CopyDecl (clang::ASTCo
     return nullptr;
 }
 
+class DeclContextOverride
+{
+private:
+    struct Backup
+    {
+        clang::DeclContext *decl_context;
+        clang::DeclContext *lexical_decl_context;
+    };
+    
+    std::map<clang::Decl *, Backup> m_backups;
+    
+    void OverrideOne(clang::Decl *decl)
+    {
+        if (m_backups.find(decl) != m_backups.end())
+        {
+            return;
+        }
+            
+        m_backups[decl] = { decl->getDeclContext(), decl->getLexicalDeclContext() };
+        
+        decl->setDeclContext(decl->getASTContext().getTranslationUnitDecl());
+        decl->setLexicalDeclContext(decl->getASTContext().getTranslationUnitDecl());
+    }
+    
+    bool ChainPassesThrough(clang::Decl *decl,
+                            clang::DeclContext *base,
+                            clang::DeclContext *(clang::Decl::*contextFromDecl)(),
+                            clang::DeclContext *(clang::DeclContext::*contextFromContext)())
+    {
+        for (DeclContext *decl_ctx = (decl->*contextFromDecl)();
+             decl_ctx;
+             decl_ctx = (decl_ctx->*contextFromContext)())
+        {
+            if (decl_ctx == base)
+            {
+                return true;
+            }
+        }
+        
+        return false;
+    }
+    
+    clang::Decl *GetEscapedChild(clang::Decl *decl, clang::DeclContext *base = nullptr)
+    {
+        if (base)
+        {
+            // decl's DeclContext chains must pass through base.
+            
+            if (!ChainPassesThrough(decl, base, &clang::Decl::getDeclContext, &clang::DeclContext::getParent) ||
+                !ChainPassesThrough(decl, base, &clang::Decl::getLexicalDeclContext, &clang::DeclContext::getLexicalParent))
+            {
+                return decl;
+            }
+        }
+        else
+        {
+            base = clang::dyn_cast<clang::DeclContext>(decl);
+            
+            if (!base)
+            {
+                return nullptr;
+            }
+        }
+        
+        if (clang::DeclContext *context = clang::dyn_cast<clang::DeclContext>(decl))
+        {
+            for (clang::Decl *decl : context->decls())
+            {
+                if (clang::Decl *escaped_child = GetEscapedChild(decl))
+                {
+                    return escaped_child;
+                }
+            }
+        }
+        
+        return nullptr;
+    }
+    
+    void Override(clang::Decl *decl)
+    {
+        if (clang::Decl *escaped_child = GetEscapedChild(decl))
+        {
+            Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_EXPRESSIONS));
+            
+            if (log)
+                log->Printf("    [ClangASTImporter] DeclContextOverride couldn't override (%sDecl*)%p - its child (%sDecl*)%p escapes",
+                            decl->getDeclKindName(), static_cast<void*>(decl),
+                            escaped_child->getDeclKindName(), static_cast<void*>(escaped_child));
+            lldbassert(0 && "Couldn't override!");
+        }
+        
+        OverrideOne(decl);
+    }
+    
+public:
+    DeclContextOverride()
+    {
+    }
+    
+    void OverrideAllDeclsFromContainingFunction(clang::Decl *decl)
+    {
+        for (DeclContext *decl_context = decl->getLexicalDeclContext();
+             decl_context;
+             decl_context = decl_context->getLexicalParent())
+        {
+            DeclContext *redecl_context = decl_context->getRedeclContext();
+            
+            if (llvm::isa<FunctionDecl>(redecl_context) &&
+                llvm::isa<TranslationUnitDecl>(redecl_context->getLexicalParent()))
+            {
+                for (clang::Decl *child_decl : decl_context->decls())
+                {
+                    Override(child_decl);
+                }
+            }
+        }
+    }
+    
+    ~DeclContextOverride()
+    {
+        for (const std::pair<clang::Decl *, Backup> &backup : m_backups)
+        {
+            backup.first->setDeclContext(backup.second.decl_context);
+            backup.first->setLexicalDeclContext(backup.second.lexical_decl_context);
+        }
+    }
+};
+
 lldb::clang_type_t
 ClangASTImporter::DeportType (clang::ASTContext *dst_ctx,
                               clang::ASTContext *src_ctx,
@@ -122,6 +251,13 @@ ClangASTImporter::DeportType (clang::AST
     std::set<NamedDecl *> decls_to_deport;
     std::set<NamedDecl *> decls_already_deported;
     
+    DeclContextOverride decl_context_override;
+    
+    if (const clang::TagType *tag_type = clang::QualType::getFromOpaquePtr(type)->getAs<TagType>())
+    {
+        decl_context_override.OverrideAllDeclsFromContainingFunction(tag_type->getDecl());
+    }
+    
     minion_sp->InitDeportWorkQueues(&decls_to_deport,
                                     &decls_already_deported);
     
@@ -156,6 +292,10 @@ ClangASTImporter::DeportDecl (clang::AST
 
     std::set<NamedDecl *> decls_to_deport;
     std::set<NamedDecl *> decls_already_deported;
+    
+    DeclContextOverride decl_context_override;
+    
+    decl_context_override.OverrideAllDeclsFromContainingFunction(decl);
 
     minion_sp->InitDeportWorkQueues(&decls_to_deport,
                                     &decls_already_deported);

Modified: lldb/trunk/test/expression_command/persistent_types/TestPersistentTypes.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/expression_command/persistent_types/TestPersistentTypes.py?rev=241695&r1=241694&r2=241695&view=diff
==============================================================================
--- lldb/trunk/test/expression_command/persistent_types/TestPersistentTypes.py (original)
+++ lldb/trunk/test/expression_command/persistent_types/TestPersistentTypes.py Wed Jul  8 13:03:41 2015
@@ -46,6 +46,12 @@ class PersistenttypesTestCase(TestBase):
         self.expect("expression struct { int a; int b; } x = { 2, 3 }; x",
                     substrs = ['a = 2', 'b = 3'])
 
+        self.expect("expression struct { int x; int y; int z; } object; object.y = 1; object.z = 3; object.x = 2; object",
+                    substrs = ['x = 2', 'y = 1', 'z = 3'])
+
+        self.expect("expression struct A { int x; int y; }; struct { struct A a; int z; } object; object.a.y = 1; object.z = 3; object.a.x = 2; object",
+                    substrs = ['x = 2', 'y = 1', 'z = 3'])
+
 
 if __name__ == '__main__':
     import atexit