[ASTImporter] Fix infinite recursion on function import with struct definition in parameters

Summary:
Importing a function having a struct definition in the parameter list
causes a crash in the importer via infinite recursion. This patch avoids
the crash and reports such functions as not supported. Unit tests make
sure that normal struct definitions inside function bodies work normally
on the other hand and LLDB-like type imports also do.

Reviewers: a.sidorin, martong

Differential Revision: https://reviews.llvm.org/D47946

Patch by Zoltan Gera!

llvm-svn: 336898

2 files changed, 98 insertions, 20 deletions

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 07b67233009..56ca9bbae46 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -1138,8 +1138,26 @@ bool ASTNodeImporter::ImportDeclParts(NamedDecl *D, DeclContext *&DC,
                                       DeclarationName &Name, 
                                       NamedDecl *&ToD,
                                       SourceLocation &Loc) {
+  // Check if RecordDecl is in FunctionDecl parameters to avoid infinite loop.
+  // example: int struct_in_proto(struct data_t{int a;int b;} *d);
+  DeclContext *OrigDC = D->getDeclContext();
+  FunctionDecl *FunDecl;
+  if (isa<RecordDecl>(D) && (FunDecl = dyn_cast<FunctionDecl>(OrigDC)) &&
+      FunDecl->hasBody()) {
+    SourceRange RecR = D->getSourceRange();
+    SourceRange BodyR = FunDecl->getBody()->getSourceRange();
+    // If RecordDecl is not in Body (it is a param), we bail out.
+    if (RecR.isValid() && BodyR.isValid() &&
+        (RecR.getBegin() < BodyR.getBegin() ||
+         BodyR.getEnd() < RecR.getEnd())) {
+      Importer.FromDiag(D->getLocation(), diag::err_unsupported_ast_node)
+          << D->getDeclKindName();
+      return true;
+    }
+  }
+
   // Import the context of this declaration.
-  DC = Importer.ImportContext(D->getDeclContext());
+  DC = Importer.ImportContext(OrigDC);
   if (!DC)
     return true;
   
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 1b3678c1233..cd8254c0a04 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -301,14 +301,25 @@ class ASTImporterTestBase : public ParameterizedTestsFixture {
       Unit->enableSourceFileDiagnostics();
     }
 
-    Decl *import(ASTUnit *ToAST, Decl *FromDecl) {
+    void lazyInitImporter(ASTUnit *ToAST) {
       assert(ToAST);
       if (!Importer) {
         Importer.reset(new ASTImporter(
             ToAST->getASTContext(), ToAST->getFileManager(),
             Unit->getASTContext(), Unit->getFileManager(), false));
       }
+      assert(&ToAST->getASTContext() == &Importer->getToContext());
+      createVirtualFileIfNeeded(ToAST, FileName, Code);
+    }
+
+    Decl *import(ASTUnit *ToAST, Decl *FromDecl) {
+      lazyInitImporter(ToAST);
       return Importer->Import(FromDecl);
+     }
+
+    QualType import(ASTUnit *ToAST, QualType FromType) {
+      lazyInitImporter(ToAST);
+      return Importer->Import(FromType);
     }
   };
 
@@ -321,6 +332,26 @@ class ASTImporterTestBase : public ParameterizedTestsFixture {
   // vector is expanding, with the list we won't have these issues.
   std::list<TU> FromTUs;
 
+  void lazyInitToAST(Language ToLang) {
+    if (ToAST)
+      return;
+    ArgVector ToArgs = getArgVectorForLanguage(ToLang);
+    // Build the AST from an empty file.
+    ToAST = tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
+    ToAST->enableSourceFileDiagnostics();
+  }
+
+  TU *findFromTU(Decl *From) {
+    // Create a virtual file in the To Ctx which corresponds to the file from
+    // which we want to import the `From` Decl. Without this source locations
+    // will be invalid in the ToCtx.
+    auto It = std::find_if(FromTUs.begin(), FromTUs.end(), [From](const TU &E) {
+      return E.TUDecl == From->getTranslationUnitDecl();
+    });
+    assert(It != FromTUs.end());
+    return &*It;
+  }
+
 public:
   // We may have several From context but only one To context.
   std::unique_ptr<ASTUnit> ToAST;
@@ -394,26 +425,17 @@ public:
   // May be called several times in a given test.
   // The different instances of the param From may have different ASTContext.
   Decl *Import(Decl *From, Language ToLang) {
-    if (!ToAST) {
-      ArgVector ToArgs = getArgVectorForLanguage(ToLang);
-      // Build the AST from an empty file.
-      ToAST =
-          tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
-      ToAST->enableSourceFileDiagnostics();
-    }
-
-    // Create a virtual file in the To Ctx which corresponds to the file from
-    // which we want to import the `From` Decl. Without this source locations
-    // will be invalid in the ToCtx.
-    auto It = std::find_if(FromTUs.begin(), FromTUs.end(), [From](const TU &E) {
-      return E.TUDecl == From->getTranslationUnitDecl();
-    });
-    assert(It != FromTUs.end());
-    createVirtualFileIfNeeded(ToAST.get(), It->FileName, It->Code);
-
-    return It->import(ToAST.get(), From);
+    lazyInitToAST(ToLang);
+    TU *FromTU = findFromTU(From);
+    return FromTU->import(ToAST.get(), From);
   }
 
+  QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang) {
+    lazyInitToAST(ToLang);
+    TU *FromTU = findFromTU(TUDecl);
+    return FromTU->import(ToAST.get(), FromType);
+   }
+
   ~ASTImporterTestBase() {
     if (!::testing::Test::HasFailure()) return;
 
@@ -943,6 +965,44 @@ TEST_P(ImportExpr, ImportTypeTraitExprValDep) {
               typeTraitExpr(hasType(booleanType())))))))));
 }
 
+TEST_P(ImportDecl, ImportRecordDeclInFunc) {
+  MatchVerifier<Decl> Verifier;
+  testImport("int declToImport() { "
+             "  struct data_t {int a;int b;};"
+             "  struct data_t d;"
+             "  return 0;"
+             "}",
+             Lang_C, "", Lang_C, Verifier,
+             functionDecl(hasBody(compoundStmt(
+                 has(declStmt(hasSingleDecl(varDecl(hasName("d")))))))));
+}
+
+TEST_P(ASTImporterTestBase, ImportRecordTypeInFunc) {
+  Decl *FromTU = getTuDecl("int declToImport() { "
+                           "  struct data_t {int a;int b;};"
+                           "  struct data_t d;"
+                           "  return 0;"
+                           "}",
+                           Lang_C, "input.c");
+  auto FromVar =
+      FirstDeclMatcher<VarDecl>().match(FromTU, varDecl(hasName("d")));
+  ASSERT_TRUE(FromVar);
+  auto ToType =
+      ImportType(FromVar->getType().getCanonicalType(), FromVar, Lang_C);
+  EXPECT_FALSE(ToType.isNull());
+}
+
+TEST_P(ASTImporterTestBase, ImportRecordDeclInFuncParams) {
+  // This construct is not supported by ASTImporter.
+  Decl *FromTU =
+      getTuDecl("int declToImport(struct data_t{int a;int b;} *d){ return 0; }",
+                Lang_C, "input.c");
+  auto From = FirstDeclMatcher<FunctionDecl>().match(FromTU, functionDecl());
+  ASSERT_TRUE(From);
+  auto To = Import(From, Lang_C);
+  EXPECT_EQ(To, nullptr);
+}
+
 const internal::VariadicDynCastAllOfMatcher<Expr, CXXPseudoDestructorExpr>
     cxxPseudoDestructorExpr;