diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 038f2b0338c8d..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" @@ -842,6 +843,16 @@ transferNonConstMemberOperatorCall(const CXXOperatorCallExpr *Expr, 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, CFGMatchSwitchBuilder Builder) { @@ -899,6 +910,43 @@ 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) diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index e075818f8a2c1..dcb1cc13146bd 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3443,6 +3443,79 @@ TEST_P(UncheckedStatusOrAccessModelTest, AccessorCall) { )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 @@ -3492,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();