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