From 2b8f7840e3f075f89db406298bc466d2658c0a3e Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Fri, 5 Dec 2025 15:17:04 -0800 Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20to=20main=20this=20commit=20is=20based=20on?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.7 [skip ci] --- .../Models/UncheckedStatusOrAccessModel.h | 4 +- .../Models/UncheckedStatusOrAccessModel.cpp | 209 +++++++++++++++ .../Analysis/FlowSensitive/MockHeaders.cpp | 90 +++++++ ...ncheckedStatusOrAccessModelTestFixture.cpp | 247 ++++++++++++++++++ 4 files changed, 549 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.h index 24f8b0b99870a..2d54bd3c6f7ad 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" @@ -69,7 +70,8 @@ struct UncheckedStatusOrAccessModelOptions {}; // Dataflow analysis that discovers unsafe uses of StatusOr values. class UncheckedStatusOrAccessModel - : public DataflowAnalysis { + : public DataflowAnalysis> { public: explicit UncheckedStatusOrAccessModel(ASTContext &Ctx, Environment &Env); diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 5869aa9a3e227..3e56094fcbc32 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" @@ -237,6 +238,49 @@ static auto isAsStatusCallWithStatusOr() { hasArgument(0, hasType(statusOrType()))); } +static auto possiblyReferencedStatusOrType() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return anyOf(statusOrType(), referenceType(pointee(statusOrType()))); +} + +static auto isConstStatusOrAccessorMemberCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxMemberCallExpr(callee( + cxxMethodDecl(parameterCountIs(0), isConst(), + returns(qualType(possiblyReferencedStatusOrType()))))); +} + +static auto isConstStatusOrAccessorMemberOperatorCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr( + callee(cxxMethodDecl(parameterCountIs(0), isConst(), + returns(possiblyReferencedStatusOrType())))); +} + +static auto isConstStatusOrPointerAccessorMemberCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxMemberCallExpr(callee(cxxMethodDecl( + parameterCountIs(0), isConst(), + returns(pointerType(pointee(possiblyReferencedStatusOrType())))))); +} + +static auto isConstStatusOrPointerAccessorMemberOperatorCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr(callee(cxxMethodDecl( + parameterCountIs(0), isConst(), + returns(pointerType(pointee(possiblyReferencedStatusOrType())))))); +} + +static auto isNonConstMemberCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxMemberCallExpr(callee(cxxMethodDecl(unless(isConst())))); +} + +static auto isNonConstMemberOperatorCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr(callee(cxxMethodDecl(unless(isConst())))); +} + static auto buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { return CFGMatchSwitchBuilder(State.Env.getValue(*Expr->getSubExpr()))) State.Env.setValue(*Expr, *SubExprVal); } +static void handleConstStatusOrAccessorMemberCall( + const CallExpr *Expr, RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, LatticeTransferState &State) { + assert(isStatusOrType(Expr->getType())); + if (RecordLoc == nullptr) + return; + const FunctionDecl *DirectCallee = Expr->getDirectCallee(); + if (DirectCallee == nullptr) + return; + StorageLocation &Loc = + State.Lattice.getOrCreateConstMethodReturnStorageLocation( + *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) { + initializeStatusOr(cast(Loc), State.Env); + }); + if (Expr->isPRValue()) { + auto &ResultLoc = State.Env.getResultObjectLocation(*Expr); + copyRecord(cast(Loc), ResultLoc, State.Env); + } else { + State.Env.setStorageLocation(*Expr, Loc); + } +} + +static void handleConstStatusOrPointerAccessorMemberCall( + const CallExpr *Expr, RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, LatticeTransferState &State) { + if (RecordLoc == nullptr) + return; + auto *Val = State.Lattice.getOrCreateConstMethodReturnValue(*RecordLoc, Expr, + State.Env); + State.Env.setValue(*Expr, *Val); +} + +static void +transferConstStatusOrAccessorMemberCall(const CXXMemberCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + handleConstStatusOrAccessorMemberCall( + Expr, getImplicitObjectLocation(*Expr, State.Env), Result, State); +} + +static void transferConstStatusOrAccessorMemberOperatorCall( + const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto *RecordLoc = cast_or_null( + State.Env.getStorageLocation(*Expr->getArg(0))); + handleConstStatusOrAccessorMemberCall(Expr, RecordLoc, Result, State); +} + +static void transferConstStatusOrPointerAccessorMemberCall( + const CXXMemberCallExpr *Expr, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + handleConstStatusOrPointerAccessorMemberCall( + Expr, getImplicitObjectLocation(*Expr, State.Env), Result, State); +} + +static void transferConstStatusOrPointerAccessorMemberOperatorCall( + const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto *RecordLoc = cast_or_null( + State.Env.getStorageLocation(*Expr->getArg(0))); + handleConstStatusOrPointerAccessorMemberCall(Expr, RecordLoc, Result, State); +} + +static void transferStatusOrReturningCall(const CallExpr *Expr, + LatticeTransferState &State) { + RecordStorageLocation *StatusOrLoc = + Expr->isPRValue() ? &State.Env.getResultObjectLocation(*Expr) + : State.Env.get(*Expr); + if (StatusOrLoc != nullptr && + State.Env.getValue(locForOk(locForStatus(*StatusOrLoc))) == nullptr) + initializeStatusOr(*StatusOrLoc, State.Env); +} + +static void handleNonConstMemberCall(const CallExpr *Expr, + RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + if (RecordLoc == nullptr) + return; + State.Lattice.clearConstMethodReturnValues(*RecordLoc); + State.Lattice.clearConstMethodReturnStorageLocations(*RecordLoc); + + if (isStatusOrType(Expr->getType())) + transferStatusOrReturningCall(Expr, State); +} + +static void transferNonConstMemberCall(const CXXMemberCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + handleNonConstMemberCall(Expr, getImplicitObjectLocation(*Expr, State.Env), + Result, State); +} + +static void +transferNonConstMemberOperatorCall(const CXXOperatorCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto *RecordLoc = cast_or_null( + State.Env.getStorageLocation(*Expr->getArg(0))); + handleNonConstMemberCall(Expr, RecordLoc, Result, State); +} + +static RecordStorageLocation * +getSmartPtrLikeStorageLocation(const Expr &E, const Environment &Env) { + if (!E.isPRValue()) + return dyn_cast_or_null(Env.getStorageLocation(E)); + if (auto *PointerVal = dyn_cast_or_null(Env.getValue(E))) + return dyn_cast_or_null( + &PointerVal->getPointeeLoc()); + return nullptr; +} CFGMatchSwitch buildTransferMatchSwitch(ASTContext &Ctx, @@ -755,6 +910,60 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferLoggingGetReferenceableValueCall) .CaseOfCFGStmt(isLoggingCheckEqImpl(), transferLoggingCheckEqImpl) + // This needs to go before the const accessor call matcher, because these + // look like them, but we model `operator`* and `get` to return the same + // object. Also, we model them for non-const cases. + .CaseOfCFGStmt( + isPointerLikeOperatorStar(), + [](const CXXOperatorCallExpr *E, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedDeref( + E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env), + State, [](StorageLocation &Loc) {}); + }) + .CaseOfCFGStmt( + isPointerLikeOperatorArrow(), + [](const CXXOperatorCallExpr *E, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedGet( + E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env), + State, [](StorageLocation &Loc) {}); + }) + .CaseOfCFGStmt( + isSmartPointerLikeValueMethodCall(), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedDeref( + E, getImplicitObjectLocation(*E, State.Env), State, + [](StorageLocation &Loc) {}); + }) + .CaseOfCFGStmt( + isSmartPointerLikeGetMethodCall(), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedGet( + E, getImplicitObjectLocation(*E, State.Env), State, + [](StorageLocation &Loc) {}); + }) + // const accessor calls + .CaseOfCFGStmt(isConstStatusOrAccessorMemberCall(), + transferConstStatusOrAccessorMemberCall) + .CaseOfCFGStmt( + isConstStatusOrAccessorMemberOperatorCall(), + transferConstStatusOrAccessorMemberOperatorCall) + .CaseOfCFGStmt( + isConstStatusOrPointerAccessorMemberCall(), + transferConstStatusOrPointerAccessorMemberCall) + .CaseOfCFGStmt( + isConstStatusOrPointerAccessorMemberOperatorCall(), + transferConstStatusOrPointerAccessorMemberOperatorCall) + // non-const member calls that may modify the state of an object. + .CaseOfCFGStmt(isNonConstMemberCall(), + transferNonConstMemberCall) + .CaseOfCFGStmt(isNonConstMemberOperatorCall(), + transferNonConstMemberOperatorCall) // N.B. These need to come after all other CXXConstructExpr. // These are there to make sure that every Status and StatusOr object // have their ok boolean initialized when constructed. If we were to diff --git a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp index 2e528edd7c1f9..76c7310e16a4f 100644 --- a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp +++ b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp @@ -2232,6 +2232,95 @@ using testing::AssertionResult; #endif // TESTING_DEFS_H )cc"; +constexpr const char StdUniquePtrHeader[] = R"cc( +namespace std { + + template + struct default_delete {}; + + template > + 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 + unique_ptr(unique_ptr&&); + + ~unique_ptr(); + + unique_ptr& operator=(unique_ptr&&); + template + unique_ptr& operator=(unique_ptr&&); + 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 + class unique_ptr { + public: + T* get() const; + T& operator[](size_t i); + const T& operator[](size_t i) const; + }; + + template + unique_ptr make_unique(Args&&...); + + template + void swap(unique_ptr& x, unique_ptr& y) noexcept; + + template + bool operator==(const unique_ptr& x, const unique_ptr& y); + template + bool operator!=(const unique_ptr& x, const unique_ptr& y); + template + bool operator<(const unique_ptr& x, const unique_ptr& y); + template + bool operator<=(const unique_ptr& x, const unique_ptr& y); + template + bool operator>(const unique_ptr& x, const unique_ptr& y); + template + bool operator>=(const unique_ptr& x, const unique_ptr& y); + + template + bool operator==(const unique_ptr& x, nullptr_t) noexcept; + template + bool operator==(nullptr_t, const unique_ptr& y) noexcept; + template + bool operator!=(const unique_ptr& x, nullptr_t) noexcept; + template + bool operator!=(nullptr_t, const unique_ptr& y) noexcept; + template + bool operator<(const unique_ptr& x, nullptr_t); + template + bool operator<(nullptr_t, const unique_ptr& y); + template + bool operator<=(const unique_ptr& x, nullptr_t); + template + bool operator<=(nullptr_t, const unique_ptr& y); + template + bool operator>(const unique_ptr& x, nullptr_t); + template + bool operator>(nullptr_t, const unique_ptr& y); + template + bool operator>=(const unique_ptr& x, nullptr_t); + template + bool operator>=(nullptr_t, const unique_ptr& y); +} +)cc"; + std::vector> getMockHeaders() { std::vector> Headers; Headers.emplace_back("cstddef.h", CStdDefHeader); @@ -2249,6 +2338,7 @@ std::vector> getMockHeaders() { Headers.emplace_back("statusor_defs.h", StatusOrDefsHeader); Headers.emplace_back("absl_log.h", AbslLogHeader); Headers.emplace_back("testing_defs.h", TestingDefsHeader); + Headers.emplace_back("std_unique_ptr.h", StdUniquePtrHeader); return Headers; } diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 13cde05df0a3f..dcb1cc13146bd 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3270,6 +3270,252 @@ TEST_P(UncheckedStatusOrAccessModelTest, ConstructStatusFromReference) { )cc"); } +TEST_P(UncheckedStatusOrAccessModelTest, AccessorCall) { + // Accessor returns reference. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + const STATUSOR_INT& sor() const { return sor_; } + }; + + void target(Foo foo) { + if (foo.sor().ok()) foo.sor().value(); + } + )cc"); + + // Uses an operator + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + const STATUSOR_INT& operator()() const { return sor_; } + }; + + void target(Foo foo) { + if (foo().ok()) foo().value(); + } + )cc"); + + // Calls nonconst method inbetween. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + void invalidate() {} + + const STATUSOR_INT& sor() const { return sor_; } + }; + + void target(Foo foo) { + if (foo.sor().ok()) { + foo.invalidate(); + foo.sor().value(); // [[unsafe]] + } + } + )cc"); + + // Calls nonconst operator inbetween. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + void operator()() {} + + const STATUSOR_INT& sor() const { return sor_; } + }; + + void target(Foo foo) { + if (foo.sor().ok()) { + foo(); + foo.sor().value(); // [[unsafe]] + } + } + )cc"); + + // Accessor returns copy. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + STATUSOR_INT sor() const { return sor_; } + }; + + void target(Foo foo) { + if (foo.sor().ok()) foo.sor().value(); + } + )cc"); + + // Non-const accessor. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + const STATUSOR_INT& sor() { return sor_; } + }; + + void target(Foo foo) { + if (foo.sor().ok()) foo.sor().value(); // [[unsafe]] + } + )cc"); + + // Non-const rvalue accessor. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + STATUSOR_INT&& sor() { return std::move(sor_); } + }; + + void target(Foo foo) { + if (foo.sor().ok()) foo.sor().value(); // [[unsafe]] + } + )cc"); + + // const pointer accessor. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + const STATUSOR_INT* sor() const { return &sor_; } + }; + + void target(Foo foo) { + if (foo.sor()->ok()) foo.sor()->value(); + } + )cc"); + + // const pointer operator. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + const STATUSOR_INT* operator->() const { return &sor_; } + }; + + void target(Foo foo) { + if (foo->ok()) foo->value(); + } + )cc"); + + // We copy the result of the accessor. + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + STATUSOR_INT sor_; + + const STATUSOR_INT& sor() const { return sor_; } + }; + void target() { + Foo foo; + if (!foo.sor().ok()) return; + const auto sor = foo.sor(); + sor.value(); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, PointerLike) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + class Foo { + public: + std::pair& operator*() const; + std::pair* operator->() const; + bool operator!=(const Foo& other) const; + }; + + void target() { + Foo foo; + if (foo->second.ok() && *foo->second != nullptr) { + *foo->second; + (*foo).second.value(); + } + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + class Foo { + public: + std::pair& operator*() const; + std::pair* operator->() const; + }; + void target() { + Foo foo; + if (!foo->second.ok()) return; + foo->second.value(); + (*foo).second.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(std::pair* foo) { + if (foo->second.ok() && *foo->second != nullptr) { + *foo->second; + (*foo).second.value(); + } + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, UniquePtr) { + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + auto sor_up = Make>(); + if (sor_up->ok()) sor_up->value(); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, UniquePtrReset) { + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + auto sor_up = Make>(); + if (sor_up->ok()) { + sor_up.reset(Make()); + sor_up->value(); // [[unsafe]] + } + } + )cc"); +} + } // namespace std::string @@ -3319,6 +3565,7 @@ GetHeaders(UncheckedStatusOrAccessModelTestAliasKind AliasKind) { #include "std_pair.h" #include "absl_log.h" #include "testing_defs.h" +#include "std_unique_ptr.h" template T Make(); From 4997102854574cab605e91645a92dbb4ddb10257 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Mon, 8 Dec 2025 11:24:31 -0800 Subject: [PATCH 2/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20introduced=20through=20rebase?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.7 [skip ci] --- .../Models/UncheckedStatusOrAccessModel.cpp | 34 ++++++++++++------- ...ncheckedStatusOrAccessModelTestFixture.cpp | 4 +-- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 3e56094fcbc32..364d5c1009a36 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -741,15 +741,26 @@ static void transferPointerToBoolean(const ImplicitCastExpr *Expr, dyn_cast_or_null(State.Env.getValue(*Expr->getSubExpr()))) State.Env.setValue(*Expr, *SubExprVal); } -static void handleConstStatusOrAccessorMemberCall( + +static void transferStatusOrReturningCall(const CallExpr *Expr, + LatticeTransferState &State) { + RecordStorageLocation *StatusOrLoc = + Expr->isPRValue() ? &State.Env.getResultObjectLocation(*Expr) + : State.Env.get(*Expr); + if (StatusOrLoc != nullptr && + State.Env.getValue(locForOk(locForStatus(*StatusOrLoc))) == nullptr) + initializeStatusOr(*StatusOrLoc, State.Env); +} + +static bool doHandleConstStatusOrAccessorMemberCall( const CallExpr *Expr, RecordStorageLocation *RecordLoc, const MatchFinder::MatchResult &Result, LatticeTransferState &State) { assert(isStatusOrType(Expr->getType())); if (RecordLoc == nullptr) - return; + return false; const FunctionDecl *DirectCallee = Expr->getDirectCallee(); if (DirectCallee == nullptr) - return; + return false; StorageLocation &Loc = State.Lattice.getOrCreateConstMethodReturnStorageLocation( *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) { @@ -761,8 +772,15 @@ static void handleConstStatusOrAccessorMemberCall( } else { State.Env.setStorageLocation(*Expr, Loc); } + return true; } +static void handleConstStatusOrAccessorMemberCall( + const CallExpr *Expr, RecordStorageLocation *RecordLoc, + const MatchFinder::MatchResult &Result, LatticeTransferState &State) { + if (!doHandleConstStatusOrAccessorMemberCall(Expr, RecordLoc, Result, State)) + transferStatusOrReturningCall(Expr, State); +} static void handleConstStatusOrPointerAccessorMemberCall( const CallExpr *Expr, RecordStorageLocation *RecordLoc, const MatchFinder::MatchResult &Result, LatticeTransferState &State) { @@ -804,16 +822,6 @@ static void transferConstStatusOrPointerAccessorMemberOperatorCall( handleConstStatusOrPointerAccessorMemberCall(Expr, RecordLoc, Result, State); } -static void transferStatusOrReturningCall(const CallExpr *Expr, - LatticeTransferState &State) { - RecordStorageLocation *StatusOrLoc = - Expr->isPRValue() ? &State.Env.getResultObjectLocation(*Expr) - : State.Env.get(*Expr); - if (StatusOrLoc != nullptr && - State.Env.getValue(locForOk(locForStatus(*StatusOrLoc))) == nullptr) - initializeStatusOr(*StatusOrLoc, State.Env); -} - static void handleNonConstMemberCall(const CallExpr *Expr, RecordStorageLocation *RecordLoc, const MatchFinder::MatchResult &Result, diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index dcb1cc13146bd..deaeea31523cb 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3303,7 +3303,7 @@ TEST_P(UncheckedStatusOrAccessModelTest, AccessorCall) { } )cc"); - // Calls nonconst method inbetween. + // Calls nonconst method in between. ExpectDiagnosticsFor( R"cc( #include "unchecked_statusor_access_test_defs.h" @@ -3324,7 +3324,7 @@ TEST_P(UncheckedStatusOrAccessModelTest, AccessorCall) { } )cc"); - // Calls nonconst operator inbetween. + // Calls nonconst operator in between. ExpectDiagnosticsFor( R"cc( #include "unchecked_statusor_access_test_defs.h" From 3dc94dfa8718638afd55266193e3ea9a12788144 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Mon, 8 Dec 2025 11:29:04 -0800 Subject: [PATCH 3/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?= =?UTF-8?q?anges=20introduced=20through=20rebase?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.7 [skip ci] --- .../Models/UncheckedStatusOrAccessModel.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 364d5c1009a36..09494322a16c4 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -826,11 +826,10 @@ static void handleNonConstMemberCall(const CallExpr *Expr, RecordStorageLocation *RecordLoc, const MatchFinder::MatchResult &Result, LatticeTransferState &State) { - if (RecordLoc == nullptr) - return; - State.Lattice.clearConstMethodReturnValues(*RecordLoc); - State.Lattice.clearConstMethodReturnStorageLocations(*RecordLoc); - + if (RecordLoc) { + State.Lattice.clearConstMethodReturnValues(*RecordLoc); + State.Lattice.clearConstMethodReturnStorageLocations(*RecordLoc); + } if (isStatusOrType(Expr->getType())) transferStatusOrReturningCall(Expr, State); }