[FlowSensitive] [StatusOr] [4/N] Support comparisons

clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp

@@ -24,6 +24,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/LLVM.h"
@@ -95,6 +96,18 @@ static QualType getStatusOrValueType(ClassTemplateSpecializationDecl *TRD) {
   return TRD->getTemplateArgs().get(0).getAsType();
 }
 
+static auto ofClassStatus() {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return ofClass(hasName("::absl::Status"));
+}
+
+static auto isStatusMemberCallWithName(llvm::StringRef member_name) {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return cxxMemberCallExpr(
+      on(expr(unless(cxxThisExpr()))),
+      callee(cxxMethodDecl(hasName(member_name), ofClassStatus())));
+}
+
 static auto isStatusOrMemberCallWithName(llvm::StringRef member_name) {
   using namespace ::clang::ast_matchers; // NOLINT: Too many names
   return cxxMemberCallExpr(
@@ -124,6 +137,19 @@ static auto valueOperatorCall() {
                     isStatusOrOperatorCallWithName("->")));
 }
 
+static clang::ast_matchers::TypeMatcher statusType() {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return hasCanonicalType(qualType(hasDeclaration(statusClass())));
+}
+
+static auto isComparisonOperatorCall(llvm::StringRef operator_name) {
+  using namespace ::clang::ast_matchers; // NOLINT: Too many names
+  return cxxOperatorCallExpr(
+      hasOverloadedOperatorName(operator_name), argumentCountIs(2),
+      hasArgument(0, anyOf(hasType(statusType()), hasType(statusOrType()))),
+      hasArgument(1, anyOf(hasType(statusType()), hasType(statusOrType()))));
+}
+
 static auto
 buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
   return CFGMatchSwitchBuilder<const Environment,
@@ -249,13 +275,184 @@ static void transferStatusOrOkCall(const CXXMemberCallExpr *Expr,
   State.Env.setValue(*Expr, OkVal);
 }
 
+static void transferStatusCall(const CXXMemberCallExpr *Expr,
+                               const MatchFinder::MatchResult &,
+                               LatticeTransferState &State) {
+  RecordStorageLocation *StatusOrLoc =
+      getImplicitObjectLocation(*Expr, State.Env);
+  if (StatusOrLoc == nullptr)
+    return;
+
+  RecordStorageLocation &StatusLoc = locForStatus(*StatusOrLoc);
+
+  if (State.Env.getValue(locForOk(StatusLoc)) == nullptr)
+    initializeStatusOr(*StatusOrLoc, State.Env);
+
+  if (Expr->isPRValue())
+    copyRecord(StatusLoc, State.Env.getResultObjectLocation(*Expr), State.Env);
+  else
+    State.Env.setStorageLocation(*Expr, StatusLoc);
+}
+
+static void transferStatusOkCall(const CXXMemberCallExpr *Expr,
+                                 const MatchFinder::MatchResult &,
+                                 LatticeTransferState &State) {
+  RecordStorageLocation *StatusLoc =
+      getImplicitObjectLocation(*Expr, State.Env);
+  if (StatusLoc == nullptr)
+    return;
+
+  if (Value *Val = State.Env.getValue(locForOk(*StatusLoc)))
+    State.Env.setValue(*Expr, *Val);
+}
+
+static void transferStatusUpdateCall(const CXXMemberCallExpr *Expr,
+                                     const MatchFinder::MatchResult &,
+                                     LatticeTransferState &State) {
+  assert(Expr->getNumArgs() == 1);
+  auto *Arg = Expr->getArg(0);
+  RecordStorageLocation *ArgRecord =
+      Arg->isPRValue() ? &State.Env.getResultObjectLocation(*Arg)
+                       : State.Env.get<RecordStorageLocation>(*Arg);
+  RecordStorageLocation *ThisLoc = getImplicitObjectLocation(*Expr, State.Env);
+  if (ThisLoc == nullptr || ArgRecord == nullptr)
+    return;
+
+  auto &ThisOkVal = valForOk(*ThisLoc, State.Env);
+  auto &ArgOkVal = valForOk(*ArgRecord, State.Env);
+  auto &A = State.Env.arena();
+  auto &NewVal = State.Env.makeAtomicBoolValue();
+  State.Env.assume(A.makeImplies(A.makeNot(ThisOkVal.formula()),
+                                 A.makeNot(NewVal.formula())));
+  State.Env.assume(A.makeImplies(NewVal.formula(), ArgOkVal.formula()));
+  State.Env.setValue(locForOk(*ThisLoc), NewVal);
+}
+
+static BoolValue *evaluateStatusEquality(RecordStorageLocation &LhsStatusLoc,
+                                         RecordStorageLocation &RhsStatusLoc,
+                                         Environment &Env) {
+  auto &A = Env.arena();
+  // Logically, a Status object is composed of an error code that could take one
+  // of multiple possible values, including the "ok" value. We track whether a
+  // Status object has an "ok" value and represent this as an `ok` bit. Equality
+  // of Status objects compares their error codes. Therefore, merely comparing
+  // the `ok` bits isn't sufficient: when two Status objects are assigned non-ok
+  // error codes the equality of their respective error codes matters. Since we
+  // only track the `ok` bits, we can't make any conclusions about equality when
+  // we know that two Status objects have non-ok values.
+
+  auto &LhsOkVal = valForOk(LhsStatusLoc, Env);
+  auto &RhsOkVal = valForOk(RhsStatusLoc, Env);
+
+  auto &Res = Env.makeAtomicBoolValue();
+
+  // lhs && rhs => res (a.k.a. !res => !lhs || !rhs)
+  Env.assume(A.makeImplies(A.makeAnd(LhsOkVal.formula(), RhsOkVal.formula()),
+                           Res.formula()));
+  // res => (lhs == rhs)
+  Env.assume(A.makeImplies(
+      Res.formula(), A.makeEquals(LhsOkVal.formula(), RhsOkVal.formula())));
+
+  return &Res;
+}
+
+static BoolValue *
+evaluateStatusOrEquality(RecordStorageLocation &LhsStatusOrLoc,
+                         RecordStorageLocation &RhsStatusOrLoc,
+                         Environment &Env) {
+  auto &A = Env.arena();
+  // Logically, a StatusOr<T> object is composed of two values - a Status and a
+  // value of type T. Equality of StatusOr objects compares both values.
+  // Therefore, merely comparing the `ok` bits of the Status values isn't
+  // sufficient. When two StatusOr objects are engaged, the equality of their
+  // respective values of type T matters. Similarly, when two StatusOr objects
+  // have Status values that have non-ok error codes, the equality of the error
+  // codes matters. Since we only track the `ok` bits of the Status values, we
+  // can't make any conclusions about equality when we know that two StatusOr
+  // objects are engaged or when their Status values contain non-ok error codes.
+  auto &LhsOkVal = valForOk(locForStatus(LhsStatusOrLoc), Env);
+  auto &RhsOkVal = valForOk(locForStatus(RhsStatusOrLoc), Env);
+  auto &res = Env.makeAtomicBoolValue();
+
+  // res => (lhs == rhs)
+  Env.assume(A.makeImplies(
+      res.formula(), A.makeEquals(LhsOkVal.formula(), RhsOkVal.formula())));
+  return &res;
+}
+
+static BoolValue *evaluateEquality(const Expr *LhsExpr, const Expr *RhsExpr,
+                                   Environment &Env) {
+  // Check the type of both sides in case an operator== is added that admits
+  // different types.
+  if (isStatusOrType(LhsExpr->getType()) &&
+      isStatusOrType(RhsExpr->getType())) {
+    auto *LhsStatusOrLoc = Env.get<RecordStorageLocation>(*LhsExpr);
+    if (LhsStatusOrLoc == nullptr)
+      return nullptr;
+    auto *RhsStatusOrLoc = Env.get<RecordStorageLocation>(*RhsExpr);
+    if (RhsStatusOrLoc == nullptr)
+      return nullptr;
+
+    return evaluateStatusOrEquality(*LhsStatusOrLoc, *RhsStatusOrLoc, Env);
+
+    // Check the type of both sides in case an operator== is added that admits
+    // different types.
+  }
+  if (isStatusType(LhsExpr->getType()) && isStatusType(RhsExpr->getType())) {
+    auto *LhsStatusLoc = Env.get<RecordStorageLocation>(*LhsExpr);
+    if (LhsStatusLoc == nullptr)
+      return nullptr;
+
+    auto *RhsStatusLoc = Env.get<RecordStorageLocation>(*RhsExpr);
+    if (RhsStatusLoc == nullptr)
+      return nullptr;
+
+    return evaluateStatusEquality(*LhsStatusLoc, *RhsStatusLoc, Env);
+  }
+  return nullptr;
+}
+
+static void transferComparisonOperator(const CXXOperatorCallExpr *Expr,
+                                       LatticeTransferState &State,
+                                       bool IsNegative) {
+  auto *LhsAndRhsVal =
+      evaluateEquality(Expr->getArg(0), Expr->getArg(1), State.Env);
+  if (LhsAndRhsVal == nullptr)
+    return;
+
+  if (IsNegative)
+    State.Env.setValue(*Expr, State.Env.makeNot(*LhsAndRhsVal));
+  else
+    State.Env.setValue(*Expr, *LhsAndRhsVal);
+}
+
 CFGMatchSwitch<LatticeTransferState>
 buildTransferMatchSwitch(ASTContext &Ctx,
                          CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
   using namespace ::clang::ast_matchers; // NOLINT: Too many names
   return std::move(Builder)
       .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("ok"),
                                         transferStatusOrOkCall)
+      .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("status"),
+                                        transferStatusCall)
+      .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusMemberCallWithName("ok"),
+                                        transferStatusOkCall)
+      .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusMemberCallWithName("Update"),
+                                        transferStatusUpdateCall)
+      .CaseOfCFGStmt<CXXOperatorCallExpr>(
+          isComparisonOperatorCall("=="),
+          [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &,
+             LatticeTransferState &State) {
+            transferComparisonOperator(Expr, State,
+                                       /*IsNegative=*/false);
+          })
+      .CaseOfCFGStmt<CXXOperatorCallExpr>(
+          isComparisonOperatorCall("!="),
+          [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &,
+             LatticeTransferState &State) {
+            transferComparisonOperator(Expr, State,
+                                       /*IsNegative=*/true);
+          })
       .Build();
 }