From 07669b0c67f9e7da4d1584496a7340d59175b14e Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Fri, 5 Dec 2025 15:38:15 -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 | 454 ++++++++++++++ .../Analysis/FlowSensitive/MockHeaders.cpp | 90 +++ ...ncheckedStatusOrAccessModelTestFixture.cpp | 571 ++++++++++++++++++ 4 files changed, 1118 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..c917c8e8c11ba 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,119 @@ 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 isMakePredicateFormatterFromIsOkMatcherCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl( + hasName("::testing::internal::MakePredicateFormatterFromMatcher"))), + hasArgument( + 0, hasType(cxxRecordDecl(hasAnyName( + "::testing::status::internal_status::IsOkMatcher", + "::absl_testing::status_internal::IsOkMatcher", + "::testing::status::internal_status::IsOkAndHoldsMatcher", + "::absl_testing::status_internal::IsOkAndHoldsMatcher"))))); +} + +static auto isStatusIsOkMatcherCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr(callee(functionDecl(hasAnyName( + "::testing::status::StatusIs", "absl_testing::StatusIs", + "::testing::status::CanonicalStatusIs", + "::absl_testing::CanonicalStatusIs"))), + hasArgument(0, declRefExpr(to(enumConstantDecl(hasAnyName( + "::absl::StatusCode::kOk", "OK")))))); +} + +static auto isMakePredicateFormatterFromStatusIsMatcherCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl( + hasName("::testing::internal::MakePredicateFormatterFromMatcher"))), + hasArgument(0, hasType(cxxRecordDecl(hasAnyName( + "::testing::status::internal_status::StatusIsMatcher", + "::testing::status::internal_status::" + "CanonicalStatusIsMatcher", + "::absl_testing::status_internal::StatusIsMatcher", + "::absl_testing::status_internal::" + "CanonicalStatusIsMatcher"))))); +} + +static auto isPredicateFormatterFromStatusMatcherCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr( + hasOverloadedOperatorName("()"), + callee(cxxMethodDecl(ofClass( + hasName("testing::internal::PredicateFormatterFromMatcher")))), + hasArgument(2, hasType(cxxRecordDecl(hasName("absl::Status"))))); +} + +static auto isPredicateFormatterFromStatusOrMatcherCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr( + hasOverloadedOperatorName("()"), + callee(cxxMethodDecl(ofClass( + hasName("testing::internal::PredicateFormatterFromMatcher")))), + hasArgument(2, hasType(statusOrType()))); +} + +static auto isAssertionResultOperatorBoolCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxMemberCallExpr( + on(expr(unless(cxxThisExpr()))), + callee(cxxMethodDecl(hasName("operator bool"), + ofClass(hasName("testing::AssertionResult"))))); +} + +static auto isAssertionResultConstructFromBoolCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxConstructExpr( + hasType(recordDecl(hasName("testing::AssertionResult"))), + hasArgument(0, hasType(booleanType()))); +} + static auto buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { return CFGMatchSwitchBuilder getSyntheticFields(QualType Ty, QualType StatusType, const CXXRecordDecl &RD) { if (auto *TRD = getStatusOrBaseClass(Ty)) @@ -328,6 +462,12 @@ llvm::StringMap getSyntheticFields(QualType Ty, QualType StatusType, if (isStatusType(Ty) || (RD.hasDefinition() && RD.isDerivedFrom(StatusType->getAsCXXRecordDecl()))) return {{"ok", RD.getASTContext().BoolTy}}; + if (isAssertionResultType(Ty)) + return {{"ok", RD.getASTContext().BoolTy}}; + if (isPredicateFormatterFromMatcherType(Ty)) + return {{"ok_predicate", RD.getASTContext().BoolTy}}; + if (isStatusIsMatcherType(Ty)) + return {{"ok_matcher", RD.getASTContext().BoolTy}}; return {}; } @@ -344,6 +484,13 @@ BoolValue &valForOk(RecordStorageLocation &StatusLoc, Environment &Env) { return *Val; return initializeStatus(StatusLoc, Env); } +static StorageLocation &locForOkPredicate(RecordStorageLocation &StatusLoc) { + return StatusLoc.getSyntheticField("ok_predicate"); +} + +static StorageLocation &locForOkMatcher(RecordStorageLocation &StatusLoc) { + return StatusLoc.getSyntheticField("ok_matcher"); +} static void transferStatusOrOkCall(const CXXMemberCallExpr *Expr, const MatchFinder::MatchResult &, @@ -697,12 +844,261 @@ static void transferPointerToBoolean(const ImplicitCastExpr *Expr, dyn_cast_or_null(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 void transferMakePredicateFormatterFromIsOkMatcherCall( + const CallExpr *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + State.Env.setValue( + locForOkPredicate(State.Env.getResultObjectLocation(*Expr)), + State.Env.getBoolLiteralValue(true)); +} + +static void transferStatusIsOkMatcherCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + BoolValue &OkMatcherVal = State.Env.getBoolLiteralValue(true); + State.Env.setValue(locForOkMatcher(State.Env.getResultObjectLocation(*Expr)), + OkMatcherVal); +} + +static void transferMakePredicateFormatterFromStatusIsMatcherCall( + const CallExpr *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->isPRValue()); + auto &Loc = State.Env.getResultObjectLocation(*Expr->getArg(0)); + auto &OkMatcherLoc = locForOkMatcher(Loc); + BoolValue *OkMatcherVal = State.Env.get(OkMatcherLoc); + if (OkMatcherVal == nullptr) + return; + State.Env.setValue( + locForOkPredicate(State.Env.getResultObjectLocation(*Expr)), + *OkMatcherVal); +} + +static void +transferPredicateFormatterMatcherCall(const CXXOperatorCallExpr *Expr, + LatticeTransferState &State, + bool IsStatusOr) { + auto *Loc = State.Env.get(*Expr->getArg(0)); + if (Loc == nullptr) + return; + + auto *ObjectLoc = State.Env.get(*Expr->getArg(2)); + if (ObjectLoc == nullptr) + return; + + auto &OkPredicateLoc = locForOkPredicate(*Loc); + BoolValue *OkPredicateVal = State.Env.get(OkPredicateLoc); + if (OkPredicateVal == nullptr) + return; + + if (IsStatusOr) + ObjectLoc = &locForStatus(*ObjectLoc); + auto &StatusOk = valForOk(*ObjectLoc, State.Env); + + auto &A = State.Env.arena(); + auto &Res = State.Env.makeAtomicBoolValue(); + State.Env.assume( + A.makeImplies(OkPredicateVal->formula(), + A.makeEquals(StatusOk.formula(), Res.formula()))); + State.Env.setValue(locForOk(State.Env.getResultObjectLocation(*Expr)), Res); +} + +static void +transferAssertionResultConstructFromBoolCall(const CXXConstructExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() > 0); + + auto *StatusAdaptorLoc = State.Env.get(*Expr->getArg(0)); + if (StatusAdaptorLoc == nullptr) + return; + BoolValue *OkVal = State.Env.get(*StatusAdaptorLoc); + if (OkVal == nullptr) + return; + State.Env.setValue(locForOk(State.Env.getResultObjectLocation(*Expr)), + *OkVal); +} + +static void +transferAssertionResultOperatorBoolCall(const CXXMemberCallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto *RecordLoc = getImplicitObjectLocation(*Expr, State.Env); + if (RecordLoc == nullptr) + return; + BoolValue *OkVal = State.Env.get(locForOk(*RecordLoc)); + if (OkVal == nullptr) + return; + auto &A = State.Env.arena(); + auto &Res = State.Env.makeAtomicBoolValue(); + State.Env.assume(A.makeEquals(OkVal->formula(), Res.formula())); + State.Env.setValue(*Expr, Res); +} + +static void transferDerefCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto *StatusOrLoc = State.Env.get(*Expr->getArg(0)); + + if (StatusOrLoc && State.Env.getStorageLocation(*Expr) == nullptr) + State.Env.setStorageLocation(*Expr, + StatusOrLoc->getSyntheticField("value")); +} + +static void transferArrowCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto *StatusOrLoc = State.Env.get(*Expr->getArg(0)); + if (!StatusOrLoc) + return; + State.Env.setValue(*Expr, State.Env.create( + StatusOrLoc->getSyntheticField("value"))); +} + +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, CFGMatchSwitchBuilder Builder) { using namespace ::clang::ast_matchers; // NOLINT: Too many names return std::move(Builder) + .CaseOfCFGStmt( + isMakePredicateFormatterFromIsOkMatcherCall(), + transferMakePredicateFormatterFromIsOkMatcherCall) + .CaseOfCFGStmt(isStatusIsOkMatcherCall(), + transferStatusIsOkMatcherCall) + .CaseOfCFGStmt( + isMakePredicateFormatterFromStatusIsMatcherCall(), + transferMakePredicateFormatterFromStatusIsMatcherCall) + .CaseOfCFGStmt( + isPredicateFormatterFromStatusOrMatcherCall(), + [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + transferPredicateFormatterMatcherCall(Expr, State, + /*IsStatusOr=*/true); + }) + .CaseOfCFGStmt( + isPredicateFormatterFromStatusMatcherCall(), + [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + transferPredicateFormatterMatcherCall(Expr, State, + /*IsStatusOr=*/false); + }) + .CaseOfCFGStmt( + isAssertionResultConstructFromBoolCall(), + transferAssertionResultConstructFromBoolCall) + .CaseOfCFGStmt(isAssertionResultOperatorBoolCall(), + transferAssertionResultOperatorBoolCall) .CaseOfCFGStmt(isStatusOrMemberCallWithName("ok"), transferStatusOrOkCall) .CaseOfCFGStmt(isStatusOrMemberCallWithName("status"), @@ -747,6 +1143,10 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferValueAssignmentCall) .CaseOfCFGStmt(isStatusOrValueConstructor(), transferValueConstructor) + .CaseOfCFGStmt(isStatusOrOperatorCallWithName("->"), + transferArrowCall) + .CaseOfCFGStmt(isStatusOrOperatorCallWithName("*"), + transferDerefCall) .CaseOfCFGStmt(isAsStatusCallWithStatus(), transferAsStatusCallWithStatus) .CaseOfCFGStmt(isAsStatusCallWithStatusOr(), @@ -755,6 +1155,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..cd7353c62f537 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -2173,6 +2173,181 @@ TEST_P(UncheckedStatusOrAccessModelTest, ExpectOkMacro) { )cc"); } +TEST_P(UncheckedStatusOrAccessModelTest, AssertThatMacro) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, testing::status::IsOk()); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, absl_testing::IsOk()); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor.status(), testing::status::IsOk()); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + void target() { + STATUSOR_INT sor = Make(); + ASSERT_THAT(sor, testing::status::IsOk()); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + using absl::StatusCode::kOk; + ASSERT_THAT(sor, testing::status::StatusIs(kOk)); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, testing::status::StatusIs(absl::StatusCode::kOk)); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, testing::status::CanonicalStatusIs(absl::StatusCode::kOk)); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, absl_testing::CanonicalStatusIs(absl::StatusCode::kOk)); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + auto matcher = testing::status::StatusIs(absl::StatusCode::kOk); + ASSERT_THAT(sor, matcher); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + auto matcher = absl_testing::StatusIs(absl::StatusCode::kOk); + ASSERT_THAT(sor, matcher); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT( + sor, testing::status::StatusIs(absl::StatusCode::kInvalidArgument)); + + sor.value(); // [[unsafe]] + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, absl_testing::StatusIs(absl::StatusCode::kInvalidArgument)); + + sor.value(); // [[unsafe]] + } + )cc"); + + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, testing::status::IsOkAndHolds(testing::IsTrue())); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_THAT(sor, absl_testing::IsOkAndHolds(testing::IsTrue())); + + sor.value(); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, AssertOkMacro) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_OK(sor); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + ASSERT_OK(sor.status()); + + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + void target() { + STATUSOR_INT sor = Make(); + ASSERT_OK(sor); + + sor.value(); + } + )cc"); + + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(std::optional sor_opt) { + STATUSOR_INT sor = *sor_opt; + ASSERT_OK(sor); + + sor.value(); + } + )cc"); +} + TEST_P(UncheckedStatusOrAccessModelTest, BreadthFirstBlockTraversalLoop) { // Evaluating the CFG blocks of the code below in breadth-first order results // in an infinite loop. Each iteration of the while loop below results in a @@ -3270,6 +3445,401 @@ 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"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, NestedStatusOrInStatusOrStruct) { + // Non-standard assignment with a nested StatusOr. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Inner { + absl::StatusOr sor; + }; + + struct Outer { + absl::StatusOr inner; + }; + + void target() { + Outer foo = Make(); + foo.inner->sor = "a"; // [[unsafe]] + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(const absl::StatusOr& foo) { + if (foo.ok() && foo->sor.ok()) foo->sor.value(); + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(const absl::StatusOr& foo) { + if (foo.ok() && (*foo).sor.ok()) (*foo).sor.value(); + } + )cc"); + + // With assignment. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(absl::StatusOr& foo) { + if (foo.ok() && foo->sor.ok()) { + foo->sor = Make>(); + foo->sor.value(); // [[unsafe]] + } + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(absl::StatusOr& foo) { + if (foo.ok() && foo->sor.ok()) { + auto n = Make>(); + if (n.ok()) foo->sor = n; + foo->sor.value(); + } + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(absl::StatusOr& foo) { + if (foo.ok() && foo->sor.ok()) { + auto n = Make>(); + if (n.ok()) foo->sor = std::move(n); + foo->sor.value(); + } + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(absl::StatusOr& foo) { + if (foo.ok() && foo->sor.ok()) *foo->sor; + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(absl::StatusOr& foo) { + if (!foo.ok()) return; + if (!foo->sor.ok()) + foo->sor.value(); // [[unsafe]] + else + foo->sor.value(); + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr sor; + }; + + void target(absl::StatusOr& foo, bool b) { + if (!foo.ok()) return; + if (b) { + if (!foo->sor.ok()) return; + foo->sor.value(); + } else { + if (!foo->sor.ok()) return; + foo->sor.value(); + } + foo->sor.value(); + } + )cc"); +} + } // namespace std::string @@ -3319,6 +3889,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 2a3b029d481c29ed93dfdf6d1a8b3c3004ec9495 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Mon, 8 Dec 2025 11:24:40 -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 c917c8e8c11ba..02a111e5008b7 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -844,15 +844,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) { @@ -864,8 +875,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) { @@ -907,16 +925,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 cd7353c62f537..2640820ca6501 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3478,7 +3478,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" @@ -3499,7 +3499,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 5047745dc995db8db3f474a3f5cae3b6d287c2df Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Mon, 8 Dec 2025 11:29:18 -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 02a111e5008b7..5d375341ce2b8 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -929,11 +929,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); }