1 files changed, 89 insertions, 0 deletions
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
index 90551c2..b42bfa3 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
@@ -177,6 +177,41 @@ static auto isPointerComparisonOperatorCall(std::string operator_name) {
                             pointee(anyOf(statusOrType(), statusType())))))));
 }
 
+// The nullPointerConstant in the two matchers below is to support
+// absl::StatusOr<void*> X = nullptr.
+// nullptr does not match the bound type.
+// TODO: be less restrictive around convertible types in general.
+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(hasAnyName("absl::in_place_t",
+                                                     "std::in_place_t"))))));
+}
+
+static auto isStatusOrConstructor() {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return cxxConstructExpr(hasType(statusOrType()));
+}
+
+static auto isStatusConstructor() {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return cxxConstructExpr(hasType(statusType()));
+}
+
 static auto
 buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
   return CFGMatchSwitchBuilder<const Environment,
@@ -528,6 +563,46 @@ 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());
+}
+
+static void transferStatusOrConstructor(const CXXConstructExpr *Expr,
+                                        const MatchFinder::MatchResult &,
+                                        LatticeTransferState &State) {
+  RecordStorageLocation &StatusOrLoc = State.Env.getResultObjectLocation(*Expr);
+  RecordStorageLocation &StatusLoc = locForStatus(StatusOrLoc);
+
+  if (State.Env.getValue(locForOk(StatusLoc)) == nullptr)
+    initializeStatusOr(StatusOrLoc, State.Env);
+}
+
+static void transferStatusConstructor(const CXXConstructExpr *Expr,
+                                      const MatchFinder::MatchResult &,
+                                      LatticeTransferState &State) {
+  RecordStorageLocation &StatusLoc = State.Env.getResultObjectLocation(*Expr);
+
+  if (State.Env.getValue(locForOk(StatusLoc)) == nullptr)
+    initializeStatus(StatusLoc, State.Env);
+}
+
 CFGMatchSwitch<LatticeTransferState>
 buildTransferMatchSwitch(ASTContext &Ctx,
                          CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -573,6 +648,20 @@ buildTransferMatchSwitch(ASTContext &Ctx,
       .CaseOfCFGStmt<CallExpr>(isNotOkStatusCall(), transferNotOkStatusCall)
       .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("emplace"),
                                         transferEmplaceCall)
+      .CaseOfCFGStmt<CXXOperatorCallExpr>(isStatusOrValueAssignmentCall(),
+                                          transferValueAssignmentCall)
+      .CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(),
+                                       transferValueConstructor)
+      // N.B. These need to come after all other CXXConstructExpr.
+      // These are there to make sure that every Status and StatusOr object
+      // have their ok boolean initialized when constructed. If we were to
+      // lazily initialize them when we first access them, we can produce
+      // false positives if that first access is in a control flow statement.
+      // You can comment out these two constructors and see tests fail.
+      .CaseOfCFGStmt<CXXConstructExpr>(isStatusOrConstructor(),
+                                       transferStatusOrConstructor)
+      .CaseOfCFGStmt<CXXConstructExpr>(isStatusConstructor(),
+                                       transferStatusConstructor)
       .Build();
 }