[FlowSensitive] [StatusOr] [8/N] Support value ctor and assignment

[fmayer]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: Florian Mayer (fmayer)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/163894.diff

3 Files Affected:

• (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (+48)
• (modified) clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp (+6-6)
• (modified) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (+172)

diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
index fa72cc24d8701..c1d9e8d202f3d 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
@@ -160,6 +160,29 @@ static auto isPointerComparisonOperatorCall(std::string operator_name) {
                     pointerType(pointee(possiblyAliasedStatusType())))))));
 }
 
+static auto isStatusOrValueAssignmentCall() {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return cxxOperatorCallExpr(
+      hasOverloadedOperatorName("="),
+      callee(cxxMethodDecl(ofClass(statusOrClass()))),
+      hasArgument(1, anyOf(hasType(hasUnqualifiedDesugaredType(
+                               type(equalsBoundNode("T")))),
+                           nullPointerConstant())));
+}
+
+static auto isStatusOrValueConstructor() {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return cxxConstructExpr(
+      hasType(statusOrType()),
+      hasArgument(0,
+                  anyOf(hasType(hasCanonicalType(type(equalsBoundNode("T")))),
+                        nullPointerConstant(),
+                        hasType(namedDecl(hasName("absl::in_place_t"))),
+                        hasType(namedDecl(hasName("std::in_place_t")))
+
+                            )));
+}
+
 static auto
 buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
   return CFGMatchSwitchBuilder<const Environment,
@@ -528,6 +551,27 @@ static void transferEmplaceCall(const CXXMemberCallExpr *Expr,
   State.Env.assume(OkVal.formula());
 }
 
+static void transferValueAssignmentCall(const CXXOperatorCallExpr *Expr,
+                                        const MatchFinder::MatchResult &,
+                                        LatticeTransferState &State) {
+  assert(Expr->getNumArgs() > 1);
+
+  auto *StatusOrLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0));
+  if (StatusOrLoc == nullptr)
+    return;
+
+  auto &OkVal = initializeStatusOr(*StatusOrLoc, State.Env);
+  State.Env.assume(OkVal.formula());
+}
+
+static void transferValueConstructor(const CXXConstructExpr *Expr,
+                                     const MatchFinder::MatchResult &,
+                                     LatticeTransferState &State) {
+  auto &OkVal =
+      initializeStatusOr(State.Env.getResultObjectLocation(*Expr), State.Env);
+  State.Env.assume(OkVal.formula());
+}
+
 CFGMatchSwitch<LatticeTransferState>
 buildTransferMatchSwitch(ASTContext &Ctx,
                          CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -573,6 +617,10 @@ buildTransferMatchSwitch(ASTContext &Ctx,
       .CaseOfCFGStmt<CallExpr>(isNotOkStatusCall(), transferNotOkStatusCall)
       .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("emplace"),
                                         transferEmplaceCall)
+      .CaseOfCFGStmt<CXXOperatorCallExpr>(isStatusOrValueAssignmentCall(),
+                                          transferValueAssignmentCall)
+      .CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(),
+                                       transferValueConstructor)
       .Build();
 }
 
diff --git a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp
index 461af73ea6c01..2e528edd7c1f9 100644
--- a/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/MockHeaders.cpp
@@ -459,6 +459,10 @@ struct is_scalar
 template <>
 struct is_scalar<nullptr_t> : public true_type {};
 
+struct in_place_t {};
+
+constexpr in_place_t in_place;
+
 } // namespace std
 
 #endif // STD_TYPE_TRAITS_H
@@ -511,9 +515,8 @@ using remove_reference_t = typename std::remove_reference<T>::type;
 template <typename T>
 using decay_t = typename std::decay<T>::type;
 
-struct in_place_t {};
-
-constexpr in_place_t in_place;
+using std::in_place;
+using std::in_place_t;
 } // namespace absl
 
 #endif // ABSL_TYPE_TRAITS_H
@@ -589,9 +592,6 @@ static constexpr char StdOptionalHeader[] = R"(
 
 namespace std {
 
-struct in_place_t {};
-constexpr in_place_t in_place;
-
 struct nullopt_t {
   constexpr explicit nullopt_t() {}
 };
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
index f354441299156..1a7aba0aa6ca5 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
@@ -2963,6 +2963,178 @@ TEST_P(UncheckedStatusOrAccessModelTest, Emplace) {
   )cc");
 }
 
+TEST_P(UncheckedStatusOrAccessModelTest, ValueConstruction) {
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_BOOL result = false;
+      result.value();
+    }
+  )cc");
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_INT result = 21;
+      result.value();
+    }
+  )cc");
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_INT result = Make<STATUSOR_INT>();
+      result.value(); // [[unsafe]]
+    }
+  )cc");
+  ExpectDiagnosticsFor(
+      R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+        void target() {
+          STATUSOR_BOOL result = false;
+          if (result.ok())
+            result.value();
+          else
+            result.value();
+        }
+      )cc");
+
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_BOOL result(false);
+      result.value();
+    }
+  )cc");
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_INT result(21);
+      result.value();
+    }
+  )cc");
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_INT result(Make<STATUSOR_INT>());
+      result.value(); // [[unsafe]]
+    }
+  )cc");
+  ExpectDiagnosticsFor(
+      R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+        void target() {
+          STATUSOR_BOOL result(false);
+          if (result.ok())
+            result.value();
+          else
+            result.value();
+        }
+      )cc");
+}
+
+TEST_P(UncheckedStatusOrAccessModelTest, ValueAssignment) {
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_BOOL result;
+      result = false;
+      result.value();
+    }
+  )cc");
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_INT result;
+      result = 21;
+      result.value();
+    }
+  )cc");
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_INT result;
+      result = Make<STATUSOR_INT>();
+      result.value();  // [[unsafe]]
+    }
+  )cc");
+  ExpectDiagnosticsFor(
+      R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+        void target() {
+          STATUSOR_BOOL result;
+          result = false;
+          if (result.ok())
+            result.value();
+          else
+            result.value();
+        }
+      )cc");
+}
+
+TEST_P(UncheckedStatusOrAccessModelTest, NestedStatusOr) {
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      absl::StatusOr<STATUSOR_INT> result;
+      result = Make<STATUSOR_INT>();
+      result.value();
+    }
+  )cc");
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      absl::StatusOr<STATUSOR_INT> result = Make<STATUSOR_INT>();
+      result.value();
+    }
+  )cc");
+}
+
+TEST_P(UncheckedStatusOrAccessModelTest, PtrConstruct) {
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_VOIDPTR sor = nullptr;
+      *sor;
+    }
+  )cc");
+
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_VOIDPTR sor(nullptr);
+      *sor;
+    }
+  )cc");
+}
+
+TEST_P(UncheckedStatusOrAccessModelTest, InPlaceConstruct) {
+  ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+    void target() {
+      STATUSOR_VOIDPTR absl_sor(absl::in_place, {nullptr});
+      *absl_sor;
+      STATUSOR_VOIDPTR std_sor(std::in_place, {nullptr});
+      *std_sor;
+    }
+  )cc");
+}
+
 } // namespace
 
 std::string

[fmayer]

The optional changes are in #163897, but I don't want to redo the whole SPR stack, so I'll just rebase once that is merged.

[fmayer]

@BaLiKfromUA CC