On 09/24/2014 03:05 PM, Brad King wrote:> On 09/15/2014 02:48 PM, Brad King wrote: >> On 09/15/2014 02:42 PM, Reid Kleckner wrote: >>> lgtm >> >> Thanks. I do not have commit access. Would someone apply, please? > > Ping. I've confirmed that the patches still work on r218400.
Adding to Cc some folks that touched delayed template parsing and/or incremental processing during the last few years. The attached patches work on r219684. Thanks, -Brad
>From 83516913e2905b9e7cc49894d54b692dd8a0e8b1 Mon Sep 17 00:00:00 2001 Message-Id: <83516913e2905b9e7cc49894d54b692dd8a0e8b1.1413301628.git.brad.k...@kitware.com> From: Brad King <[email protected]> Date: Tue, 19 Aug 2014 14:11:34 -0400 Subject: [PATCH 1/2] Demonstrate late template parsing leak with incremental processing Add a case to FrontendActionTest showing an assertion failure when enableIncrementalProcessing and DelayedTemplateParsing are used together. The CleanupRAII object in Parser::ParseTopLevelDecl goes out of scope without a call to ActOnEndOfTranslationUnit. If the application later calls ActOnEndOfTranslationUnit, perhaps in its override of ASTConsumer::HandleTranslationUnit, then late template parsing is performed and allocates TemplateIdAnnotation instances that are never freed. An assertion in Parser::~Parser fails. --- unittests/Frontend/FrontendActionTest.cpp | 49 ++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/unittests/Frontend/FrontendActionTest.cpp b/unittests/Frontend/FrontendActionTest.cpp index 3171156..9973d3f 100644 --- a/unittests/Frontend/FrontendActionTest.cpp +++ b/unittests/Frontend/FrontendActionTest.cpp @@ -14,6 +14,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Sema/Sema.h" #include "llvm/ADT/Triple.h" #include "llvm/Support/MemoryBuffer.h" #include "gtest/gtest.h" @@ -25,10 +26,13 @@ namespace { class TestASTFrontendAction : public ASTFrontendAction { public: - TestASTFrontendAction(bool enableIncrementalProcessing = false) - : EnableIncrementalProcessing(enableIncrementalProcessing) { } + TestASTFrontendAction(bool enableIncrementalProcessing = false, + bool actOnEndOfTranslationUnit = false) + : EnableIncrementalProcessing(enableIncrementalProcessing), + ActOnEndOfTranslationUnit(actOnEndOfTranslationUnit) { } bool EnableIncrementalProcessing; + bool ActOnEndOfTranslationUnit; std::vector<std::string> decl_names; virtual bool BeginSourceFileAction(CompilerInstance &ci, StringRef filename) { @@ -40,15 +44,22 @@ public: virtual std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { - return llvm::make_unique<Visitor>(decl_names); + return llvm::make_unique<Visitor>(CI, ActOnEndOfTranslationUnit, + decl_names); } private: class Visitor : public ASTConsumer, public RecursiveASTVisitor<Visitor> { public: - Visitor(std::vector<std::string> &decl_names) : decl_names_(decl_names) {} + Visitor(CompilerInstance &ci, bool actOnEndOfTranslationUnit, + std::vector<std::string> &decl_names) : + CI(ci), ActOnEndOfTranslationUnit(actOnEndOfTranslationUnit), + decl_names_(decl_names) {} virtual void HandleTranslationUnit(ASTContext &context) { + if (ActOnEndOfTranslationUnit) { + CI.getSema().ActOnEndOfTranslationUnit(); + } TraverseDecl(context.getTranslationUnitDecl()); } @@ -58,6 +69,8 @@ private: } private: + CompilerInstance &CI; + bool ActOnEndOfTranslationUnit; std::vector<std::string> &decl_names_; }; }; @@ -102,6 +115,34 @@ TEST(ASTFrontendAction, IncrementalParsing) { EXPECT_EQ("x", test_action.decl_names[1]); } +TEST(ASTFrontendAction, LateTemplateIncrementalParsing) { + CompilerInvocation *invocation = new CompilerInvocation; + invocation->getLangOpts()->CPlusPlus = true; + invocation->getLangOpts()->DelayedTemplateParsing = true; + invocation->getPreprocessorOpts().addRemappedFile( + "test.cc", MemoryBuffer::getMemBuffer( + "template<typename T> struct A { A(T); T data; };\n" + "template<typename T> struct B: public A<T> {\n" + " B();\n" + " B(B const& b): A<T>(b.data) {}\n" + "};\n" + "B<char> c() { return B<char>(); }\n").release()); + invocation->getFrontendOpts().Inputs.push_back(FrontendInputFile("test.cc", + IK_CXX)); + invocation->getFrontendOpts().ProgramAction = frontend::ParseSyntaxOnly; + invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu"; + CompilerInstance compiler; + compiler.setInvocation(invocation); + compiler.createDiagnostics(); + + TestASTFrontendAction test_action(/*enableIncrementalProcessing=*/true, + /*actOnEndOfTranslationUnit=*/true); + ASSERT_TRUE(compiler.ExecuteAction(test_action)); + ASSERT_EQ(13U, test_action.decl_names.size()); + EXPECT_EQ("A", test_action.decl_names[0]); + EXPECT_EQ("c", test_action.decl_names[12]); +} + struct TestPPCallbacks : public PPCallbacks { TestPPCallbacks() : SeenEnd(false) {} -- 2.1.1
>From bc2b77c7e292a0cffe4792b4b588286ee8769942 Mon Sep 17 00:00:00 2001 Message-Id: <bc2b77c7e292a0cffe4792b4b588286ee8769942.1413301628.git.brad.k...@kitware.com> In-Reply-To: <83516913e2905b9e7cc49894d54b692dd8a0e8b1.1413301628.git.brad.k...@kitware.com> References: <83516913e2905b9e7cc49894d54b692dd8a0e8b1.1413301628.git.brad.k...@kitware.com> From: Brad King <[email protected]> Date: Tue, 19 Aug 2014 15:19:57 -0400 Subject: [PATCH 2/2] Fix late template parsing leak with incremental processing Add a second late template parser callback meant to cleanup any resources allocated by late template parsing. Call it from the Sema::ActOnEndOfTranslationUnit method after all pending template instantiations have been completed. Teach Parser::ParseTopLevelDecl to install the cleanup callback when incremental processing is enabled so that Parser::TemplateIds can be freed. --- include/clang/Parse/Parser.h | 1 + include/clang/Sema/Sema.h | 7 ++++++- lib/Parse/Parser.cpp | 9 ++++++++- lib/Sema/Sema.cpp | 3 +++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 5aaa6ca..6d53396 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1155,6 +1155,7 @@ private: void ParseLateTemplatedFuncDef(LateParsedTemplate &LPT); static void LateTemplateParserCallback(void *P, LateParsedTemplate &LPT); + static void LateTemplateParserCleanupCallback(void *P); Sema::ParsingClassState PushParsingClass(Decl *TagOrTemplate, bool TopLevelClass, bool IsInterface); diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index fa53a38..3ccd7e4 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -480,11 +480,16 @@ public: /// \brief Callback to the parser to parse templated functions when needed. typedef void LateTemplateParserCB(void *P, LateParsedTemplate &LPT); + typedef void LateTemplateParserCleanupCB(void *P); LateTemplateParserCB *LateTemplateParser; + LateTemplateParserCleanupCB *LateTemplateParserCleanup; void *OpaqueParser; - void SetLateTemplateParser(LateTemplateParserCB *LTP, void *P) { + void SetLateTemplateParser(LateTemplateParserCB *LTP, + LateTemplateParserCleanupCB *LTPCleanup, + void *P) { LateTemplateParser = LTP; + LateTemplateParserCleanup = LTPCleanup; OpaqueParser = P; } diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 5a06530..641543f 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -510,6 +510,10 @@ namespace { }; } +void Parser::LateTemplateParserCleanupCallback(void *P) { + DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(((Parser *)P)->TemplateIds); +} + /// ParseTopLevelDecl - Parse one top-level declaration, return whatever the /// action tells us to. This returns true if the EOF was encountered. bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result) { @@ -542,7 +546,10 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result) { case tok::eof: // Late template parsing can begin. if (getLangOpts().DelayedTemplateParsing) - Actions.SetLateTemplateParser(LateTemplateParserCallback, this); + Actions.SetLateTemplateParser(LateTemplateParserCallback, + PP.isIncrementalProcessingEnabled() ? + LateTemplateParserCleanupCallback : nullptr, + this); if (!PP.isIncrementalProcessingEnabled()) Actions.ActOnEndOfTranslationUnit(); //else don't tell Sema that we ended parsing: more input might come. diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index aae2008..70e4eaa 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -672,6 +672,9 @@ void Sema::ActOnEndOfTranslationUnit() { } PerformPendingInstantiations(); + if (LateTemplateParserCleanup) + LateTemplateParserCleanup(OpaqueParser); + CheckDelayedMemberExceptionSpecs(); } -- 2.1.1
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
