On 09/15/2014 02:42 PM, Reid Kleckner wrote:
> lgtm
Thanks. I do not have commit access. Would someone apply, please?
> + if(LateTemplateParserCleanup)
> + LateTemplateParserCleanup(OpaqueParser);
>
> Should be "if (".
There was another case of that in the test code in patch 0001 too.
Revised patches attached.
Thanks,
-Brad
>From 2153ed8691ff0b5bca60d05a4ec33a2682123f66 Mon Sep 17 00:00:00 2001
Message-Id: <2153ed8691ff0b5bca60d05a4ec33a2682123f66.1410806814.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.0
>From 24f38a651f955e914443b514d663e5df61465ac2 Mon Sep 17 00:00:00 2001
Message-Id: <24f38a651f955e914443b514d663e5df61465ac2.1410806814.git.brad.k...@kitware.com>
In-Reply-To: <2153ed8691ff0b5bca60d05a4ec33a2682123f66.1410806814.git.brad.k...@kitware.com>
References: <2153ed8691ff0b5bca60d05a4ec33a2682123f66.1410806814.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 7fe2c7a..fa2a9f4 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 dedc472..74dd02e 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 808e426..120923c 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 69a4356..ee3698d 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -666,6 +666,9 @@ void Sema::ActOnEndOfTranslationUnit() {
}
PerformPendingInstantiations();
+ if (LateTemplateParserCleanup)
+ LateTemplateParserCleanup(OpaqueParser);
+
CheckDelayedMemberExceptionSpecs();
}
--
2.1.0
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
