Fix resolution of certain recursive types.

[sas]

Summary

This is a cross-port of r234441.

If a struct type S has a member T that has a member that is a function that
returns a typedef of S* the respective field would be duplicated, which caused
an assert down the line in RecordLayoutBuilder. This patch adds a check that
removes the possibility of trying to resolve the same type twice within the
same callstack.

This commit also adds unit tests for these failures.

Fixes https://llvm.org/bugs/show_bug.cgi?id=20486.

Patch by Cristian Hancila.

[clayborg]

I'm good as long as Sean is good with this.

[spyffe]

I'm a fan.

[sas]

Amazing!

lldb/branches/release_36/include/lldb/Expression/ClangASTSource.h

public:
     /// @param[in] declMap
     ///     A reference to the LLDB object that handles entity lookup.
     //------------------------------------------------------------------
   ClangASTSource (const lldb::TargetSP &target) :
         m_import_in_progress (false),
         m_lookups_enabled (false),
         m_target (target),
         m_ast_context (NULL),
+        m_active_lexical_decls (),
         m_active_lookups ()
     {
         m_ast_importer = m_target->GetClangASTImporter();
     }
     
     //------------------------------------------------------------------
     /// Destructor
     //------------------------------------------------------------------
protected:
     friend struct NameSearchContext;
     
     bool                    m_import_in_progress;
     bool                    m_lookups_enabled;
 
     const lldb::TargetSP                m_target;           ///< The target to use in finding variables and types.
   clang::ASTContext                  *m_ast_context;      ///< The AST context requests are coming in for.
     ClangASTImporter                   *m_ast_importer;     ///< The target's AST importer.
+    std::set<const clang::Decl *>       m_active_lexical_decls;
     std::set<const char *>              m_active_lookups;
 };
 
 //----------------------------------------------------------------------
 /// @class NameSearchContext ClangASTSource.h "lldb/Expression/ClangASTSource.h"
 /// @brief Container for all objects relevant to a single name lookup
 ///     
 /// LLDB needs to create Decls for entities it finds.  This class communicates

lldb/branches/release_36/source/Expression/ClangASTSource.cpp

 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/ObjCLanguageRuntime.h"
 #include "lldb/Target/Target.h"
 
 using namespace clang;
 using namespace lldb_private;
 
+//------------------------------------------------------------------
+// Scoped class that will remove an active lexical decl from the set
+// when it goes out of scope.
+//------------------------------------------------------------------
+namespace {
+    class ScopedLexicalDeclEraser
+    {
+    public:
+        ScopedLexicalDeclEraser(std::set<const clang::Decl *> &decls,
+                                const clang::Decl *decl)
+            : m_active_lexical_decls(decls), m_decl(decl)
+        {
+        }
+
+        ~ScopedLexicalDeclEraser()
+        {
+            m_active_lexical_decls.erase(m_decl);
+        }
+
+    private:
+        std::set<const clang::Decl *> &m_active_lexical_decls;
+        const clang::Decl *m_decl;
+    };
+}
+
 ClangASTSource::~ClangASTSource()
 {
     m_ast_importer->ForgetDestination(m_ast_context);
 
     // We are in the process of destruction, don't create clang ast context on demand
     // by passing false to Target::GetScratchClangASTContext(create_on_demand).
     ClangASTContext *scratch_clang_ast_context = m_target->GetScratchClangASTContext(false);
 
if (log)
                     static_cast<void*>(tag_decl),
                     tag_decl->getName().str().c_str());
 
         log->Printf("      CTD[%u] Before:", current_id);
         ASTDumper dumper((Decl*)tag_decl);
         dumper.ToLog(log, "      [CTD] ");
     }
 
+    auto iter = m_active_lexical_decls.find(tag_decl);
+    if (iter != m_active_lexical_decls.end())
+        return;
+    m_active_lexical_decls.insert(tag_decl);
+    ScopedLexicalDeclEraser eraser(m_active_lexical_decls, tag_decl);
+
     if (!m_ast_importer->CompleteTagDecl (tag_decl))
     {
         // We couldn't complete the type.  Maybe there's a definition
         // somewhere else that can be completed.
 
         if (log)
             log->Printf("      CTD[%u] Type could not be completed in the module in which it was first found.", current_id);
 
ClangASTSource::FindExternalLexicalDecls (const DeclContext *decl_context,
 
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_EXPRESSIONS));
 
     const Decl *context_decl = dyn_cast<Decl>(decl_context);
 
     if (!context_decl)
         return ELR_Failure;
 
+    auto iter = m_active_lexical_decls.find(context_decl);
+    if (iter != m_active_lexical_decls.end())
+        return ELR_Failure;
+    m_active_lexical_decls.insert(context_decl);
+    ScopedLexicalDeclEraser eraser(m_active_lexical_decls, context_decl);
+
     static unsigned int invocation_id = 0;
     unsigned int current_id = invocation_id++;
 
     if (log)
     {
         if (const NamedDecl *context_named_decl = dyn_cast<NamedDecl>(context_decl))
             log->Printf("FindExternalLexicalDecls[%u] on (ASTContext*)%p in '%s' (%sDecl*)%p with %s predicate",
                         current_id, static_cast<void*>(m_ast_context),

lldb/branches/release_36/source/Symbol/ClangASTImporter.cpp

 bool
 ClangASTImporter::RequireCompleteType (clang::QualType type)
 {
     if (type.isNull())
         return false;
     
     if (const TagType *tag_type = type->getAs<TagType>())
     {
-        return CompleteTagDecl(tag_type->getDecl());
+        TagDecl *tag_decl = tag_type->getDecl();
+
+        if (tag_decl->getDefinition() || tag_decl->isBeingDefined())
+            return true;
+
+        return CompleteTagDecl(tag_decl);
     }
     if (const ObjCObjectType *objc_object_type = type->getAs<ObjCObjectType>())
     {
         if (ObjCInterfaceDecl *objc_interface_decl = objc_object_type->getInterface())
             return CompleteObjCInterfaceDecl(objc_interface_decl);
         else
             return false;
     }

lldb/branches/release_36/test/types/TestRecursiveTypes.py

+"""
+Test that recursive types are handled correctly.
+"""
+
+import lldb
+import lldbutil
+import sys
+import unittest2
+from lldbtest import *
+
+class RecursiveTypesTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # disable "There is a running process, kill it and restart?" prompt
+        self.runCmd("settings set auto-confirm true")
+        self.addTearDownHook(lambda: self.runCmd("settings clear auto-confirm"))
+        # Find the line number to break for main.c.
+        self.line = line_number('recursive_type_main.cpp',
+                                '// Test at this line.')
+
+        self.d1 = {'CXX_SOURCES': 'recursive_type_main.cpp recursive_type_1.cpp'}
+        self.d2 = {'CXX_SOURCES': 'recursive_type_main.cpp recursive_type_2.cpp'}
+
+    @unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin")
+    @dsym_test
+    def test_recursive_dsym_type_1(self):
+        """Test that recursive structs are displayed correctly."""
+        self.buildDsym(dictionary=self.d1)
+        self.print_struct()
+
+    @dwarf_test
+    def test_recursive_dwarf_type_1(self):
+        """Test that recursive structs are displayed correctly."""
+        self.buildDwarf(dictionary=self.d1)
+        self.print_struct()
+
+    @unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin")
+    @dsym_test
+    def test_recursive_dsym_type_2(self):
+        """Test that recursive structs are displayed correctly."""
+        self.buildDsym(dictionary=self.d2)
+        self.print_struct()
+
+    @dwarf_test
+    def test_recursive_dwarf_type_2(self):
+        """Test that recursive structs are displayed correctly."""
+        self.buildDwarf(dictionary=self.d2)
+        self.print_struct()
+
+    def print_struct(self):
+        self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
+
+        lldbutil.run_break_set_by_file_and_line (self, "recursive_type_main.cpp", self.line, num_expected_locations=-1, loc_exact=True)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        self.expect("print tpi", RUN_SUCCEEDED)
+        self.expect("print *tpi", RUN_SUCCEEDED)
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

lldb/branches/release_36/test/types/recursive_type_1.cpp

+typedef struct t *tp;
+typedef tp (*get_tp)();
+
+struct s {
+    get_tp get_tp_p;
+};
+
+struct t {
+    struct s *s;
+};
+
+struct t t;

lldb/branches/release_36/test/types/recursive_type_2.cpp

+typedef struct t *tp;
+typedef tp (*get_tp)();
+
+struct t {
+    struct {
+      get_tp get_tp_p;
+    };
+};
+
+struct t t;

lldb/branches/release_36/test/types/recursive_type_main.cpp

+typedef struct t *tp;
+extern struct t t;
+
+int main() {
+    tp tpi = &t;
+    // Test at this line.
+    return 0;
+}