[clang] ExprEngine::performTrivialCopy triggers checkLocation (PR #129016)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (T-Gruber)

<details>
<summary>Changes</summary>

The triggered callbacks for the default copy constructed instance and the instance used for initialization now behave in the same way. The LHS already calls checkBind. To keep this consistent, checkLocation is now triggered accordingly for the RHS. 
Further details on the previous discussion: https://discourse.llvm.org/t/checklocation-for-implicitcastexpr-of-kind-ck-noop/84729

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


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (+7-2) 
- (modified) clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp (+54) 


``````````diff
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index f7020da2e6da2..47888cf9689c7 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -69,6 +69,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
 
   assert(ThisRD);
   SVal V = Call.getArgSVal(0);
+  const Expr *VExpr = Call.getArgExpr(0);
 
   // If the value being copied is not unknown, load from its location to get
   // an aggregate rvalue.
@@ -76,7 +77,11 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
     V = Pred->getState()->getSVal(*L);
   else
     assert(V.isUnknownOrUndef());
-  evalBind(Dst, CallExpr, Pred, ThisVal, V, true);
+  
+  ExplodedNodeSet Tmp;
+  evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V, true);
+  for (ExplodedNode *N : Tmp)
+    evalBind(Dst, CallExpr, N, ThisVal, V, true);
 
   PostStmt PS(CallExpr, LCtx);
   for (ExplodedNode *N : Dst) {
@@ -1199,4 +1204,4 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred,
 
   // FIXME: Move all post/pre visits to ::Visit().
   getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this);
-}
+}
\ No newline at end of file
diff --git a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
index a8579f9d0f90c..19de9919701a3 100644
--- a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
+++ b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp
@@ -12,6 +12,8 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/Support/Casting.h"
 #include "gtest/gtest.h"
 
 using namespace clang;
@@ -27,6 +29,14 @@ void emitErrorReport(CheckerContext &C, const BugType &Bug,
   }
 }
 
+inline std::string getMemRegionName(const SVal &Val) {
+  if (auto MemVal = llvm::dyn_cast<loc::MemRegionVal>(Val))
+    return MemVal->getRegion()->getDescriptiveName(false);
+  if (auto ComVal = llvm::dyn_cast<nonloc::LazyCompoundVal>(Val))
+    return ComVal->getRegion()->getDescriptiveName(false);
+  return "";
+}
+
 #define CREATE_EXPR_ENGINE_CHECKER(CHECKER_NAME, CALLBACK, STMT_TYPE,          \
                                    BUG_NAME)                                   \
   class CHECKER_NAME : public Checker<check::CALLBACK<STMT_TYPE>> {            \
@@ -44,6 +54,21 @@ CREATE_EXPR_ENGINE_CHECKER(ExprEngineVisitPreChecker, PreStmt, GCCAsmStmt,
 CREATE_EXPR_ENGINE_CHECKER(ExprEngineVisitPostChecker, PostStmt, GCCAsmStmt,
                            "GCCAsmStmtBug")
 
+class MemAccessChecker : public Checker<check::Location, check::Bind> {
+public:
+  void checkLocation(const SVal &Loc, bool IsLoad, const Stmt *S,
+                     CheckerContext &C) const {
+    emitErrorReport(C, Bug, "checkLocation: Loc = " + getMemRegionName(Loc));
+  }
+
+  void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const {
+    emitErrorReport(C, Bug, "checkBind: Loc = " + getMemRegionName(Loc));
+  }
+
+private:
+  const BugType Bug{this, "MemAccess"};
+};
+
 void addExprEngineVisitPreChecker(AnalysisASTConsumer &AnalysisConsumer,
                                   AnalyzerOptions &AnOpts) {
   AnOpts.CheckersAndPackages = {{"ExprEngineVisitPreChecker", true}};
@@ -62,6 +87,15 @@ void addExprEngineVisitPostChecker(AnalysisASTConsumer &AnalysisConsumer,
   });
 }
 
+void addMemAccessChecker(AnalysisASTConsumer &AnalysisConsumer,
+                         AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"MemAccessChecker", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+    Registry.addChecker<MemAccessChecker>("MemAccessChecker", "Desc",
+                                          "DocsURI");
+  });
+}
+
 TEST(ExprEngineVisitTest, checkPreStmtGCCAsmStmt) {
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode<addExprEngineVisitPreChecker>(R"(
@@ -84,4 +118,24 @@ TEST(ExprEngineVisitTest, checkPostStmtGCCAsmStmt) {
   EXPECT_EQ(Diags, "ExprEngineVisitPostChecker: checkPostStmt<GCCAsmStmt>\n");
 }
 
+TEST(ExprEngineVisitTest, checkLocationAndBind) {
+  std::string Diags;
+  EXPECT_TRUE(runCheckerOnCode<addMemAccessChecker>(R"(
+    class MyClass{
+    public:
+      int Value;
+    };
+    extern MyClass MyClassWrite, MyClassRead; 
+    void top() {
+      MyClassWrite = MyClassRead;
+    }
+  )",
+                                                    Diags));
+
+  std::string RHSMsg = "checkLocation: Loc = MyClassRead";
+  std::string LHSMsg = "checkBind: Loc = MyClassWrite";
+  EXPECT_NE(Diags.find(RHSMsg), std::string::npos);
+  EXPECT_NE(Diags.find(LHSMsg), std::string::npos);
+}
+
 } // namespace

``````````

</details>


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