[llvm-branch-commits] [clang] [ssaf][UnsafeBufferUsage] Add support for extracting unsafe pointers from all kinds of contributors (PR #184899)
Ziqing Luo via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Mar 16 16:53:29 PDT 2026
https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/184899
>From 48d337f91bc2e1096a0364fad6d796864e18ff9c Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Thu, 5 Mar 2026 13:00:44 -0800
Subject: [PATCH 1/2] [ssaf][UnsafeBufferUsage] Add support for extracting
unsafe pointers from all kinds of contributors
- Generalize the -Wunsafe-buffer-usage API for finding unsafe pointers in all kinds of Decls
- Add support in SSAF-based UnsafeBufferUsage analysis for extracting from various contributors
- Mock implementation of HandleTranslationUnit
rdar://171735836
---
.../Analysis/Analyses/UnsafeBufferUsage.h | 9 +-
.../UnsafeBufferUsage/UnsafeBufferUsage.h | 2 +
.../UnsafeBufferUsageExtractor.cpp | 140 ++++++++++++++----
clang/lib/Analysis/UnsafeBufferUsage.cpp | 61 +++++---
.../UnsafeBufferUsageTest.cpp | 62 +++++++-
5 files changed, 226 insertions(+), 48 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 876682ad779d4..e0d583c735e61 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -201,7 +201,14 @@ bool anyConflict(const llvm::SmallVectorImpl<FixItHint> &FixIts,
const SourceManager &SM);
} // namespace internal
-std::set<const Expr *> findUnsafePointers(const FunctionDecl *FD);
+/// Find unsafe pointers in body/initializer of `D`, if `D` is one of the
+/// followings:
+/// VarDecl
+/// FieldDecl
+/// FunctionDecl
+/// BlockDecl
+/// ObjCMethodDecl
+std::set<const Expr *> findUnsafePointers(const Decl *D);
} // end namespace clang
#endif /* LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H */
diff --git a/clang/include/clang/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h
index 825fffad49b4a..75371f4d95547 100644
--- a/clang/include/clang/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h
@@ -98,6 +98,8 @@ class UnsafeBufferUsageEntitySummary final : public EntitySummary {
bool operator==(const EntityPointerLevelSet &Other) const {
return UnsafeBuffers == Other;
}
+
+ bool empty() const { return UnsafeBuffers.empty(); }
};
} // namespace clang::ssaf
diff --git a/clang/lib/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp b/clang/lib/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp
index 440012a2b278b..74ab6d84ee949 100644
--- a/clang/lib/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp
+++ b/clang/lib/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageExtractor.cpp
@@ -33,6 +33,32 @@ static bool hasPointerType(const Expr *E) {
constexpr inline auto buildEntityPointerLevel =
UnsafeBufferUsageTUSummaryExtractor::buildEntityPointerLevel;
+static llvm::Error makeUnsupportedStmtKindError(const Stmt *Unsupported) {
+ return llvm::createStringError(
+ "unsupported expression kind for translation to "
+ "EntityPointerLevel: %s",
+ Unsupported->getStmtClassName());
+}
+
+static llvm::Error makeCreateEntityNameError(const NamedDecl *FailedDecl,
+ ASTContext &Ctx) {
+ std::string LocStr = FailedDecl->getSourceRange().getBegin().printToString(
+ Ctx.getSourceManager());
+ return llvm::createStringError(
+ "failed to create entity name for %s declared at %s",
+ FailedDecl->getNameAsString().c_str(), LocStr.c_str());
+}
+
+static llvm::Error makeAddEntitySummaryError(const NamedDecl *FailedContributor,
+ ASTContext &Ctx) {
+ std::string LocStr =
+ FailedContributor->getSourceRange().getBegin().printToString(
+ Ctx.getSourceManager());
+ return llvm::createStringError(
+ "failed to add entity summary for contributor %s declared at %s",
+ FailedContributor->getNameAsString().c_str(), LocStr.c_str());
+}
+
// Translate a pointer type expression 'E' to a (set of) EntityPointerLevel(s)
// associated with the declared type of the base address of `E`. If the base
// address of `E` is not associated with an entity, the translation result is an
@@ -59,10 +85,7 @@ class EntityPointerLevelTranslator
// Fallback method for all unsupported expression kind:
llvm::Error fallback(const Stmt *E) {
- return llvm::createStringError(
- "unsupported expression kind for translation to "
- "EntityPointerLevel: %s",
- E->getStmtClassName());
+ return makeUnsupportedStmtKindError(E);
}
static EntityPointerLevel incrementPointerLevel(const EntityPointerLevel &E) {
@@ -92,10 +115,12 @@ class EntityPointerLevelTranslator
}
UnsafeBufferUsageTUSummaryExtractor &Extractor;
+ ASTContext &Ctx;
public:
- EntityPointerLevelTranslator(UnsafeBufferUsageTUSummaryExtractor &Extractor)
- : Extractor(Extractor) {}
+ EntityPointerLevelTranslator(UnsafeBufferUsageTUSummaryExtractor &Extractor,
+ ASTContext &Ctx)
+ : Extractor(Extractor), Ctx(Ctx) {}
Expected<EntityPointerLevelSet> translate(const Expr *E) { return Visit(E); }
@@ -210,18 +235,14 @@ class EntityPointerLevelTranslator
Expected<EntityPointerLevelSet> VisitDeclRefExpr(const DeclRefExpr *E) {
if (auto EntityName = getEntityName(E->getDecl()))
return EntityPointerLevelSet{createEntityPointerLevelFor(*EntityName)};
- return llvm::createStringError(
- "failed to create entity name from the Decl of " +
- E->getNameInfo().getAsString());
+ return makeCreateEntityNameError(E->getDecl(), Ctx);
}
// Translate({., ->}f) -> {(MemberDecl, 1)}
Expected<EntityPointerLevelSet> VisitMemberExpr(const MemberExpr *E) {
if (auto EntityName = getEntityName(E->getMemberDecl()))
return EntityPointerLevelSet{createEntityPointerLevelFor(*EntityName)};
- return llvm::createStringError(
- "failed to create entity name from the MemberDecl of " +
- E->getMemberDecl()->getNameAsString());
+ return makeCreateEntityNameError(E->getMemberDecl(), Ctx);
}
Expected<EntityPointerLevelSet>
@@ -232,9 +253,10 @@ class EntityPointerLevelTranslator
Expected<EntityPointerLevelSet>
buildEntityPointerLevels(std::set<const Expr *> &&UnsafePointers,
- UnsafeBufferUsageTUSummaryExtractor &Extractor) {
+ UnsafeBufferUsageTUSummaryExtractor &Extractor,
+ ASTContext &Ctx) {
EntityPointerLevelSet Result{};
- EntityPointerLevelTranslator Translator{Extractor};
+ EntityPointerLevelTranslator Translator{Extractor, Ctx};
llvm::Error AllErrors = llvm::ErrorSuccess();
for (const Expr *Ptr : UnsafePointers) {
@@ -258,24 +280,90 @@ buildEntityPointerLevels(std::set<const Expr *> &&UnsafePointers,
}
} // namespace
+static std::set<const Expr *> findUnsafePointersInContributor(const Decl *D) {
+ if (isa<FunctionDecl>(D) || isa<VarDecl>(D))
+ return findUnsafePointers(D);
+ if (auto *RD = dyn_cast<RecordDecl>(D)) {
+ std::set<const Expr *> Result;
+
+ for (const FieldDecl *FD : RD->fields()) {
+ Result.merge(findUnsafePointers(FD));
+ }
+ return Result;
+ }
+ return {};
+}
+
std::unique_ptr<UnsafeBufferUsageEntitySummary>
UnsafeBufferUsageTUSummaryExtractor::extractEntitySummary(
const Decl *Contributor, ASTContext &Ctx, llvm::Error &Error) {
- // FIXME: findUnsafePointers should accept more kinds of `Decl`s than just
- // `FunctionDecl`:
- if (const auto *FD = dyn_cast<FunctionDecl>(Contributor)) {
- Expected<EntityPointerLevelSet> EPLs =
- buildEntityPointerLevels(findUnsafePointers(FD), *this);
-
- if (EPLs)
- return std::make_unique<UnsafeBufferUsageEntitySummary>(
- UnsafeBufferUsageEntitySummary(std::move(*EPLs)));
- Error = EPLs.takeError();
- }
+ Expected<EntityPointerLevelSet> EPLs = buildEntityPointerLevels(
+ findUnsafePointersInContributor(Contributor), *this, Ctx);
+
+ if (EPLs)
+ return std::make_unique<UnsafeBufferUsageEntitySummary>(
+ UnsafeBufferUsageEntitySummary(std::move(*EPLs)));
+ Error = EPLs.takeError();
return nullptr;
}
void UnsafeBufferUsageTUSummaryExtractor::HandleTranslationUnit(
ASTContext &Ctx) {
- // FIXME: impl me!
+
+ // FIXME: I suppose finding contributor Decls is commonly needed by all/many
+ // extractors
+ class ContributorFinder : public DynamicRecursiveASTVisitor {
+ public:
+ std::vector<const NamedDecl *> Contributors;
+
+ bool VisitFunctionDecl(FunctionDecl *D) override {
+ Contributors.push_back(D);
+ return true;
+ }
+
+ bool VisitRecordDecl(RecordDecl *D) override {
+ Contributors.push_back(D);
+ return true;
+ }
+
+ bool VisitVarDecl(VarDecl *D) override {
+ DeclContext *DC = D->getDeclContext();
+
+ if (DC->isFileContext() || DC->isNamespace())
+ Contributors.push_back(D);
+ return false;
+ }
+ } ContributorFinder;
+
+ ContributorFinder.VisitTranslationUnitDecl(Ctx.getTranslationUnitDecl());
+
+ llvm::Error Errors = llvm::ErrorSuccess();
+ auto addError = [&Errors](llvm::Error Err) {
+ Errors = llvm::joinErrors(std::move(Errors), std::move(Err));
+ };
+
+ for (auto *CD : ContributorFinder.Contributors) {
+ llvm::Error Error = llvm::ErrorSuccess();
+ auto EntitySummary = extractEntitySummary(CD, Ctx, Error);
+
+ if (Error)
+ addError(std::move(Error));
+ if (EntitySummary->empty())
+ continue;
+
+ auto ContributorName = getEntityName(CD);
+
+ if (!ContributorName) {
+ addError(makeCreateEntityNameError(CD, Ctx));
+ continue;
+ }
+
+ auto [EntitySummaryPtr, Success] = SummaryBuilder.addSummary(
+ addEntity(*ContributorName), std::move(EntitySummary));
+
+ if (!Success)
+ addError(makeAddEntitySummaryError(CD, Ctx));
+ }
+ // FIXME: handle errors!
+ llvm::consumeError(std::move(Errors));
}
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 193851cc5f381..84f117eb1c26b 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -28,6 +28,7 @@
#include "clang/Lex/Preprocessor.h"
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/APSInt.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -36,6 +37,7 @@
#include <queue>
#include <set>
#include <sstream>
+#include <vector>
using namespace clang;
@@ -2945,7 +2947,37 @@ template <typename NodeTy> struct CompareNode {
}
};
-std::set<const Expr *> clang::findUnsafePointers(const FunctionDecl *FD) {
+// Populate `Stmts` with the body/initializer Stmt of `D`, if `D` is one of the
+// followings:
+// VarDecl
+// FieldDecl
+// FunctionDecl
+// BlockDecl
+// ObjCMethodDecl
+static void populateStmtsForFindingGadgets(SmallVector<const Stmt *> &Stmts,
+ const Decl *D) {
+ auto AddStmt = [&Stmts](const Stmt *S) {
+ if (S)
+ Stmts.push_back(S);
+ };
+ if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+ AddStmt(FD->getBody());
+ for (const auto *PD : FD->parameters())
+ AddStmt(PD->getDefaultArg());
+ if (const auto *CtorD = dyn_cast<CXXConstructorDecl>(FD))
+ llvm::append_range(
+ Stmts, llvm::map_range(CtorD->inits(),
+ std::mem_fn(&CXXCtorInitializer::getInit)));
+ } else if (isa<BlockDecl>(D) || isa<ObjCMethodDecl>(D)) {
+ AddStmt(D->getBody());
+ } else if (const auto *VD = dyn_cast<VarDecl>(D)) {
+ AddStmt(VD->getInit()); // FIXME: default arg for ParmVarDecl?
+ } else if (const auto *FD = dyn_cast<FieldDecl>(D)) {
+ AddStmt(FD->getInClassInitializer());
+ }
+}
+
+std::set<const Expr *> clang::findUnsafePointers(const Decl *D) {
class MockReporter : public UnsafeBufferUsageHandler {
public:
MockReporter() {}
@@ -2984,9 +3016,13 @@ std::set<const Expr *> clang::findUnsafePointers(const FunctionDecl *FD) {
WarningGadgetList WarningGadgets;
DeclUseTracker Tracker;
MockReporter IgnoreHandler;
+ ASTContext &Ctx = D->getASTContext();
+ SmallVector<const Stmt *> Stmts;
- findGadgets(FD->getBody(), FD->getASTContext(), IgnoreHandler, false,
- FixableGadgets, WarningGadgets, Tracker);
+ populateStmtsForFindingGadgets(Stmts, D);
+ for (auto *Stmt : Stmts)
+ findGadgets(Stmt, Ctx, IgnoreHandler, false, FixableGadgets, WarningGadgets,
+ Tracker);
std::set<const Expr *> Result;
for (auto &G : WarningGadgets) {
@@ -4676,9 +4712,6 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
#endif
assert(D);
-
- SmallVector<Stmt *> Stmts;
-
if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
// Consteval functions are free of UB by the spec, so we don't need to
// visit them or produce diagnostics.
@@ -4700,27 +4733,21 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
break;
}
}
+ }
- Stmts.push_back(FD->getBody());
+ SmallVector<const Stmt *> Stmts;
- if (const auto *ID = dyn_cast<CXXConstructorDecl>(D)) {
- for (const CXXCtorInitializer *CI : ID->inits()) {
- Stmts.push_back(CI->getInit());
- }
- }
- } else if (isa<BlockDecl>(D) || isa<ObjCMethodDecl>(D)) {
- Stmts.push_back(D->getBody());
- }
+ populateStmtsForFindingGadgets(Stmts, D);
assert(!Stmts.empty());
FixableGadgetList FixableGadgets;
WarningGadgetList WarningGadgets;
DeclUseTracker Tracker;
- for (Stmt *S : Stmts) {
+ for (const Stmt *S : Stmts) {
findGadgets(S, D->getASTContext(), Handler, EmitSuggestions, FixableGadgets,
WarningGadgets, Tracker);
}
applyGadgets(D, std::move(FixableGadgets), std::move(WarningGadgets),
std::move(Tracker), Handler, EmitSuggestions);
-}
+}
\ No newline at end of file
diff --git a/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp
index 8d65ce51b2926..f363adb9c298d 100644
--- a/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp
+++ b/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp
@@ -34,7 +34,7 @@ const SomeDecl *findDeclByName(StringRef Name, ASTContext &Ctx) {
NamedDeclFinder(StringRef SearchingName) : SearchingName(SearchingName) {}
bool VisitDecl(Decl *D) override {
- if (const auto *ND = dyn_cast<NamedDecl>(D)) {
+ if (const auto *ND = dyn_cast<SomeDecl>(D)) {
if (ND->getNameAsString() == SearchingName) {
FoundDecl = ND;
return false;
@@ -69,16 +69,21 @@ class UnsafeBufferUsageTest : public testing::Test {
BuildNamespace(BuildNamespaceKind::CompilationUnit, "Mock.cpp")),
TUSummaryBuilder(TUSummary), Extractor(TUSummaryBuilder) {}
+ template <typename ContributorDecl = NamedDecl>
std::unique_ptr<UnsafeBufferUsageEntitySummary>
setUpTest(StringRef Code, StringRef ContributorName) {
AST = tooling::buildASTFromCodeWithArgs(
- Code, {"-Wno-unused-value -Wno-int-to-pointer-cast"});
+ Code, {"-Wno-unused-value", "-Wno-int-to-pointer-cast"});
const auto *ContributorDefn =
- findDeclByName(ContributorName, AST->getASTContext());
+ findDeclByName<ContributorDecl>(ContributorName, AST->getASTContext());
+
+ if (!ContributorDefn)
+ return nullptr;
+
std::optional<EntityName> EN = getEntityName(ContributorDefn);
- if (!ContributorDefn || !EN)
+ if (!EN)
return nullptr;
llvm::Error Error = llvm::ErrorSuccess();
@@ -442,4 +447,53 @@ TEST_F(UnsafeBufferUsageTest, PointerToArrayOfPointers) {
EXPECT_NE(Sum, nullptr);
EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 3}}));
}
+
+TEST_F(UnsafeBufferUsageTest, UnsafePointerInGlobalVariableInitializer) {
+ auto Sum = setUpTest(R"cpp(
+ int *gp;
+ int x = gp[5];
+ )cpp",
+ {"x"});
+
+ EXPECT_NE(Sum, nullptr);
+ EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}}));
+}
+
+TEST_F(UnsafeBufferUsageTest, UnsafePointerInFieldInitializer) {
+ auto Sum = setUpTest<>(R"cpp(
+ int *gp;
+ struct Foo {
+ int field = gp[5];
+ };
+ )cpp",
+ {"Foo"});
+
+ EXPECT_NE(Sum, nullptr);
+ EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}}));
+}
+
+TEST_F(UnsafeBufferUsageTest, UnsafePointerInCXXCtorInitializer) {
+ auto Sum = setUpTest<CXXConstructorDecl>(R"cpp(
+ struct Foo {
+ int member;
+ Foo(int *p) : member(p[5]) {}
+ };
+ )cpp",
+ {"Foo"});
+
+ EXPECT_NE(Sum, nullptr);
+ EXPECT_EQ(*Sum, makeSet(__LINE__, {{"p", 1U}}));
+}
+
+TEST_F(UnsafeBufferUsageTest, UnsafePointerInDefaultArg) {
+ auto Sum = setUpTest(R"cpp(
+ int * gp;
+ void foo(int x = gp[5]);
+ )cpp",
+ {"foo"});
+
+ EXPECT_NE(Sum, nullptr);
+ EXPECT_EQ(*Sum, makeSet(__LINE__, {{"gp", 1U}}));
+}
+
} // namespace
>From c67d6022095c4087e54b70ea527fb9b9542733af Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Thu, 5 Mar 2026 14:09:34 -0800
Subject: [PATCH 2/2] clean up
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +-
.../Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 84f117eb1c26b..8a8e73651d98f 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -4750,4 +4750,4 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
}
applyGadgets(D, std::move(FixableGadgets), std::move(WarningGadgets),
std::move(Tracker), Handler, EmitSuggestions);
-}
\ No newline at end of file
+}
diff --git a/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp b/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp
index f363adb9c298d..381172ddffa61 100644
--- a/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp
+++ b/clang/unittests/Analysis/Scalable/Analyses/UnsafeBufferUsage/UnsafeBufferUsageTest.cpp
@@ -460,7 +460,7 @@ TEST_F(UnsafeBufferUsageTest, UnsafePointerInGlobalVariableInitializer) {
}
TEST_F(UnsafeBufferUsageTest, UnsafePointerInFieldInitializer) {
- auto Sum = setUpTest<>(R"cpp(
+ auto Sum = setUpTest(R"cpp(
int *gp;
struct Foo {
int field = gp[5];
More information about the llvm-branch-commits
mailing list