[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336898: [ASTImporter] Fix infinite recursion on function
import with struct definition… (authored by martong, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47946?vs=154991&id=155151#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47946
Files:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -1138,8 +1138,26 @@
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(D) && (FunDecl = dyn_cast(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;
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -301,14 +301,25 @@
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 @@
// vector is expanding, with the list we won't have these issues.
std::list 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 ToAST;
@@ -394,26 +425,17 @@
// 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 = find
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
gerazo added a comment. @martong I don't have commit rights. Thanks for your help in advance. https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
martong added a comment. @gerazo, Do you have commit rights, or should I help with the commit? https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
gerazo updated this revision to Diff 154991.
gerazo marked an inline comment as done.
gerazo added a comment.
Minor fixes for Aleksei's comments.
https://reviews.llvm.org/D47946
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -284,19 +284,44 @@
// Buffer for the To context, must live in the test scope.
std::string ToCode;
+ // Represents a "From" translation unit and holds an importer object which we
+ // use to import from this translation unit.
struct TU {
// Buffer for the context, must live in the test scope.
std::string Code;
std::string FileName;
std::unique_ptr Unit;
TranslationUnitDecl *TUDecl = nullptr;
+std::unique_ptr Importer;
+
TU(StringRef Code, StringRef FileName, ArgVector Args)
: Code(Code), FileName(FileName),
Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
this->FileName)),
TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
Unit->enableSourceFileDiagnostics();
}
+
+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);
+}
};
// We may have several From contexts and related translation units. In each
@@ -308,6 +333,26 @@
// vector is expanding, with the list we won't have these issues.
std::list 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 ToAST;
@@ -329,14 +374,10 @@
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
ToAST->enableSourceFileDiagnostics();
-ASTContext &FromCtx = FromTU.Unit->getASTContext(),
- &ToCtx = ToAST->getASTContext();
+ASTContext &FromCtx = FromTU.Unit->getASTContext();
createVirtualFileIfNeeded(ToAST.get(), InputFileName, FromTU.Code);
-ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx,
- FromTU.Unit->getFileManager(), false);
-
IdentifierInfo *ImportedII = &FromCtx.Idents.get(Identifier);
assert(ImportedII && "Declaration with the given identifier "
"should be specified in test!");
@@ -347,7 +388,7 @@
assert(FoundDecls.size() == 1);
-Decl *Imported = Importer.Import(FoundDecls.front());
+Decl *Imported = FromTU.import(ToAST.get(), FoundDecls.front());
assert(Imported);
return std::make_tuple(*FoundDecls.begin(), Imported);
}
@@ -384,30 +425,17 @@
// 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);
-
-ASTContext &FromCtx = From->getASTContext(),
- &ToCtx = ToAST->getASTContext();
-
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
gerazo marked 2 inline comments as done. gerazo added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:234 +assert(ToAST); +createVirtualFileIfNeeded(ToAST.get(), It->FileName, It->Code); +return &*It; a.sidorin wrote: > Can we move the side effect into import()? Sure, thank you. https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
a.sidorin accepted this revision. a.sidorin added a comment. Hello Zoltán, Sorry for the delay. I think the patch is fine, just some minor nits inline. Comment at: unittests/AST/ASTImporterTest.cpp:234 +assert(ToAST); +createVirtualFileIfNeeded(ToAST.get(), It->FileName, It->Code); +return &*It; Can we move the side effect into import()? Comment at: unittests/AST/ASTImporterTest.cpp:1049 + ImportType(FromVar->getType().getCanonicalType(), FromVar, Lang_C); + ASSERT_FALSE(ToType.isNull()); +} EXPECT_FALSE? https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47946: [ASTImporter] Fix infinite recursion on function import with struct definition in parameters
martong added a comment. Ping. https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
