browneee created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman, martong.
Herald added a reviewer: shafik.
Herald added a project: All.
browneee requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126944
Files:
clang-tools-extra/clangd/AST.cpp
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
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3801,13 +3801,14 @@
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 +5555,18 @@
// declaration of the definition.
TemplateDeclInstantiator Instantiator(*this, Var->getDeclContext(),
TemplateArgs);
+
+ TemplateArgumentListInfo TemplateArgInfo;
+ if (auto *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 *,
Index: clang/lib/AST/TemplateBase.cpp
===================================================================
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -617,6 +617,15 @@
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 +637,17 @@
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) {
Index: clang/lib/AST/DeclTemplate.cpp
===================================================================
--- clang/lib/AST/DeclTemplate.cpp
+++ clang/lib/AST/DeclTemplate.cpp
@@ -1335,10 +1335,14 @@
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);
}
//===----------------------------------------------------------------------===//
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -6018,9 +6018,11 @@
return TInfoOrErr.takeError();
TemplateArgumentListInfo ToTAInfo;
- if (Error Err = ImportTemplateArgumentListInfo(
- D->getTemplateArgsInfo(), ToTAInfo))
- return std::move(Err);
+ if (auto *Args = D->getTemplateArgsInfo()) {
+ if (Error Err = ImportTemplateArgumentListInfo(
+ *Args, ToTAInfo))
+ return std::move(Err);
+ }
using PartVarSpecDecl = VarTemplatePartialSpecializationDecl;
// Create a new specialization.
Index: clang/include/clang/AST/TemplateBase.h
===================================================================
--- clang/include/clang/AST/TemplateBase.h
+++ clang/include/clang/AST/TemplateBase.h
@@ -618,6 +618,9 @@
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 @@
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.,
Index: clang/include/clang/AST/DeclTemplate.h
===================================================================
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -2718,7 +2718,7 @@
/// 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 @@
// 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;
}
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -53,8 +53,10 @@
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;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
