From 20cee7d894a7a9e28901ba02906b2a36967bca07 Mon Sep 17 00:00:00 2001
From: Rafael Stahl <r.stahl@tum.de>
Date: Mon, 7 Jan 2019 15:07:01 +0000
Subject: [PATCH] [analyzer] Pass the correct loc Expr from VisitIncDecOp to
 evalStore

Summary: The LocationE parameter of evalStore is documented as "The location expression that is stored to". When storing from an increment / decrement operator this was not satisfied. In user code this causes an inconsistency between the SVal and Stmt parameters of checkLocation.

Reviewers: NoQ, dcoughlin, george.karpenkov

Reviewed By: NoQ

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, cfe-commits

Differential Revision: https://reviews.llvm.org/D55701

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@350528 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/StaticAnalyzer/Core/ExprEngineC.cpp       |  4 +-
 .../RegisterCustomCheckersTest.cpp            | 55 +++++++++++++------
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 7d47cf4f33..b980628878 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1052,7 +1052,7 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U,
       // Perform the store, so that the uninitialized value detection happens.
       Bldr.takeNodes(*I);
       ExplodedNodeSet Dst3;
-      evalStore(Dst3, U, U, *I, state, loc, V2_untested);
+      evalStore(Dst3, U, Ex, *I, state, loc, V2_untested);
       Bldr.addNodes(Dst3);
 
       continue;
@@ -1120,7 +1120,7 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U,
     // Perform the store.
     Bldr.takeNodes(*I);
     ExplodedNodeSet Dst3;
-    evalStore(Dst3, U, U, *I, state, loc, Result);
+    evalStore(Dst3, U, Ex, *I, state, loc, Result);
     Bldr.addNodes(Dst3);
   }
   Dst.insert(Dst2);
diff --git a/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp b/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
index 0dbf3ae451..568a719e33 100644
--- a/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ b/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -21,16 +21,7 @@ namespace clang {
 namespace ento {
 namespace {
 
-class CustomChecker : public Checker<check::ASTCodeBody> {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
-                        BugReporter &BR) const {
-    BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
-                       "Custom diagnostic description",
-                       PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
-  }
-};
-
+template <typename CheckerT>
 class TestAction : public ASTFrontendAction {
   class DiagConsumer : public PathDiagnosticConsumer {
     llvm::raw_ostream &Output;
@@ -59,23 +50,55 @@ public:
     Compiler.getAnalyzerOpts()->CheckersControlList = {
         {"custom.CustomChecker", true}};
     AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
-      Registry.addChecker<CustomChecker>("custom.CustomChecker", "Description",
-                                         "");
+      Registry.addChecker<CheckerT>("custom.CustomChecker", "Description", "");
     });
     return std::move(AnalysisConsumer);
   }
 };
 
+template <typename CheckerT>
+bool runCheckerOnCode(const std::string &Code, std::string &Diags) {
+  llvm::raw_string_ostream OS(Diags);
+  return tooling::runToolOnCode(new TestAction<CheckerT>(OS), Code);
+}
+template <typename CheckerT>
+bool runCheckerOnCode(const std::string &Code) {
+  std::string Diags;
+  return runCheckerOnCode<CheckerT>(Code, Diags);
+}
+
+
+class CustomChecker : public Checker<check::ASTCodeBody> {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+                        BugReporter &BR) const {
+    BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+                       "Custom diagnostic description",
+                       PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  {
-    llvm::raw_string_ostream OS(Diags);
-    EXPECT_TRUE(tooling::runToolOnCode(new TestAction(OS), "void f() {;}"));
-  }
+  EXPECT_TRUE(runCheckerOnCode<CustomChecker>("void f() {;}", Diags));
   EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
 }
 
+class LocIncDecChecker : public Checker<check::Location> {
+public:
+  void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
+                     CheckerContext &C) const {
+    auto UnaryOp = dyn_cast<UnaryOperator>(S);
+    if (UnaryOp && !IsLoad)
+      EXPECT_FALSE(UnaryOp->isIncrementOp());
+  }
+};
+
+TEST(RegisterCustomCheckers, CheckLocationIncDec) {
+  EXPECT_TRUE(
+      runCheckerOnCode<LocIncDecChecker>("void f() { int *p; (*p)++; }"));
+}
+
 }
 }
 }
-- 
2.40.0