[clang-tools-extra] [clang-doc] Avoid merging into default Info types (PR #191644)
Paul Kirth via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 11 17:20:20 PDT 2026
https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/191644
>From 88a879434b0209e2099ef693269c3204c461b500 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Sat, 11 Apr 2026 16:55:09 +0000
Subject: [PATCH 1/3] [clang-doc] Avoid merging into default Info types
When merging into arenas, the code assumed that all using a default
constructed info would be safe, since in the merge we replace any
differing data. However, that appears to be a risky assumption, due
to default initialized members participating in comparisons, and
other operations, leading the program to read garbage data in some
cases. Earlier patches added default initializers to these fields,
but we should prefer (which the old code used to do) to just start
with properly initialized and complete data from the start.
This patch updates the remaining Info types to have copy constructors
that support choosing the arena to allocate into. This is already the
strategy used in several places to avoid use after free bugs. Since
the handling is now uniform, we can simplify things a bit at the same
time and extract the cloning operation into a helper, making the logic
very clear.
This should avoid any potential pitfalls or missed cases that resulted
in the errors discover after landing #190054.
---
.../clang-doc/Representation.cpp | 116 +++++++++++++-----
clang-tools-extra/clang-doc/Representation.h | 6 +
2 files changed, 93 insertions(+), 29 deletions(-)
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
index 50c5f9bb1d071..4ea0b0eb2fc3e 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -174,39 +174,47 @@ void mergeUnkeyed<OwningVec<CommentInfo>>(OwningVec<CommentInfo> &Target,
}
}
+static llvm::Expected<doc::OwnedPtr<doc::Info>>
+cloneInfo(const doc::Info &Src, llvm::BumpPtrAllocator &Arena) {
+ switch (Src.IT) {
+ case InfoType::IT_namespace:
+ return allocatePtr<NamespaceInfo>(
+ Arena, static_cast<const NamespaceInfo &>(Src), Arena);
+ case InfoType::IT_record:
+ return allocatePtr<RecordInfo>(Arena, static_cast<const RecordInfo &>(Src),
+ Arena);
+ case InfoType::IT_enum:
+ return allocatePtr<EnumInfo>(Arena, static_cast<const EnumInfo &>(Src),
+ Arena);
+ case InfoType::IT_function:
+ return allocatePtr<FunctionInfo>(
+ Arena, static_cast<const FunctionInfo &>(Src), Arena);
+ case InfoType::IT_typedef:
+ return allocatePtr<TypedefInfo>(Arena, static_cast<const TypedefInfo &>(Src),
+ Arena);
+ case InfoType::IT_concept:
+ return allocatePtr<ConceptInfo>(Arena, static_cast<const ConceptInfo &>(Src),
+ Arena);
+ case InfoType::IT_variable:
+ return allocatePtr<VarInfo>(Arena, static_cast<const VarInfo &>(Src), Arena);
+ case InfoType::IT_friend:
+ return allocatePtr<FriendInfo>(Arena, static_cast<const FriendInfo &>(Src),
+ Arena);
+ default:
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "unknown info type");
+ }
+}
+
llvm::Error mergeSingleInfo(doc::OwnedPtr<doc::Info> &Reduced,
doc::OwnedPtr<doc::Info> &&NewInfo,
llvm::BumpPtrAllocator &Arena) {
if (!Reduced) {
- switch (NewInfo->IT) {
- case InfoType::IT_namespace:
- Reduced = allocatePtr<NamespaceInfo>(Arena, NewInfo->USR);
- break;
- case InfoType::IT_record:
- Reduced = allocatePtr<RecordInfo>(Arena, NewInfo->USR);
- break;
- case InfoType::IT_enum:
- Reduced = allocatePtr<EnumInfo>(Arena, NewInfo->USR);
- break;
- case InfoType::IT_function:
- Reduced = allocatePtr<FunctionInfo>(Arena, NewInfo->USR);
- break;
- case InfoType::IT_typedef:
- Reduced = allocatePtr<TypedefInfo>(Arena, NewInfo->USR);
- break;
- case InfoType::IT_concept:
- Reduced = allocatePtr<ConceptInfo>(Arena, NewInfo->USR);
- break;
- case InfoType::IT_variable:
- Reduced = allocatePtr<VarInfo>(Arena, NewInfo->USR);
- break;
- case InfoType::IT_friend:
- Reduced = allocatePtr<FriendInfo>(Arena, NewInfo->USR);
- break;
- default:
- return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "unknown info type");
- }
+ auto Cloned = cloneInfo(*NewInfo, Arena);
+ if (!Cloned)
+ return Cloned.takeError();
+ Reduced = *Cloned;
+ return llvm::Error::success();
}
if (Reduced->IT != NewInfo->IT)
@@ -485,6 +493,54 @@ SymbolInfo::SymbolInfo(const SymbolInfo &Other, llvm::BumpPtrAllocator &Arena)
}
}
+NamespaceInfo::NamespaceInfo(const NamespaceInfo &Other, llvm::BumpPtrAllocator &Arena)
+ : Info(Other, Arena) {
+ for (const auto &N : Other.Children.Namespaces)
+ Children.Namespaces.push_back(*allocatePtr<Reference>(Arena, N));
+ for (const auto &R : Other.Children.Records)
+ Children.Records.push_back(*allocatePtr<Reference>(Arena, R));
+ for (const auto &F : Other.Children.Functions)
+ Children.Functions.push_back(*allocatePtr<FunctionInfo>(Arena, F, Arena));
+ for (const auto &E : Other.Children.Enums)
+ Children.Enums.push_back(*allocatePtr<EnumInfo>(Arena, E, Arena));
+ for (const auto &T : Other.Children.Typedefs)
+ Children.Typedefs.push_back(*allocatePtr<TypedefInfo>(Arena, T, Arena));
+ for (const auto &C : Other.Children.Concepts)
+ Children.Concepts.push_back(*allocatePtr<ConceptInfo>(Arena, C, Arena));
+ for (const auto &V : Other.Children.Variables)
+ Children.Variables.push_back(*allocatePtr<VarInfo>(Arena, V, Arena));
+}
+
+VarInfo::VarInfo(const VarInfo &Other, llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), Type(Other.Type) {}
+
+FunctionInfo::FunctionInfo(const FunctionInfo &Other, llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), Parent(Other.Parent), ReturnType(Other.ReturnType),
+ Access(Other.Access), IsMethod(Other.IsMethod) {
+ Prototype = internString(Other.Prototype);
+ Params = allocateArray(Other.Params, Arena);
+ if (Other.Template)
+ Template = TemplateInfo(*Other.Template, Arena);
+}
+
+TypedefInfo::TypedefInfo(const TypedefInfo &Other, llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), Underlying(Other.Underlying), IsUsing(Other.IsUsing) {
+ TypeDeclaration = internString(Other.TypeDeclaration);
+ if (Other.Template)
+ Template = TemplateInfo(*Other.Template, Arena);
+}
+
+EnumInfo::EnumInfo(const EnumInfo &Other, llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), Scoped(Other.Scoped) {
+ BaseType = Other.BaseType;
+ Members = deepCopyArray(Other.Members, Arena);
+}
+
+ConceptInfo::ConceptInfo(const ConceptInfo &Other, llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), IsType(Other.IsType), Template(Other.Template, Arena) {
+ ConstraintExpression = internString(Other.ConstraintExpression);
+}
+
void SymbolInfo::merge(SymbolInfo &&Other) {
assert(mergeable(Other));
if (!DefLoc)
@@ -530,6 +586,8 @@ RecordInfo::RecordInfo(const RecordInfo &Other, llvm::BumpPtrAllocator &Arena)
VirtualParents = allocateArray(Other.VirtualParents, Arena);
Bases = deepCopyArray(Other.Bases, Arena);
Friends = deepCopyArray(Other.Friends, Arena);
+ if (Other.Template)
+ Template = TemplateInfo(*Other.Template, Arena);
}
MemberTypeInfo::MemberTypeInfo(const MemberTypeInfo &Other,
diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h
index 5f404463b012e..135cf2a563fce 100644
--- a/clang-tools-extra/clang-doc/Representation.h
+++ b/clang-tools-extra/clang-doc/Representation.h
@@ -528,6 +528,7 @@ inline Context::Context(const Info &I)
struct NamespaceInfo : public Info {
NamespaceInfo(SymbolID USR = SymbolID(), StringRef Name = StringRef(),
StringRef Path = StringRef());
+ NamespaceInfo(const NamespaceInfo &Other, llvm::BumpPtrAllocator &Arena);
void merge(NamespaceInfo &&I);
@@ -585,6 +586,7 @@ struct FriendInfo : public SymbolInfo, public llvm::ilist_node<FriendInfo> {
struct VarInfo : public SymbolInfo, public llvm::ilist_node<VarInfo> {
VarInfo() : SymbolInfo(InfoType::IT_variable) {}
explicit VarInfo(SymbolID USR) : SymbolInfo(InfoType::IT_variable, USR) {}
+ VarInfo(const VarInfo &Other, llvm::BumpPtrAllocator &Arena);
void merge(VarInfo &&I);
@@ -596,6 +598,7 @@ struct VarInfo : public SymbolInfo, public llvm::ilist_node<VarInfo> {
struct FunctionInfo : public SymbolInfo, public llvm::ilist_node<FunctionInfo> {
FunctionInfo(SymbolID USR = SymbolID())
: SymbolInfo(InfoType::IT_function, USR) {}
+ FunctionInfo(const FunctionInfo &Other, llvm::BumpPtrAllocator &Arena);
void merge(FunctionInfo &&I);
@@ -659,6 +662,7 @@ struct RecordInfo : public SymbolInfo {
struct TypedefInfo : public SymbolInfo, public llvm::ilist_node<TypedefInfo> {
TypedefInfo(SymbolID USR = SymbolID())
: SymbolInfo(InfoType::IT_typedef, USR) {}
+ TypedefInfo(const TypedefInfo &Other, llvm::BumpPtrAllocator &Arena);
void merge(TypedefInfo &&I);
@@ -727,6 +731,7 @@ struct EnumValueInfo {
struct EnumInfo : public SymbolInfo, public llvm::ilist_node<EnumInfo> {
EnumInfo() : SymbolInfo(InfoType::IT_enum) {}
EnumInfo(SymbolID USR) : SymbolInfo(InfoType::IT_enum, USR) {}
+ EnumInfo(const EnumInfo &Other, llvm::BumpPtrAllocator &Arena);
void merge(EnumInfo &&I);
@@ -744,6 +749,7 @@ struct EnumInfo : public SymbolInfo, public llvm::ilist_node<EnumInfo> {
struct ConceptInfo : public SymbolInfo, public llvm::ilist_node<ConceptInfo> {
ConceptInfo() : SymbolInfo(InfoType::IT_concept) {}
ConceptInfo(SymbolID USR) : SymbolInfo(InfoType::IT_concept, USR) {}
+ ConceptInfo(const ConceptInfo &Other, llvm::BumpPtrAllocator &Arena);
void merge(ConceptInfo &&I);
>From 36f5db2533495f332222949342b16ba2ca82d9e5 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Sat, 11 Apr 2026 17:24:07 +0000
Subject: [PATCH 2/3] fix formatting
---
.../clang-doc/Representation.cpp | 34 ++++++++++++-------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
index 4ea0b0eb2fc3e..c99f3bd816dc6 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -190,13 +190,14 @@ cloneInfo(const doc::Info &Src, llvm::BumpPtrAllocator &Arena) {
return allocatePtr<FunctionInfo>(
Arena, static_cast<const FunctionInfo &>(Src), Arena);
case InfoType::IT_typedef:
- return allocatePtr<TypedefInfo>(Arena, static_cast<const TypedefInfo &>(Src),
- Arena);
+ return allocatePtr<TypedefInfo>(
+ Arena, static_cast<const TypedefInfo &>(Src), Arena);
case InfoType::IT_concept:
- return allocatePtr<ConceptInfo>(Arena, static_cast<const ConceptInfo &>(Src),
- Arena);
+ return allocatePtr<ConceptInfo>(
+ Arena, static_cast<const ConceptInfo &>(Src), Arena);
case InfoType::IT_variable:
- return allocatePtr<VarInfo>(Arena, static_cast<const VarInfo &>(Src), Arena);
+ return allocatePtr<VarInfo>(Arena, static_cast<const VarInfo &>(Src),
+ Arena);
case InfoType::IT_friend:
return allocatePtr<FriendInfo>(Arena, static_cast<const FriendInfo &>(Src),
Arena);
@@ -493,7 +494,8 @@ SymbolInfo::SymbolInfo(const SymbolInfo &Other, llvm::BumpPtrAllocator &Arena)
}
}
-NamespaceInfo::NamespaceInfo(const NamespaceInfo &Other, llvm::BumpPtrAllocator &Arena)
+NamespaceInfo::NamespaceInfo(const NamespaceInfo &Other,
+ llvm::BumpPtrAllocator &Arena)
: Info(Other, Arena) {
for (const auto &N : Other.Children.Namespaces)
Children.Namespaces.push_back(*allocatePtr<Reference>(Arena, N));
@@ -514,17 +516,21 @@ NamespaceInfo::NamespaceInfo(const NamespaceInfo &Other, llvm::BumpPtrAllocator
VarInfo::VarInfo(const VarInfo &Other, llvm::BumpPtrAllocator &Arena)
: SymbolInfo(Other, Arena), Type(Other.Type) {}
-FunctionInfo::FunctionInfo(const FunctionInfo &Other, llvm::BumpPtrAllocator &Arena)
- : SymbolInfo(Other, Arena), Parent(Other.Parent), ReturnType(Other.ReturnType),
- Access(Other.Access), IsMethod(Other.IsMethod) {
+FunctionInfo::FunctionInfo(const FunctionInfo &Other,
+ llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), Parent(Other.Parent),
+ ReturnType(Other.ReturnType), Access(Other.Access),
+ IsMethod(Other.IsMethod) {
Prototype = internString(Other.Prototype);
Params = allocateArray(Other.Params, Arena);
if (Other.Template)
Template = TemplateInfo(*Other.Template, Arena);
}
-TypedefInfo::TypedefInfo(const TypedefInfo &Other, llvm::BumpPtrAllocator &Arena)
- : SymbolInfo(Other, Arena), Underlying(Other.Underlying), IsUsing(Other.IsUsing) {
+TypedefInfo::TypedefInfo(const TypedefInfo &Other,
+ llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), Underlying(Other.Underlying),
+ IsUsing(Other.IsUsing) {
TypeDeclaration = internString(Other.TypeDeclaration);
if (Other.Template)
Template = TemplateInfo(*Other.Template, Arena);
@@ -536,8 +542,10 @@ EnumInfo::EnumInfo(const EnumInfo &Other, llvm::BumpPtrAllocator &Arena)
Members = deepCopyArray(Other.Members, Arena);
}
-ConceptInfo::ConceptInfo(const ConceptInfo &Other, llvm::BumpPtrAllocator &Arena)
- : SymbolInfo(Other, Arena), IsType(Other.IsType), Template(Other.Template, Arena) {
+ConceptInfo::ConceptInfo(const ConceptInfo &Other,
+ llvm::BumpPtrAllocator &Arena)
+ : SymbolInfo(Other, Arena), IsType(Other.IsType),
+ Template(Other.Template, Arena) {
ConstraintExpression = internString(Other.ConstraintExpression);
}
>From e3d7dee6de48a86d98d081286d5639aecce2e096 Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Sun, 12 Apr 2026 00:15:58 +0000
Subject: [PATCH 3/3] Use copy constructor for ScopeChildren
---
.../clang-doc/Representation.cpp | 55 +++++++++----------
clang-tools-extra/clang-doc/Representation.h | 3 +
2 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
index c99f3bd816dc6..a54fa25fb1970 100644
--- a/clang-tools-extra/clang-doc/Representation.cpp
+++ b/clang-tools-extra/clang-doc/Representation.cpp
@@ -98,11 +98,10 @@ static llvm::Expected<OwnedPtr<Info>> reduce(OwningPtrArray<Info> &Values) {
if (Values.empty() || !Values[0])
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"no value to reduce");
- OwnedPtr<Info> Merged = allocatePtr<T>(Values[0]->USR);
- T *Tmp = static_cast<T *>(getPtr(Merged));
- for (auto &I : Values)
- Tmp->merge(std::move(*static_cast<T *>(getPtr(I))));
- return std::move(Merged);
+ T *Merged = allocatePtr<T>(*static_cast<T *>(Values[0]), TransientArena);
+ for (const auto &I : llvm::ArrayRef<OwnedPtr<Info>>(Values).drop_front(1))
+ Merged->merge(std::move(*static_cast<T *>(I)));
+ return Merged;
}
template <typename T>
@@ -115,8 +114,8 @@ static void reduceChildren(llvm::simple_ilist<T> &Children,
auto It = llvm::find_if(
Children, [&](const T &C) { return C.USR == ChildToMerge->USR; });
if (It == Children.end()) {
- T *NewChild = allocatePtr<T>(PersistentArena, ChildToMerge->USR);
- NewChild->merge(std::move(*ChildToMerge));
+ T *NewChild =
+ allocatePtr<T>(PersistentArena, *ChildToMerge, PersistentArena);
Children.push_back(*NewChild);
} else {
It->merge(std::move(*ChildToMerge));
@@ -494,24 +493,27 @@ SymbolInfo::SymbolInfo(const SymbolInfo &Other, llvm::BumpPtrAllocator &Arena)
}
}
+ScopeChildren::ScopeChildren(const ScopeChildren &Other,
+ llvm::BumpPtrAllocator &Arena) {
+ for (const auto &N : Other.Namespaces)
+ Namespaces.push_back(*allocatePtr<Reference>(Arena, N));
+ for (const auto &R : Other.Records)
+ Records.push_back(*allocatePtr<Reference>(Arena, R));
+ for (const auto &F : Other.Functions)
+ Functions.push_back(*allocatePtr<FunctionInfo>(Arena, F, Arena));
+ for (const auto &E : Other.Enums)
+ Enums.push_back(*allocatePtr<EnumInfo>(Arena, E, Arena));
+ for (const auto &T : Other.Typedefs)
+ Typedefs.push_back(*allocatePtr<TypedefInfo>(Arena, T, Arena));
+ for (const auto &C : Other.Concepts)
+ Concepts.push_back(*allocatePtr<ConceptInfo>(Arena, C, Arena));
+ for (const auto &V : Other.Variables)
+ Variables.push_back(*allocatePtr<VarInfo>(Arena, V, Arena));
+}
+
NamespaceInfo::NamespaceInfo(const NamespaceInfo &Other,
llvm::BumpPtrAllocator &Arena)
- : Info(Other, Arena) {
- for (const auto &N : Other.Children.Namespaces)
- Children.Namespaces.push_back(*allocatePtr<Reference>(Arena, N));
- for (const auto &R : Other.Children.Records)
- Children.Records.push_back(*allocatePtr<Reference>(Arena, R));
- for (const auto &F : Other.Children.Functions)
- Children.Functions.push_back(*allocatePtr<FunctionInfo>(Arena, F, Arena));
- for (const auto &E : Other.Children.Enums)
- Children.Enums.push_back(*allocatePtr<EnumInfo>(Arena, E, Arena));
- for (const auto &T : Other.Children.Typedefs)
- Children.Typedefs.push_back(*allocatePtr<TypedefInfo>(Arena, T, Arena));
- for (const auto &C : Other.Children.Concepts)
- Children.Concepts.push_back(*allocatePtr<ConceptInfo>(Arena, C, Arena));
- for (const auto &V : Other.Children.Variables)
- Children.Variables.push_back(*allocatePtr<VarInfo>(Arena, V, Arena));
-}
+ : Info(Other, Arena), Children(Other.Children, Arena) {}
VarInfo::VarInfo(const VarInfo &Other, llvm::BumpPtrAllocator &Arena)
: SymbolInfo(Other, Arena), Type(Other.Type) {}
@@ -581,14 +583,9 @@ void NamespaceInfo::merge(NamespaceInfo &&Other) {
RecordInfo::RecordInfo(SymbolID USR, StringRef Name, StringRef Path)
: SymbolInfo(InfoType::IT_record, USR, Name, Path) {}
-// FIXME: This constructor is currently unsafe for cross-arena copies of
-// populated records. Because a default copy of ScopeChildren will shallow-copy
-// the intrusive pointers, leading to a use-after-free when the TransientArena
-// is reset. Subsequent patches will address this by deep-copying children
-// individually via reduceChildren.
RecordInfo::RecordInfo(const RecordInfo &Other, llvm::BumpPtrAllocator &Arena)
: SymbolInfo(Other, Arena), TagType(Other.TagType),
- IsTypeDef(Other.IsTypeDef) {
+ IsTypeDef(Other.IsTypeDef), Children(Other.Children, Arena) {
Members = deepCopyArray(Other.Members, Arena);
Parents = allocateArray(Other.Parents, Arena);
VirtualParents = allocateArray(Other.VirtualParents, Arena);
diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h
index 135cf2a563fce..c0ebc7600c7ae 100644
--- a/clang-tools-extra/clang-doc/Representation.h
+++ b/clang-tools-extra/clang-doc/Representation.h
@@ -325,6 +325,9 @@ struct ScopeChildren {
OwningVec<ConceptInfo> Concepts;
OwningVec<VarInfo> Variables;
+ ScopeChildren() = default;
+ ScopeChildren(const ScopeChildren &Other, llvm::BumpPtrAllocator &Arena);
+
void sort();
};
More information about the cfe-commits
mailing list