[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 10:24:42 PDT 2026
https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/191644
>From 3a1266f02c30cf94bd0f76ddd3634d7a2007e657 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/2] [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 b88c707214f7f..1334c33d04c47 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 fde16a19d5a1f385166e0db04ad9a30864d6b455 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/2] 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);
}
More information about the cfe-commits
mailing list