WIP smartpointers

[fmayer]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: Florian Mayer (fmayer)

Changes

---

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 --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"