[clang-tools-extra] [clang-doc] Avoid merging into default Info types (PR #191644)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 11 10:04:42 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Paul Kirth (ilovepi)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/191644.diff
2 Files Affected:
- (modified) clang-tools-extra/clang-doc/Representation.cpp (+87-29)
- (modified) clang-tools-extra/clang-doc/Representation.h (+6)
``````````diff
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);
``````````
</details>
https://github.com/llvm/llvm-project/pull/191644
More information about the cfe-commits
mailing list