[llvm-branch-commits] WIP smartpointers (PR #164016)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Oct 17 13:48:09 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Florian Mayer (fmayer)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/164016.diff
6 Files Affected:
- (modified) clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h (+3-1)
- (modified) clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h (+16)
- (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (+65)
- (modified) clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp (+19-8)
- (modified) clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp (+48)
- (modified) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (+18)
``````````diff
diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h
index cb619fb0cb5bb..3b407cc4f20b2 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h
@@ -13,6 +13,7 @@
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
@@ -70,7 +71,8 @@ struct UncheckedStatusOrAccessModelOptions {};
// Dataflow analysis that discovers unsafe uses of StatusOr values.
class UncheckedStatusOrAccessModel
- : public DataflowAnalysis<UncheckedStatusOrAccessModel, NoopLattice> {
+ : public DataflowAnalysis<UncheckedStatusOrAccessModel,
+ CachedConstAccessorsLattice<NoopLattice>> {
public:
explicit UncheckedStatusOrAccessModel(ASTContext &Ctx, Environment &Env);
diff --git a/clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h b/clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h
index e55b83aa845d4..1f188e3eaa194 100644
--- a/clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h
+++ b/clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h
@@ -58,6 +58,7 @@ namespace clang::dataflow {
/// for `std::optional`, we assume the (Matcher, TransferFunction) case
/// with custom handling is ordered early so that these generic cases
/// do not trigger.
+ast_matchers::StatementMatcher isSmartPointerLikeContructor();
ast_matchers::StatementMatcher isPointerLikeOperatorStar();
ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar();
ast_matchers::StatementMatcher isPointerLikeOperatorArrow();
@@ -80,6 +81,8 @@ isSmartPointerLikeGetMethodCall(clang::StringRef MethodName = "get");
const FunctionDecl *
getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE);
+const FunctionDecl *
+getCanonicalSmartPointerLikeOperatorCallee2(const CXXRecordDecl *RD);
/// A transfer function for `operator*` (and `value`) calls that can be
/// cached. Runs the `InitializeLoc` callback to initialize any new
/// StorageLocations.
@@ -141,6 +144,19 @@ void transferSmartPointerLikeCachedDeref(
State.Env.setStorageLocation(*DerefExpr, LocForValue);
}
+template <typename LatticeT>
+void transferSmartPointerLikeConstructor(
+ const CXXConstructExpr *ConstructOperator,
+ RecordStorageLocation *SmartPointerLoc, TransferState<LatticeT> &State,
+ llvm::function_ref<void(StorageLocation &)> InitializeLoc) {
+ abort();
+ const FunctionDecl *CanonicalCallee =
+ getCanonicalSmartPointerLikeOperatorCallee2(
+ ConstructOperator->getType()->getAsCXXRecordDecl());
+ State.Lattice.getOrCreateConstMethodReturnStorageLocation(
+ *SmartPointerLoc, CanonicalCallee, State.Env, InitializeLoc);
+}
+
template <typename LatticeT>
void transferSmartPointerLikeCachedGet(
const CallExpr *GetExpr, RecordStorageLocation *SmartPointerLoc,
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
index 7a698f276d6c1..8a3670ae6ba5b 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
@@ -25,6 +25,7 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
#include "clang/Analysis/FlowSensitive/RecordOps.h"
+#include "clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h"
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Basic/LLVM.h"
@@ -608,6 +609,29 @@ static void transferStatusConstructor(const CXXConstructExpr *Expr,
initializeStatus(StatusLoc, State.Env);
}
+static RecordStorageLocation *
+getSmartPtrLikeStorageLocation(const Expr &E, const Environment &Env) {
+ if (!E.isPRValue())
+ return dyn_cast_or_null<RecordStorageLocation>(Env.getStorageLocation(E));
+ if (auto *PointerVal = dyn_cast_or_null<PointerValue>(Env.getValue(E)))
+ return dyn_cast_or_null<RecordStorageLocation>(
+ &PointerVal->getPointeeLoc());
+ return nullptr;
+}
+
+static void maybeInitLocForExpr(const Expr *E, StorageLocation &Loc,
+ Environment &Env) {
+ auto T = E->getType();
+ if (!T->isPointerOrReferenceType())
+ return;
+ T = T->getPointeeType();
+ if (isStatusOrType(T)) {
+ initializeStatusOr(cast<RecordStorageLocation>(Loc), Env);
+ } else if (isStatusType(T)) {
+ initializeStatus(cast<RecordStorageLocation>(Loc), Env);
+ }
+}
+
CFGMatchSwitch<LatticeTransferState>
buildTransferMatchSwitch(ASTContext &Ctx,
CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -667,6 +691,47 @@ buildTransferMatchSwitch(ASTContext &Ctx,
transferStatusOrConstructor)
.CaseOfCFGStmt<CXXConstructExpr>(isStatusConstructor(),
transferStatusConstructor)
+ .CaseOfCFGStmt<CXXOperatorCallExpr>(
+ isPointerLikeOperatorArrow(),
+ [](const CXXOperatorCallExpr *E,
+ const MatchFinder::MatchResult &Result,
+ LatticeTransferState &State) {
+ transferSmartPointerLikeCachedGet(
+ E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env),
+ State, [&](StorageLocation &Loc) {
+ maybeInitLocForExpr(E, Loc, State.Env);
+ });
+ })
+ .CaseOfCFGStmt<CXXConstructExpr>(
+ isSmartPointerLikeContructor(),
+ [](const CXXConstructExpr *E, const MatchFinder::MatchResult &Result,
+ LatticeTransferState &State) {
+ transferSmartPointerLikeConstructor(
+ E, &State.Env.getResultObjectLocation(*E), State,
+ [&](StorageLocation &Loc) {
+ maybeInitLocForExpr(E, Loc, State.Env);
+ });
+ })
+ .CaseOfCFGStmt<CXXMemberCallExpr>(
+ isSmartPointerLikeValueMethodCall(),
+ [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
+ LatticeTransferState &State) {
+ transferSmartPointerLikeCachedDeref(
+ E, getImplicitObjectLocation(*E, State.Env), State,
+ [&](StorageLocation &Loc) {
+ maybeInitLocForExpr(E, Loc, State.Env);
+ });
+ })
+ .CaseOfCFGStmt<CXXMemberCallExpr>(
+ isSmartPointerLikeGetMethodCall(),
+ [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
+ LatticeTransferState &State) {
+ transferSmartPointerLikeCachedGet(
+ E, getImplicitObjectLocation(*E, State.Env), State,
+ [&](StorageLocation &Loc) {
+ maybeInitLocForExpr(E, Loc, State.Env);
+ });
+ })
.Build();
}
diff --git a/clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp b/clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp
index d87b2e6f03857..45bfbb07932dd 100644
--- a/clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp
+++ b/clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp
@@ -129,6 +129,12 @@ AST_MATCHER(clang::CXXRecordDecl, pointerClass) {
namespace clang::dataflow {
+ast_matchers::StatementMatcher isSmartPointerLikeContructor() {
+ using namespace ast_matchers;
+ return cxxConstructExpr(hasType(hasCanonicalType(qualType(
+ hasDeclaration(cxxRecordDecl(smartPointerClassWithGetOrValue()))))));
+}
+
ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar() {
return cxxOperatorCallExpr(
hasOverloadedOperatorName("*"),
@@ -177,15 +183,8 @@ isSmartPointerLikeGetMethodCall(clang::StringRef MethodName) {
}
const FunctionDecl *
-getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE) {
+getCanonicalSmartPointerLikeOperatorCallee2(const CXXRecordDecl *RD) {
const FunctionDecl *CanonicalCallee = nullptr;
- const CXXMethodDecl *Callee =
- cast_or_null<CXXMethodDecl>(CE->getDirectCallee());
- if (Callee == nullptr)
- return nullptr;
- const CXXRecordDecl *RD = Callee->getParent();
- if (RD == nullptr)
- return nullptr;
for (const auto *MD : RD->methods()) {
if (MD->getOverloadedOperator() == OO_Star && MD->isConst() &&
MD->getNumParams() == 0 && MD->getReturnType()->isReferenceType()) {
@@ -196,4 +195,16 @@ getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE) {
return CanonicalCallee;
}
+const FunctionDecl *
+getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE) {
+ const CXXMethodDecl *Callee =
+ cast_or_null<CXXMethodDecl>(CE->getDirectCallee());
+ if (Callee == nullptr)
+ return nullptr;
+ const CXXRecordDecl *RD = Callee->getParent();
+ if (RD == nullptr)
+ return nullptr;
+ return getCanonicalSmartPointerLikeOperatorCallee2(RD);
+}
+
} // namespace clang::dataflow
diff --git a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp
index 2e528edd7c1f9..779d277ccaa6d 100644
--- a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp
@@ -1809,6 +1809,53 @@ template <class T1, class T2> pair<T1, T2> make_pair(T1 &&t1, T2 &&t2);
#endif // STD_PAIR_H
)cc";
+static constexpr const char StdUniquePtrHeader[] = R"cc(
+namespace std {
+ template <typename T>
+ struct default_delete {};
+
+ template <typename T, typename D = default_delete<T>>
+ class unique_ptr {
+ public:
+ using element_type = T;
+ using deleter_type = D;
+
+ constexpr unique_ptr();
+ constexpr unique_ptr(nullptr_t) noexcept;
+ unique_ptr(unique_ptr&&);
+ explicit unique_ptr(T*);
+ template <typename U, typename E>
+ unique_ptr(unique_ptr<U, E>&&);
+
+ ~unique_ptr();
+
+ unique_ptr& operator=(unique_ptr&&);
+ template <typename U, typename E>
+ unique_ptr& operator=(unique_ptr<U, E>&&);
+ unique_ptr& operator=(nullptr_t);
+
+ void reset(T* = nullptr) noexcept;
+ T* release();
+ T* get() const;
+
+ T& operator*() const;
+ T* operator->() const;
+ explicit operator bool() const noexcept;
+ };
+
+ template <typename T, typename D>
+ class unique_ptr<T[], D> {
+ public:
+ T* get() const;
+ T& operator[](size_t i);
+ const T& operator[](size_t i) const;
+ };
+
+ template <typename T, typename... Args>
+ unique_ptr<T> make_unique(Args&&...);
+}
+)cc";
+
constexpr const char AbslLogHeader[] = R"cc(
#ifndef ABSL_LOG_H
#define ABSL_LOG_H
@@ -2245,6 +2292,7 @@ std::vector<std::pair<std::string, std::string>> getMockHeaders() {
Headers.emplace_back("base_optional.h", BaseOptionalHeader);
Headers.emplace_back("std_vector.h", StdVectorHeader);
Headers.emplace_back("std_pair.h", StdPairHeader);
+ Headers.emplace_back("std_unique_ptr.h", StdUniquePtrHeader);
Headers.emplace_back("status_defs.h", StatusDefsHeader);
Headers.emplace_back("statusor_defs.h", StatusOrDefsHeader);
Headers.emplace_back("absl_log.h", AbslLogHeader);
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
index 67c37e1e0f77f..70cd6751e4024 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
@@ -3174,6 +3174,23 @@ TEST_P(UncheckedStatusOrAccessModelTest, ConstructStatusFromReference) {
)cc");
}
+TEST_P(UncheckedStatusOrAccessModelTest, SmartPtr) {
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+ void target() {
+ const auto sor1 = Make<std::unique_ptr<STATUSOR_INT>>();
+ const auto sor2 = Make<std::unique_ptr<STATUSOR_INT>>();
+ if (!sor1->ok() && !sor2->ok()) return;
+ if (sor1->ok() && !sor2->ok()) {
+ } else if (!sor1->ok() && sor2->ok()) {
+ } else {
+ sor1->value();
+ sor2->value();
+ }
+ }
+ )cc");
+}
+
} // namespace
std::string
@@ -3221,6 +3238,7 @@ GetHeaders(UncheckedStatusOrAccessModelTestAliasKind AliasKind) {
#include "std_optional.h"
#include "std_vector.h"
#include "std_pair.h"
+#include "std_unique_ptr.h"
#include "absl_log.h"
#include "testing_defs.h"
``````````
</details>
https://github.com/llvm/llvm-project/pull/164016
More information about the llvm-branch-commits
mailing list