diff --git a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h index 6496771ad037e..4e316aaf09bdb 100644 --- a/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h +++ b/clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h @@ -73,6 +73,10 @@ template class CachedConstAccessorsLattice : public Base { /// /// - `Callee` should return a location (return type is a reference type or a /// record type). + StorageLocation &getOrCreateConstMethodReturnStorageLocation( + const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee, + Environment &Env, + llvm::function_ref Initialize); StorageLocation &getOrCreateConstMethodReturnStorageLocation( const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee, Environment &Env, llvm::function_ref Initialize); @@ -196,7 +200,8 @@ template StorageLocation & CachedConstAccessorsLattice::getOrCreateConstMethodReturnStorageLocation( const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee, - Environment &Env, llvm::function_ref Initialize) { + Environment &Env, + llvm::function_ref Initialize) { assert(Callee != nullptr); QualType Type = Callee->getReturnType(); assert(!Type.isNull()); @@ -206,13 +211,24 @@ CachedConstAccessorsLattice::getOrCreateConstMethodReturnStorageLocation( if (it != ObjMap.end()) return *it->second; + auto T = Type.getNonReferenceType(); StorageLocation &Loc = Env.createStorageLocation(Type.getNonReferenceType()); - Initialize(Loc); + Initialize(T, Loc); ObjMap.insert({Callee, &Loc}); return Loc; } +template +StorageLocation & +CachedConstAccessorsLattice::getOrCreateConstMethodReturnStorageLocation( + const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee, + Environment &Env, llvm::function_ref Initialize) { + return getOrCreateConstMethodReturnStorageLocation( + RecordLoc, Callee, Env, + [Initialize](QualType T, StorageLocation &Loc) { Initialize(Loc); }); +} + } // namespace dataflow } // namespace clang 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 { + : public DataflowAnalysis> { 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..03fa05f69ef1d 100644 --- a/clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h +++ b/clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h @@ -58,6 +58,8 @@ 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 isPointerLikeConstructor(); +ast_matchers::StatementMatcher isSmartPointerLikeConstructor(); ast_matchers::StatementMatcher isPointerLikeOperatorStar(); ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar(); ast_matchers::StatementMatcher isPointerLikeOperatorArrow(); @@ -80,6 +82,8 @@ isSmartPointerLikeGetMethodCall(clang::StringRef MethodName = "get"); const FunctionDecl * getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE); +const FunctionDecl * +getCanonicalSmartPointerLikeOperatorCalleeForType(const CXXRecordDecl *RD); /// A transfer function for `operator*` (and `value`) calls that can be /// cached. Runs the `InitializeLoc` callback to initialize any new /// StorageLocations. @@ -91,7 +95,18 @@ template void transferSmartPointerLikeCachedDeref( const CallExpr *DerefExpr, RecordStorageLocation *SmartPointerLoc, TransferState &State, - llvm::function_ref InitializeLoc); + llvm::function_ref InitializeLoc); +template +void transferSmartPointerLikeCachedDeref( + const CallExpr *DerefExpr, RecordStorageLocation *SmartPointerLoc, + TransferState &State, + llvm::function_ref InitializeLoc) { + transferSmartPointerLikeCachedDeref( + DerefExpr, SmartPointerLoc, State, + [InitializeLoc](QualType T, StorageLocation &Loc) { + InitializeLoc(Loc); + }); +} /// A transfer function for `operator->` (and `get`) calls that can be cached. /// Runs the `InitializeLoc` callback to initialize any new StorageLocations. @@ -103,13 +118,24 @@ template void transferSmartPointerLikeCachedGet( const CallExpr *GetExpr, RecordStorageLocation *SmartPointerLoc, TransferState &State, - llvm::function_ref InitializeLoc); + llvm::function_ref InitializeLoc); +template +void transferSmartPointerLikeCachedGet( + const CallExpr *GetExpr, RecordStorageLocation *SmartPointerLoc, + TransferState &State, + llvm::function_ref InitializeLoc) { + transferSmartPointerLikeCachedGet( + GetExpr, SmartPointerLoc, State, + [InitializeLoc](QualType T, StorageLocation &Loc) { + InitializeLoc(Loc); + }); +} template void transferSmartPointerLikeCachedDeref( const CallExpr *DerefExpr, RecordStorageLocation *SmartPointerLoc, TransferState &State, - llvm::function_ref InitializeLoc) { + llvm::function_ref InitializeLoc) { if (State.Env.getStorageLocation(*DerefExpr) != nullptr) return; if (SmartPointerLoc == nullptr) @@ -141,11 +167,25 @@ void transferSmartPointerLikeCachedDeref( State.Env.setStorageLocation(*DerefExpr, LocForValue); } +// This was introduced after the QualType was added to InitializeLoc, so +// we don't provide a compatibility wrapper. +template +void transferSmartPointerLikeConstructor( + const CXXConstructExpr *ConstructOperator, + RecordStorageLocation *SmartPointerLoc, TransferState &State, + llvm::function_ref InitializeLoc) { + const FunctionDecl *CanonicalCallee = + getCanonicalSmartPointerLikeOperatorCalleeForType( + ConstructOperator->getType()->getAsCXXRecordDecl()); + State.Lattice.getOrCreateConstMethodReturnStorageLocation( + *SmartPointerLoc, CanonicalCallee, State.Env, InitializeLoc); +} + template void transferSmartPointerLikeCachedGet( const CallExpr *GetExpr, RecordStorageLocation *SmartPointerLoc, TransferState &State, - llvm::function_ref InitializeLoc) { + llvm::function_ref InitializeLoc) { if (SmartPointerLoc == nullptr) return; diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 7a698f276d6c1..3fb73a4210778 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" @@ -211,6 +212,16 @@ static auto isStatusConstructor() { return cxxConstructExpr(hasType(statusType())); } +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 CFGMatchSwitchBuilderisPRValue() ? &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; +} + +static void maybeInitLocForCtor(QualType T, StorageLocation &Loc, + Environment &Env) { + if (isStatusOrType(T)) { + initializeStatusOr(cast(Loc), Env); + } else if (isStatusType(T)) { + initializeStatus(cast(Loc), Env); + } else { + llvm::errs() << T << "\n"; + } +} + CFGMatchSwitch buildTransferMatchSwitch(ASTContext &Ctx, CFGMatchSwitchBuilder Builder) { @@ -667,6 +735,51 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferStatusOrConstructor) .CaseOfCFGStmt(isStatusConstructor(), transferStatusConstructor) + .CaseOfCFGStmt( + isPointerLikeOperatorArrow(), + [](const CXXOperatorCallExpr *E, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedGet( + E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env), + State, [&](QualType T, StorageLocation &Loc) { + maybeInitLocForCtor(T, Loc, State.Env); + }); + }) + .CaseOfCFGStmt( + isSmartPointerLikeConstructor(), + [](const CXXConstructExpr *E, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeConstructor( + E, &State.Env.getResultObjectLocation(*E), State, + [&](QualType T, StorageLocation &Loc) { + maybeInitLocForCtor(T, Loc, State.Env); + }); + }) + .CaseOfCFGStmt( + isSmartPointerLikeValueMethodCall(), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedDeref( + E, getImplicitObjectLocation(*E, State.Env), State, + [&](QualType T, StorageLocation &Loc) { + maybeInitLocForCtor(T, Loc, State.Env); + }); + }) + .CaseOfCFGStmt( + isSmartPointerLikeGetMethodCall(), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + transferSmartPointerLikeCachedGet( + E, getImplicitObjectLocation(*E, State.Env), State, + [&](QualType T, StorageLocation &Loc) { + maybeInitLocForCtor(T, Loc, State.Env); + }); + }) + .CaseOfCFGStmt(isNonConstMemberCall(), + transferNonConstMemberCall) + .CaseOfCFGStmt(isNonConstMemberOperatorCall(), + transferNonConstMemberOperatorCall) .Build(); } diff --git a/clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp b/clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp index d87b2e6f03857..e639119e1a290 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 isSmartPointerLikeConstructor() { + using namespace ast_matchers; + return cxxConstructExpr(hasType(hasCanonicalType(qualType( + hasDeclaration(cxxRecordDecl(smartPointerClassWithGetOrValue())))))); +} + ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar() { return cxxOperatorCallExpr( hasOverloadedOperatorName("*"), @@ -145,6 +151,12 @@ ast_matchers::StatementMatcher isSmartPointerLikeOperatorArrow() { ofClass(smartPointerClassWithGetOrValue())))); } +ast_matchers::StatementMatcher isPointerLikeConstructor() { + using namespace ast_matchers; + return cxxConstructExpr(hasType(hasCanonicalType( + qualType(hasDeclaration(cxxRecordDecl(pointerClass())))))); +} + ast_matchers::StatementMatcher isPointerLikeOperatorStar() { return cxxOperatorCallExpr( hasOverloadedOperatorName("*"), @@ -177,15 +189,8 @@ isSmartPointerLikeGetMethodCall(clang::StringRef MethodName) { } const FunctionDecl * -getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE) { +getCanonicalSmartPointerLikeOperatorCalleeForType(const CXXRecordDecl *RD) { const FunctionDecl *CanonicalCallee = nullptr; - const CXXMethodDecl *Callee = - cast_or_null(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 +201,16 @@ getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE) { return CanonicalCallee; } +const FunctionDecl * +getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE) { + const CXXMethodDecl *Callee = + cast_or_null(CE->getDirectCallee()); + if (Callee == nullptr) + return nullptr; + const CXXRecordDecl *RD = Callee->getParent(); + if (RD == nullptr) + return nullptr; + return getCanonicalSmartPointerLikeOperatorCalleeForType(RD); +} + } // namespace clang::dataflow diff --git a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp index 67b471e328b5e..5a195ed6cfc32 100644 --- a/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp @@ -307,5 +307,23 @@ TEST_F(CachedConstAccessorsLatticeTest, ProducesNewValueAfterJoinDistinct) { EXPECT_NE(ValAfterJoin2, Val3); } +TEST_F(CachedConstAccessorsLatticeTest, TypePassed) { + CommonTestInputs Inputs; + auto *CE = Inputs.CallRef; + RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(), + {}); + + LatticeT Lattice; + + const FunctionDecl *Callee = CE->getDirectCallee(); + auto RetType = Callee->getReturnType().getNonReferenceType(); + auto CheckedInit = [RetType](QualType T, StorageLocation &) { + ASSERT_EQ(T, RetType); + }; + ASSERT_NE(Callee, nullptr); + Lattice.getOrCreateConstMethodReturnStorageLocation(Loc, Callee, Env, + CheckedInit); +} + } // namespace } // 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 pair make_pair(T1 &&t1, T2 &&t2); #endif // STD_PAIR_H )cc"; +static 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&&...); +} +)cc"; + constexpr const char AbslLogHeader[] = R"cc( #ifndef ABSL_LOG_H #define ABSL_LOG_H @@ -2245,6 +2292,7 @@ std::vector> 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/SmartPointerAccessorCachingTest.cpp b/clang/unittests/Analysis/FlowSensitive/SmartPointerAccessorCachingTest.cpp index 5d3d5b0cfea09..05c66b0847c7a 100644 --- a/clang/unittests/Analysis/FlowSensitive/SmartPointerAccessorCachingTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/SmartPointerAccessorCachingTest.cpp @@ -64,6 +64,15 @@ TEST(SmartPointerAccessorCachingTest, MatchesClassWithStarArrowGet) { isSmartPointerLikeOperatorArrow())); EXPECT_TRUE(matches(Decls, "int target(UniquePtrAlias P) { return P->i; }", isPointerLikeOperatorArrow())); + + EXPECT_TRUE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); + EXPECT_TRUE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, MatchesClassWithStarArrow) { @@ -101,6 +110,15 @@ TEST(SmartPointerAccessorCachingTest, MatchesClassWithStarArrow) { isSmartPointerLikeOperatorArrow())); EXPECT_TRUE(matches(Decls, "int target(UniquePtrAlias P) { return P->i; }", isPointerLikeOperatorArrow())); + + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); + EXPECT_TRUE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, NoMatchIfUnexpectedReturnTypes) { @@ -141,6 +159,15 @@ TEST(SmartPointerAccessorCachingTest, NoMatchIfUnexpectedReturnTypes) { EXPECT_TRUE(matches(Decls, "int target(std::unique_ptr P) { return P->i; }", isPointerLikeOperatorArrow())); + + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, NoMatchIfBinaryStar) { @@ -163,6 +190,15 @@ TEST(SmartPointerAccessorCachingTest, NoMatchIfBinaryStar) { EXPECT_FALSE( matches(Decls, "int target(std::unique_ptr P) { return (P * 10).i; }", isPointerLikeOperatorStar())); + + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, NoMatchIfNoConstOverloads) { @@ -196,6 +232,15 @@ TEST(SmartPointerAccessorCachingTest, NoMatchIfNoConstOverloads) { EXPECT_FALSE( matches(Decls, "int target(std::unique_ptr P) { return P.get()->i; }", isSmartPointerLikeGetMethodCall())); + + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, NoMatchIfNoStarMethod) { @@ -221,6 +266,15 @@ TEST(SmartPointerAccessorCachingTest, NoMatchIfNoStarMethod) { EXPECT_FALSE(matches(Decls, "int target(std::unique_ptr P) { return P->i; }", isSmartPointerLikeGetMethodCall())); + + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); + EXPECT_FALSE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, MatchesWithValueAndNonConstOverloads) { @@ -276,6 +330,13 @@ TEST(SmartPointerAccessorCachingTest, MatchesWithValueAndNonConstOverloads) { Decls, "int target(const std::optional &Const) { return Const.value().i; }", isSmartPointerLikeValueMethodCall())); + + EXPECT_TRUE(matches( + Decls, "std::optional& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); + EXPECT_TRUE(matches( + Decls, "std::optional& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, MatchesWithTypeAliases) { @@ -329,6 +390,9 @@ TEST(SmartPointerAccessorCachingTest, MatchesWithTypeAliases) { EXPECT_TRUE(matches( Decls, "int target(const HasGetAndValue &Const) { return Const->i; }", isPointerLikeOperatorArrow())); + EXPECT_TRUE(matches( + Decls, "HasGetAndValue& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); EXPECT_TRUE(matches( Decls, @@ -346,6 +410,9 @@ TEST(SmartPointerAccessorCachingTest, MatchesWithTypeAliases) { Decls, "int target(const HasGetAndValue &Const) { return Const.get()->i; }", isSmartPointerLikeGetMethodCall())); + EXPECT_TRUE(matches( + Decls, "HasGetAndValue& Helper(); int target() { auto S = Helper(); }", + isSmartPointerLikeConstructor())); } TEST(SmartPointerAccessorCachingTest, Renamed) { @@ -390,6 +457,11 @@ TEST(SmartPointerAccessorCachingTest, Renamed) { EXPECT_TRUE(matches(Decls, "int target(UniquePtrAlias P) { return P->i; }", isPointerLikeOperatorArrow())); + + EXPECT_TRUE(matches( + Decls, + "std::unique_ptr& Helper(); int target() { auto S = Helper(); }", + isPointerLikeConstructor())); } } // namespace diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 67c37e1e0f77f..0a34435f63f2d 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3174,6 +3174,57 @@ TEST_P(UncheckedStatusOrAccessModelTest, ConstructStatusFromReference) { )cc"); } +TEST_P(UncheckedStatusOrAccessModelTest, SmartPtrLikeFromReference) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + class GetAble { + public: + STATUSOR_INT* get() const; + STATUSOR_INT* operator->() const; + STATUSOR_INT& operator*() const; + STATUSOR_INT& value(); + }; + void target() { + const auto sor1 = Make(); + const auto sor2 = Make(); + if (!sor1->ok() && !sor2->ok()) return; + if (sor1->ok() && !sor2->ok()) { + } else if (!sor1->ok() && sor2->ok()) { + } else { + sor1->value(); + sor2->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 @@ -3221,6 +3272,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" diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.h b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.h index 92dcd9328c322..97afdc77531d4 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.h +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.h @@ -102,7 +102,7 @@ class UncheckedStatusOrAccessModelTestExecutor std::vector Diagnostics; llvm::Error Error = test::checkDataflow( test::AnalysisInputs( - SourceCode, std::move(FuncMatcher), + SourceCode, FuncMatcher, [](ASTContext &Ctx, Environment &Env) { return Model(Ctx, Env); }) .withPostVisitCFG( [&Diagnostics, @@ -117,7 +117,7 @@ class UncheckedStatusOrAccessModelTestExecutor {"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}) .withASTBuildVirtualMappedFiles( tooling::FileContentMappings(Headers.begin(), Headers.end())), - /*VerifyResults=*/[&Diagnostics, SourceCode]( + /*VerifyResults=*/[&Diagnostics, SourceCode, &FuncMatcher]( const llvm::DenseMap &Annotations, const test::AnalysisOutputs &AO) { @@ -128,10 +128,21 @@ class UncheckedStatusOrAccessModelTestExecutor llvm::DenseSet DiagnosticLines; for (SourceLocation &Loc : Diagnostics) DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc)); - EXPECT_THAT(DiagnosticLines, testing::ContainerEq(AnnotationLines)) << "\nFailing code:\n" << SourceCode; + ast_matchers::internal::Matcher FM = FuncMatcher; + SmallVector MatchResult = + ast_matchers::match( + ast_matchers::functionDecl( + ast_matchers::hasBody(ast_matchers::stmt()), FM) + .bind("target"), + AO.ASTCtx); + for (auto N : MatchResult) { + if (auto *FD = N.getNodeAs("target")) { + FD->dump(llvm::errs()); + } + } }); if (Error) FAIL() << llvm::toString(std::move(Error));