[llvm-branch-commits] [FlowSensitive] [StatusOr] [12/N] Add support for smart pointers (PR #170943)

[via llvm-branch-commits]

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/170943.diff


2 Files Affected:

- (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (+48) 
- (modified) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (+74) 


``````````diff
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<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;
+}
+
 CFGMatchSwitch<LatticeTransferState>
 buildTransferMatchSwitch(ASTContext &Ctx,
                          CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -899,6 +910,43 @@ buildTransferMatchSwitch(ASTContext &Ctx,
                                transferLoggingGetReferenceableValueCall)
       .CaseOfCFGStmt<CallExpr>(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<CXXOperatorCallExpr>(
+          isPointerLikeOperatorStar(),
+          [](const CXXOperatorCallExpr *E,
+             const MatchFinder::MatchResult &Result,
+             LatticeTransferState &State) {
+            transferSmartPointerLikeCachedDeref(
+                E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env),
+                State, [](StorageLocation &Loc) {});
+          })
+      .CaseOfCFGStmt<CXXOperatorCallExpr>(
+          isPointerLikeOperatorArrow(),
+          [](const CXXOperatorCallExpr *E,
+             const MatchFinder::MatchResult &Result,
+             LatticeTransferState &State) {
+            transferSmartPointerLikeCachedGet(
+                E, getSmartPtrLikeStorageLocation(*E->getArg(0), State.Env),
+                State, [](StorageLocation &Loc) {});
+          })
+      .CaseOfCFGStmt<CXXMemberCallExpr>(
+          isSmartPointerLikeValueMethodCall(),
+          [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
+             LatticeTransferState &State) {
+            transferSmartPointerLikeCachedDeref(
+                E, getImplicitObjectLocation(*E, State.Env), State,
+                [](StorageLocation &Loc) {});
+          })
+      .CaseOfCFGStmt<CXXMemberCallExpr>(
+          isSmartPointerLikeGetMethodCall(),
+          [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
+             LatticeTransferState &State) {
+            transferSmartPointerLikeCachedGet(
+                E, getImplicitObjectLocation(*E, State.Env), State,
+                [](StorageLocation &Loc) {});
+          })
       // const accessor calls
       .CaseOfCFGStmt<CXXMemberCallExpr>(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<int, STATUSOR_VOIDPTR>& operator*() const;
+      std::pair<int, STATUSOR_VOIDPTR>* 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<int, STATUSOR_INT>& operator*() const;
+      std::pair<int, STATUSOR_INT>* 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<int, STATUSOR_VOIDPTR>* 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<std::unique_ptr<STATUSOR_INT>>();
+          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<std::unique_ptr<STATUSOR_INT>>();
+          if (sor_up->ok()) {
+            sor_up.reset(Make<STATUSOR_INT*>());
+            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 <typename T>
                              T Make();

``````````

</details>


https://github.com/llvm/llvm-project/pull/170943