Hi Folks, I'm working on a Clang-based tool that wants to detect whether implicit class members may be safely called. It needs to be able to declare and define implicit members in HandleTranslationUnit, so it calls CI.getPreprocessor().enableIncrementalProcessing() to prevent Parser::ParseTopLevelDecl from tearing down infrastructure with Sema::ActOnEndOfTranslationUnit. When my HandleTranslationUnit is done adding the members it calls ActOnEndOfTranslationUnit to finish things up. This works well without -fdelayed-template-parsing.
With -fdelayed-template-parsing enabled (for MSVC compat) then late template parsing allocates TemplateIdAnnotation instances that are never freed. The CleanupRAII guard in ParseTopLevelDecl is no longer in scope while ActOnEndOfTranslationUnit is called. The late template parsing implementation lacks a place to cleanup such resources as required by the change in r154743 (Parser: Don't manage TemplateAnnotationIds in a delayed cleanup pool, 2012-04-14). Here is a patch series to add a test for this case and propose a possible fix. It applies on trunk as of r215995. Patch 0001 modifies unittests/Frontend/FrontendActionTest.cpp to add a test case showing the problem. Patch 0002 proposes to fix this by adding a second callback to perform cleanup after late template parsing. See commit messages for details. Can anyone familiar with the lifetime needed for TemplateIdAnnotation instances suggest another approach? Thanks, -Brad
>From 51a9b6675495be6ae0df4ebd2c1cd7128b9789ae Mon Sep 17 00:00:00 2001 Message-Id: <51a9b6675495be6ae0df4ebd2c1cd7128b9789ae.1408476311.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 cb9cc52..e158c7c 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_; }; }; @@ -100,6 +113,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")); + 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.0.0
>From b962a339facbaf6daff5b4b781ac9cd794b72614 Mon Sep 17 00:00:00 2001 Message-Id: <b962a339facbaf6daff5b4b781ac9cd794b72614.1408476311.git.brad.k...@kitware.com> In-Reply-To: <51a9b6675495be6ae0df4ebd2c1cd7128b9789ae.1408476311.git.brad.k...@kitware.com> References: <51a9b6675495be6ae0df4ebd2c1cd7128b9789ae.1408476311.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 e3d557b..fdb9ef1 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1162,6 +1162,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 eb53a93..b9e3226 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -477,11 +477,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 37ce157..009154b 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 205081b..8875b19 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -651,6 +651,9 @@ void Sema::ActOnEndOfTranslationUnit() { } PerformPendingInstantiations(); + if(LateTemplateParserCleanup) + LateTemplateParserCleanup(OpaqueParser); + CheckDelayedMemberExceptionSpecs(); } -- 2.0.0
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
