[clang] c7689fd - [Clang] Fix memory leak due to TemplateArgumentListInfo used in AST node.
Andrew Browne via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 8 09:58:43 PDT 2022
Author: Andrew Browne
Date: 2022-06-08T09:58:25-07:00
New Revision: c7689fd552cdf2b37e804a59322bd0661ccdd3c5
URL: https://github.com/llvm/llvm-project/commit/c7689fd552cdf2b37e804a59322bd0661ccdd3c5
DIFF: https://github.com/llvm/llvm-project/commit/c7689fd552cdf2b37e804a59322bd0661ccdd3c5.diff
LOG: [Clang] Fix memory leak due to TemplateArgumentListInfo used in AST node.
It looks like the leak is rooted at the allocation here:
https://github.com/llvm/llvm-project/blob/1a155ee7de3b62a2fabee86fb470a1554fadc54d/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L3857
The VarTemplateSpecializationDecl is allocated using placement new which uses the AST structure for ownership: https://github.com/llvm/llvm-project/blob/1a155ee7de3b62a2fabee86fb470a1554fadc54d/clang/lib/AST/DeclBase.cpp#L99
The problem is the TemplateArgumentListInfo inside https://github.com/llvm/llvm-project/blob/1a155ee7de3b62a2fabee86fb470a1554fadc54d/clang/include/clang/AST/DeclTemplate.h#L2721
This object contains a vector which does not use placement new: https://github.com/llvm/llvm-project/blob/1a155ee7de3b62a2fabee86fb470a1554fadc54d/clang/include/clang/AST/TemplateBase.h#L564
Apparently ASTTemplateArgumentListInfo should be used instead https://github.com/llvm/llvm-project/blob/1a155ee7de3b62a2fabee86fb470a1554fadc54d/clang/include/clang/AST/TemplateBase.h#L575
https://reviews.llvm.org/D125802#3551305
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D126944
Added:
Modified:
clang-tools-extra/clangd/AST.cpp
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/DeclTemplate.h
clang/include/clang/AST/TemplateBase.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/DeclTemplate.cpp
clang/lib/AST/TemplateBase.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 70d98d0e0bb40..ca838618badd9 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -53,8 +53,10 @@ getTemplateSpecializationArgLocs(const NamedDecl &ND) {
llvm::dyn_cast<VarTemplatePartialSpecializationDecl>(&ND)) {
if (auto *Args = Var->getTemplateArgsAsWritten())
return Args->arguments();
- } else if (auto *Var = llvm::dyn_cast<VarTemplateSpecializationDecl>(&ND))
- return Var->getTemplateArgsInfo().arguments();
+ } else if (auto *Var = llvm::dyn_cast<VarTemplateSpecializationDecl>(&ND)) {
+ if (auto *Args = Var->getTemplateArgsInfo())
+ return Args->arguments();
+ }
// We return None for ClassTemplateSpecializationDecls because it does not
// contain TemplateArgumentLoc information.
return llvm::None;
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1e95d3cef51c0..1b1649ac09c87 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -168,6 +168,8 @@ Bug Fixes
`Issue 55562 <https://github.com/llvm/llvm-project/issues/55562>`_.
- Clang will allow calling a ``consteval`` function in a default argument. This
fixes `Issue 48230 <https://github.com/llvm/llvm-project/issues/48230>`_.
+- Fixed memory leak due to ``VarTemplateSpecializationDecl`` using
+ ``TemplateArgumentListInfo`` instead of ``ASTTemplateArgumentListInfo``.
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 66e0bb2c02a1f..a00917913e41d 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -2718,7 +2718,7 @@ class VarTemplateSpecializationDecl : public VarDecl,
/// The template arguments used to describe this specialization.
const TemplateArgumentList *TemplateArgs;
- TemplateArgumentListInfo TemplateArgsInfo;
+ const ASTTemplateArgumentListInfo *TemplateArgsInfo = nullptr;
/// The point where this template was instantiated (if any).
SourceLocation PointOfInstantiation;
@@ -2773,8 +2773,9 @@ class VarTemplateSpecializationDecl : public VarDecl,
// TODO: Always set this when creating the new specialization?
void setTemplateArgsInfo(const TemplateArgumentListInfo &ArgsInfo);
+ void setTemplateArgsInfo(const ASTTemplateArgumentListInfo *ArgsInfo);
- const TemplateArgumentListInfo &getTemplateArgsInfo() const {
+ const ASTTemplateArgumentListInfo *getTemplateArgsInfo() const {
return TemplateArgsInfo;
}
diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h
index e8064121d2796..3ac755ef74a17 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -618,6 +618,9 @@ struct ASTTemplateArgumentListInfo final
ASTTemplateArgumentListInfo(const TemplateArgumentListInfo &List);
+ // FIXME: Is it ever necessary to copy to another context?
+ ASTTemplateArgumentListInfo(const ASTTemplateArgumentListInfo *List);
+
public:
/// The source location of the left angle bracket ('<').
SourceLocation LAngleLoc;
@@ -647,6 +650,10 @@ struct ASTTemplateArgumentListInfo final
static const ASTTemplateArgumentListInfo *
Create(const ASTContext &C, const TemplateArgumentListInfo &List);
+
+ // FIXME: Is it ever necessary to copy to another context?
+ static const ASTTemplateArgumentListInfo *
+ Create(const ASTContext &C, const ASTTemplateArgumentListInfo *List);
};
/// Represents an explicit template argument list in C++, e.g.,
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 5b35b70f09603..21b9f21b8e6e2 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6018,9 +6018,10 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateSpecializationDecl(
return TInfoOrErr.takeError();
TemplateArgumentListInfo ToTAInfo;
- if (Error Err = ImportTemplateArgumentListInfo(
- D->getTemplateArgsInfo(), ToTAInfo))
- return std::move(Err);
+ if (const ASTTemplateArgumentListInfo *Args = D->getTemplateArgsInfo()) {
+ if (Error Err = ImportTemplateArgumentListInfo(*Args, ToTAInfo))
+ return std::move(Err);
+ }
using PartVarSpecDecl = VarTemplatePartialSpecializationDecl;
// Create a new specialization.
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 3b4ed3107e5a8..e7e5f355809b0 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -1335,10 +1335,14 @@ VarTemplateDecl *VarTemplateSpecializationDecl::getSpecializedTemplate() const {
void VarTemplateSpecializationDecl::setTemplateArgsInfo(
const TemplateArgumentListInfo &ArgsInfo) {
- TemplateArgsInfo.setLAngleLoc(ArgsInfo.getLAngleLoc());
- TemplateArgsInfo.setRAngleLoc(ArgsInfo.getRAngleLoc());
- for (const TemplateArgumentLoc &Loc : ArgsInfo.arguments())
- TemplateArgsInfo.addArgument(Loc);
+ TemplateArgsInfo =
+ ASTTemplateArgumentListInfo::Create(getASTContext(), ArgsInfo);
+}
+
+void VarTemplateSpecializationDecl::setTemplateArgsInfo(
+ const ASTTemplateArgumentListInfo *ArgsInfo) {
+ TemplateArgsInfo =
+ ASTTemplateArgumentListInfo::Create(getASTContext(), ArgsInfo);
}
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index 229a8db21ab63..e0f5916a9a0b7 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -617,6 +617,17 @@ ASTTemplateArgumentListInfo::Create(const ASTContext &C,
return new (Mem) ASTTemplateArgumentListInfo(List);
}
+const ASTTemplateArgumentListInfo *
+ASTTemplateArgumentListInfo::Create(const ASTContext &C,
+ const ASTTemplateArgumentListInfo *List) {
+ if (!List)
+ return nullptr;
+ std::size_t size =
+ totalSizeToAlloc<TemplateArgumentLoc>(List->getNumTemplateArgs());
+ void *Mem = C.Allocate(size, alignof(ASTTemplateArgumentListInfo));
+ return new (Mem) ASTTemplateArgumentListInfo(List);
+}
+
ASTTemplateArgumentListInfo::ASTTemplateArgumentListInfo(
const TemplateArgumentListInfo &Info) {
LAngleLoc = Info.getLAngleLoc();
@@ -628,6 +639,17 @@ ASTTemplateArgumentListInfo::ASTTemplateArgumentListInfo(
new (&ArgBuffer[i]) TemplateArgumentLoc(Info[i]);
}
+ASTTemplateArgumentListInfo::ASTTemplateArgumentListInfo(
+ const ASTTemplateArgumentListInfo *Info) {
+ LAngleLoc = Info->getLAngleLoc();
+ RAngleLoc = Info->getRAngleLoc();
+ NumTemplateArgs = Info->getNumTemplateArgs();
+
+ TemplateArgumentLoc *ArgBuffer = getTrailingObjects<TemplateArgumentLoc>();
+ for (unsigned i = 0; i != NumTemplateArgs; ++i)
+ new (&ArgBuffer[i]) TemplateArgumentLoc((*Info)[i]);
+}
+
void ASTTemplateKWAndArgsInfo::initializeFrom(
SourceLocation TemplateKWLoc, const TemplateArgumentListInfo &Info,
TemplateArgumentLoc *OutArgArray) {
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 77c5677b65c87..c89dc8c3513d9 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3801,13 +3801,15 @@ Decl *TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
return nullptr;
// Substitute the current template arguments.
- const TemplateArgumentListInfo &TemplateArgsInfo = D->getTemplateArgsInfo();
- VarTemplateArgsInfo.setLAngleLoc(TemplateArgsInfo.getLAngleLoc());
- VarTemplateArgsInfo.setRAngleLoc(TemplateArgsInfo.getRAngleLoc());
+ if (const ASTTemplateArgumentListInfo *TemplateArgsInfo =
+ D->getTemplateArgsInfo()) {
+ VarTemplateArgsInfo.setLAngleLoc(TemplateArgsInfo->getLAngleLoc());
+ VarTemplateArgsInfo.setRAngleLoc(TemplateArgsInfo->getRAngleLoc());
- if (SemaRef.SubstTemplateArguments(TemplateArgsInfo.arguments(), TemplateArgs,
- VarTemplateArgsInfo))
- return nullptr;
+ if (SemaRef.SubstTemplateArguments(TemplateArgsInfo->arguments(),
+ TemplateArgs, VarTemplateArgsInfo))
+ return nullptr;
+ }
// Check that the template argument list is well-formed for this template.
SmallVector<TemplateArgument, 4> Converted;
@@ -5554,8 +5556,18 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation,
// declaration of the definition.
TemplateDeclInstantiator Instantiator(*this, Var->getDeclContext(),
TemplateArgs);
+
+ TemplateArgumentListInfo TemplateArgInfo;
+ if (const ASTTemplateArgumentListInfo *ArgInfo =
+ VarSpec->getTemplateArgsInfo()) {
+ TemplateArgInfo.setLAngleLoc(ArgInfo->getLAngleLoc());
+ TemplateArgInfo.setRAngleLoc(ArgInfo->getRAngleLoc());
+ for (const TemplateArgumentLoc &Arg : ArgInfo->arguments())
+ TemplateArgInfo.addArgument(Arg);
+ }
+
Var = cast_or_null<VarDecl>(Instantiator.VisitVarTemplateSpecializationDecl(
- VarSpec->getSpecializedTemplate(), Def, VarSpec->getTemplateArgsInfo(),
+ VarSpec->getSpecializedTemplate(), Def, TemplateArgInfo,
VarSpec->getTemplateArgs().asArray(), VarSpec));
if (Var) {
llvm::PointerUnion<VarTemplateDecl *,
More information about the cfe-commits
mailing list