WIP smartpointers

clang/include/clang/Analysis/FlowSensitive/CachedConstAccessorsLattice.h

@@ -73,6 +73,10 @@ template <typename Base> class CachedConstAccessorsLattice : public Base {
   ///
   ///  - `Callee` should return a location (return type is a reference type or a
   ///     record type).
+  StorageLocation &getOrCreateConstMethodReturnStorageLocation(
+      const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee,
+      Environment &Env,
+      llvm::function_ref<void(QualType, StorageLocation &)> Initialize);
   StorageLocation &getOrCreateConstMethodReturnStorageLocation(
       const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee,
       Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize);
@@ -196,7 +200,8 @@ template <typename Base>
 StorageLocation &
 CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
     const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee,
-    Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize) {
+    Environment &Env,
+    llvm::function_ref<void(QualType, StorageLocation &)> Initialize) {
   assert(Callee != nullptr);
   QualType Type = Callee->getReturnType();
   assert(!Type.isNull());
@@ -206,13 +211,24 @@ CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
   if (it != ObjMap.end())
     return *it->second;
 
+  auto T = Type.getNonReferenceType();
   StorageLocation &Loc = Env.createStorageLocation(Type.getNonReferenceType());
-  Initialize(Loc);
+  Initialize(T, Loc);
 
   ObjMap.insert({Callee, &Loc});
   return Loc;
 }
 
+template <typename Base>
+StorageLocation &
+CachedConstAccessorsLattice<Base>::getOrCreateConstMethodReturnStorageLocation(
+    const RecordStorageLocation &RecordLoc, const FunctionDecl *Callee,
+    Environment &Env, llvm::function_ref<void(StorageLocation &)> Initialize) {
+  return getOrCreateConstMethodReturnStorageLocation(
+      RecordLoc, Callee, Env,
+      [Initialize](QualType T, StorageLocation &Loc) { Initialize(Loc); });
+}
+
 } // namespace dataflow
 } // namespace clang
 

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);
 

clang/include/clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h

@@ -58,6 +58,8 @@ 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 isPointerLikeConstructor();
+ast_matchers::StatementMatcher isSmartPointerLikeConstructor();
 ast_matchers::StatementMatcher isPointerLikeOperatorStar();
 ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar();
 ast_matchers::StatementMatcher isPointerLikeOperatorArrow();
@@ -80,6 +82,8 @@ isSmartPointerLikeGetMethodCall(clang::StringRef MethodName = "get");
 const FunctionDecl *
 getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE);
 
+const FunctionDecl *
+getCanonicalSmartPointerLikeOperatorCalleeForType(const CXXRecordDecl *RD);
 /// A transfer function for `operator*` (and `value`) calls that can be
 /// cached. Runs the `InitializeLoc` callback to initialize any new
 /// StorageLocations.
@@ -91,7 +95,18 @@ template <typename LatticeT>
 void transferSmartPointerLikeCachedDeref(
     const CallExpr *DerefExpr, RecordStorageLocation *SmartPointerLoc,
     TransferState<LatticeT> &State,
-    llvm::function_ref<void(StorageLocation &)> InitializeLoc);
+    llvm::function_ref<void(QualType, StorageLocation &)> InitializeLoc);
+template <typename LatticeT>
+void transferSmartPointerLikeCachedDeref(
+    const CallExpr *DerefExpr, RecordStorageLocation *SmartPointerLoc,
+    TransferState<LatticeT> &State,
+    llvm::function_ref<void(StorageLocation &)> InitializeLoc) {
+  transferSmartPointerLikeCachedDeref<LatticeT>(
+      DerefExpr, SmartPointerLoc, State,
+      [InitializeLoc](QualType T, StorageLocation &Loc) {
+        InitializeLoc(Loc);
+      });
+}
 
 /// A transfer function for `operator->` (and `get`) calls that can be cached.
 /// Runs the `InitializeLoc` callback to initialize any new StorageLocations.
@@ -103,13 +118,24 @@ template <typename LatticeT>
 void transferSmartPointerLikeCachedGet(
     const CallExpr *GetExpr, RecordStorageLocation *SmartPointerLoc,
     TransferState<LatticeT> &State,
-    llvm::function_ref<void(StorageLocation &)> InitializeLoc);
+    llvm::function_ref<void(QualType, StorageLocation &)> InitializeLoc);
+template <typename LatticeT>
+void transferSmartPointerLikeCachedGet(
+    const CallExpr *GetExpr, RecordStorageLocation *SmartPointerLoc,
+    TransferState<LatticeT> &State,
+    llvm::function_ref<void(StorageLocation &)> InitializeLoc) {
+  transferSmartPointerLikeCachedGet<LatticeT>(
+      GetExpr, SmartPointerLoc, State,
+      [InitializeLoc](QualType T, StorageLocation &Loc) {
+        InitializeLoc(Loc);
+      });
+}
 
 template <typename LatticeT>
 void transferSmartPointerLikeCachedDeref(
     const CallExpr *DerefExpr, RecordStorageLocation *SmartPointerLoc,
     TransferState<LatticeT> &State,
-    llvm::function_ref<void(StorageLocation &)> InitializeLoc) {
+    llvm::function_ref<void(QualType, StorageLocation &)> InitializeLoc) {
   if (State.Env.getStorageLocation(*DerefExpr) != nullptr)
     return;
   if (SmartPointerLoc == nullptr)
@@ -141,11 +167,25 @@ void transferSmartPointerLikeCachedDeref(
   State.Env.setStorageLocation(*DerefExpr, LocForValue);
 }
 
+// This was introduced after the QualType was added to InitializeLoc, so
+// we don't provide a compatibility wrapper.
+template <typename LatticeT>
+void transferSmartPointerLikeConstructor(
+    const CXXConstructExpr *ConstructOperator,
+    RecordStorageLocation *SmartPointerLoc, TransferState<LatticeT> &State,
+    llvm::function_ref<void(QualType, StorageLocation &)> InitializeLoc) {
+  const FunctionDecl *CanonicalCallee =
+      getCanonicalSmartPointerLikeOperatorCalleeForType(
+          ConstructOperator->getType()->getAsCXXRecordDecl());
+  State.Lattice.getOrCreateConstMethodReturnStorageLocation(
+      *SmartPointerLoc, CanonicalCallee, State.Env, InitializeLoc);
+}
+
 template <typename LatticeT>
 void transferSmartPointerLikeCachedGet(
     const CallExpr *GetExpr, RecordStorageLocation *SmartPointerLoc,
     TransferState<LatticeT> &State,
-    llvm::function_ref<void(StorageLocation &)> InitializeLoc) {
+    llvm::function_ref<void(QualType, StorageLocation &)> InitializeLoc) {
   if (SmartPointerLoc == nullptr)
     return;
 

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"
@@ -211,6 +212,16 @@ static auto isStatusConstructor() {
   return cxxConstructExpr(hasType(statusType()));
 }
 
+static auto isNonConstMemberCall() {
+  using namespace ::clang::ast_matchers;  // NOLINT: Too many names
+  return cxxMemberCallExpr(callee(cxxMethodDecl(unless(isConst()))));
+}
+
+static auto isNonConstMemberOperatorCall() {
+  using namespace ::clang::ast_matchers;  // NOLINT: Too many names
+  return cxxOperatorCallExpr(callee(cxxMethodDecl(unless(isConst()))));
+}
+
 static auto
 buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
   return CFGMatchSwitchBuilder<const Environment,
@@ -608,6 +619,63 @@ static void transferStatusConstructor(const CXXConstructExpr *Expr,
     initializeStatus(StatusLoc, State.Env);
 }
 
+static void transferStatusOrReturningCall(const CallExpr* Expr,
+                                          LatticeTransferState& State) {
+  RecordStorageLocation* StatusOrLoc =
+      Expr->isPRValue() ? &State.Env.getResultObjectLocation(*Expr)
+                        : State.Env.get<RecordStorageLocation>(*Expr);
+  if (StatusOrLoc != nullptr &&
+      State.Env.getValue(locForOk(locForStatus(*StatusOrLoc))) == nullptr)
+    initializeStatusOr(*StatusOrLoc, State.Env);
+}
+
+static void handleNonConstMemberCall(const CallExpr* Expr,
+                                     RecordStorageLocation* RecordLoc,
+                                     const MatchFinder::MatchResult& Result,
+                                     LatticeTransferState& State) {
+  if (RecordLoc == nullptr) return;
+  State.Lattice.clearConstMethodReturnValues(*RecordLoc);
+  State.Lattice.clearConstMethodReturnStorageLocations(*RecordLoc);
+
+  if (isStatusOrType(Expr->getType()))
+    transferStatusOrReturningCall(Expr, State);
+}
+static void transferNonConstMemberCall(const CXXMemberCallExpr* Expr,
+                                       const MatchFinder::MatchResult& Result,
+                                       LatticeTransferState& State) {
+  handleNonConstMemberCall(Expr, getImplicitObjectLocation(*Expr, State.Env),
+                           Result, State);
+}
+
+static void transferNonConstMemberOperatorCall(
+    const CXXOperatorCallExpr* Expr, const MatchFinder::MatchResult& Result,
+    LatticeTransferState& State) {
+  auto* RecordLoc = cast_or_null<RecordStorageLocation>(
+      State.Env.getStorageLocation(*Expr->getArg(0)));
+  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;
+}
+
+static void maybeInitLocForCtor(QualType T, StorageLocation &Loc,
+                                Environment &Env) {
+  if (isStatusOrType(T)) {
+    initializeStatusOr(cast<RecordStorageLocation>(Loc), Env);
+  } else if (isStatusType(T)) {
+    initializeStatus(cast<RecordStorageLocation>(Loc), Env);
+  } else {
+    llvm::errs() << T << "\n";
+  }
+}
+
 CFGMatchSwitch<LatticeTransferState>
 buildTransferMatchSwitch(ASTContext &Ctx,
                          CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
@@ -667,6 +735,51 @@ 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, [&](QualType T, StorageLocation &Loc) {
+                  maybeInitLocForCtor(T, Loc, State.Env);
+                });
+          })
+      .CaseOfCFGStmt<CXXConstructExpr>(
+          isSmartPointerLikeConstructor(),
+          [](const CXXConstructExpr *E, const MatchFinder::MatchResult &Result,
+             LatticeTransferState &State) {
+            transferSmartPointerLikeConstructor(
+                E, &State.Env.getResultObjectLocation(*E), State,
+                [&](QualType T, StorageLocation &Loc) {
+                  maybeInitLocForCtor(T, Loc, State.Env);
+                });
+          })
+      .CaseOfCFGStmt<CXXMemberCallExpr>(
+          isSmartPointerLikeValueMethodCall(),
+          [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
+             LatticeTransferState &State) {
+            transferSmartPointerLikeCachedDeref(
+                E, getImplicitObjectLocation(*E, State.Env), State,
+                [&](QualType T, StorageLocation &Loc) {
+                  maybeInitLocForCtor(T, Loc, State.Env);
+                });
+          })
+      .CaseOfCFGStmt<CXXMemberCallExpr>(
+          isSmartPointerLikeGetMethodCall(),
+          [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &Result,
+             LatticeTransferState &State) {
+            transferSmartPointerLikeCachedGet(
+                E, getImplicitObjectLocation(*E, State.Env), State,
+                [&](QualType T, StorageLocation &Loc) {
+                  maybeInitLocForCtor(T, Loc, State.Env);
+                });
+          })
+      .CaseOfCFGStmt<CXXMemberCallExpr>(isNonConstMemberCall(),
+                                        transferNonConstMemberCall)
+      .CaseOfCFGStmt<CXXOperatorCallExpr>(isNonConstMemberOperatorCall(),
+                                          transferNonConstMemberOperatorCall)
       .Build();
 }
 

clang/lib/Analysis/FlowSensitive/SmartPointerAccessorCaching.cpp

@@ -129,6 +129,12 @@ AST_MATCHER(clang::CXXRecordDecl, pointerClass) {
 
 namespace clang::dataflow {
 
+ast_matchers::StatementMatcher isSmartPointerLikeConstructor() {
+  using namespace ast_matchers;
+  return cxxConstructExpr(hasType(hasCanonicalType(qualType(
+      hasDeclaration(cxxRecordDecl(smartPointerClassWithGetOrValue()))))));
+}
+
 ast_matchers::StatementMatcher isSmartPointerLikeOperatorStar() {
   return cxxOperatorCallExpr(
       hasOverloadedOperatorName("*"),
@@ -145,6 +151,12 @@ ast_matchers::StatementMatcher isSmartPointerLikeOperatorArrow() {
                            ofClass(smartPointerClassWithGetOrValue()))));
 }
 
+ast_matchers::StatementMatcher isPointerLikeConstructor() {
+  using namespace ast_matchers;
+  return cxxConstructExpr(hasType(hasCanonicalType(
+      qualType(hasDeclaration(cxxRecordDecl(pointerClass()))))));
+}
+
 ast_matchers::StatementMatcher isPointerLikeOperatorStar() {
   return cxxOperatorCallExpr(
       hasOverloadedOperatorName("*"),
@@ -177,15 +189,8 @@ isSmartPointerLikeGetMethodCall(clang::StringRef MethodName) {
 }
 
 const FunctionDecl *
-getCanonicalSmartPointerLikeOperatorCallee(const CallExpr *CE) {
+getCanonicalSmartPointerLikeOperatorCalleeForType(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 +201,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 getCanonicalSmartPointerLikeOperatorCalleeForType(RD);
+}
+
 } // namespace clang::dataflow

clang/unittests/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp

@@ -307,5 +307,23 @@ TEST_F(CachedConstAccessorsLatticeTest, ProducesNewValueAfterJoinDistinct) {
   EXPECT_NE(ValAfterJoin2, Val3);
 }
 
+TEST_F(CachedConstAccessorsLatticeTest, TypePassed) {
+  CommonTestInputs Inputs;
+  auto *CE = Inputs.CallRef;
+  RecordStorageLocation Loc(Inputs.SType, RecordStorageLocation::FieldToLoc(),
+                            {});
+
+  LatticeT Lattice;
+
+  const FunctionDecl *Callee = CE->getDirectCallee();
+  auto RetType = Callee->getReturnType().getNonReferenceType();
+  auto CheckedInit = [RetType](QualType T, StorageLocation &) {
+    ASSERT_EQ(T, RetType);
+  };
+  ASSERT_NE(Callee, nullptr);
+  Lattice.getOrCreateConstMethodReturnStorageLocation(Loc, Callee, Env,
+                                                      CheckedInit);
+}
+
 } // namespace
 } // namespace clang::dataflow