[analyzer] Add a testing facility for testing relationships between s…

…ymbols. Tests introduced in r329780 was disabled in r342317 because these tests were accidentally testing dump infrastructure, when all they cared about was how symbols relate to each other. So when dump infrastructure changed, tests became annoying to maintain. Add a new feature to ExprInspection: clang_analyzer_denote() and clang_analyzer_explain(). The former adds a notation to a symbol, the latter expresses another symbol in terms of previously denoted symbols. It's currently a bit wonky - doesn't print parentheses and only supports denoting atomic symbols. But it's even more readable that way. Differential Revision: https://reviews.llvm.org/D52133 llvm-svn: 343048
llvm · Sep 25, 2018 · e527df0 · e527df0
1 parent 8dfcd83
commit e527df0
Show file tree

Hide file tree

Showing 4 changed files with 708 additions and 521 deletions.
diff --git a/clang/docs/analyzer/DebugChecks.rst b/clang/docs/analyzer/DebugChecks.rst
@@ -255,6 +255,23 @@ ExprInspection checks
       clang_analyzer_hashDump(x); // expected-warning{{hashed string for x}}
     }
 
+- ``void clang_analyzer_denote(int, const char *);``
+
+  Denotes symbols with strings. A subsequent call to clang_analyzer_express()
+  will expresses another symbol in terms of these string. Useful for testing
+  relationships between different symbols.
+
+  Example usage::
+
+    void foo(int x) {
+      clang_analyzer_denote(x, "$x");
+      clang_analyzer_express(x + 1); // expected-warning{{$x + 1}}
+    }
+
+- ``void clang_analyzer_express(int);``
+
+  See clang_analyzer_denote().
+
 Statistics
 ==========
 

diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -43,6 +43,8 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols,
   void analyzerPrintState(const CallExpr *CE, CheckerContext &C) const;
   void analyzerGetExtent(const CallExpr *CE, CheckerContext &C) const;
   void analyzerHashDump(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerDenote(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerExpress(const CallExpr *CE, CheckerContext &C) const;
 
   typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *,
                                                  CheckerContext &C) const;
@@ -60,6 +62,7 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols,
 }
 
 REGISTER_SET_WITH_PROGRAMSTATE(MarkedSymbols, SymbolRef)
+REGISTER_MAP_WITH_PROGRAMSTATE(DenotedSymbols, SymbolRef, const StringLiteral *)
 
 bool ExprInspectionChecker::evalCall(const CallExpr *CE,
                                      CheckerContext &C) const {
@@ -82,6 +85,8 @@ bool ExprInspectionChecker::evalCall(const CallExpr *CE,
     .Case("clang_analyzer_numTimesReached",
           &ExprInspectionChecker::analyzerNumTimesReached)
     .Case("clang_analyzer_hashDump", &ExprInspectionChecker::analyzerHashDump)
+    .Case("clang_analyzer_denote", &ExprInspectionChecker::analyzerDenote)
+    .Case("clang_analyzer_express", &ExprInspectionChecker::analyzerExpress)
     .Default(nullptr);
 
   if (!Handler)
@@ -264,6 +269,13 @@ void ExprInspectionChecker::checkDeadSymbols(SymbolReaper &SymReaper,
       N = BugNode;
     State = State->remove<MarkedSymbols>(Sym);
   }
+
+  for (auto I : State->get<DenotedSymbols>()) {
+    SymbolRef Sym = I.first;
+    if (!SymReaper.isLive(Sym))
+      State = State->remove<DenotedSymbols>(Sym);
+  }
+
   C.addTransition(State, N);
 }
 
@@ -295,6 +307,92 @@ void ExprInspectionChecker::analyzerHashDump(const CallExpr *CE,
   reportBug(HashContent, C);
 }
 
+void ExprInspectionChecker::analyzerDenote(const CallExpr *CE,
+                                           CheckerContext &C) const {
+  if (CE->getNumArgs() < 2) {
+    reportBug("clang_analyzer_denote() requires a symbol and a string literal",
+              C);
+    return;
+  }
+
+  SymbolRef Sym = C.getSVal(CE->getArg(0)).getAsSymbol();
+  if (!Sym) {
+    reportBug("Not a symbol", C);
+    return;
+  }
+
+  if (!isa<SymbolData>(Sym)) {
+    reportBug("Not an atomic symbol", C);
+    return;
+  }
+
+  const auto *E = dyn_cast<StringLiteral>(CE->getArg(1)->IgnoreParenCasts());
+  if (!E) {
+    reportBug("Not a string literal", C);
+    return;
+  }
+
+  ProgramStateRef State = C.getState();
+
+  C.addTransition(C.getState()->set<DenotedSymbols>(Sym, E));
+}
+
+class SymbolExpressor
+    : public SymExprVisitor<SymbolExpressor, Optional<std::string>> {
+  ProgramStateRef State;
+
+public:
+  SymbolExpressor(ProgramStateRef State) : State(State) {}
+
+  Optional<std::string> VisitSymExpr(const SymExpr *S) {
+    if (const StringLiteral *const *SLPtr = State->get<DenotedSymbols>(S)) {
+      const StringLiteral *SL = *SLPtr;
+      return std::string(SL->getBytes());
+    }
+    return None;
+  }
+
+  Optional<std::string> VisitSymIntExpr(const SymIntExpr *S) {
+    if (auto Str = Visit(S->getLHS()))
+      return (*Str + " " + BinaryOperator::getOpcodeStr(S->getOpcode()) + " " +
+              std::to_string(S->getRHS().getLimitedValue()) +
+              (S->getRHS().isUnsigned() ? "U" : ""))
+          .str();
+    return None;
+  }
+
+  Optional<std::string> VisitSymSymExpr(const SymSymExpr *S) {
+    if (auto Str1 = Visit(S->getLHS()))
+      if (auto Str2 = Visit(S->getRHS()))
+        return (*Str1 + " " + BinaryOperator::getOpcodeStr(S->getOpcode()) +
+                " " + *Str2).str();
+    return None;
+  }
+};
+
+void ExprInspectionChecker::analyzerExpress(const CallExpr *CE,
+                                            CheckerContext &C) const {
+  if (CE->getNumArgs() == 0) {
+    reportBug("clang_analyzer_express() requires a symbol", C);
+    return;
+  }
+
+  SymbolRef Sym = C.getSVal(CE->getArg(0)).getAsSymbol();
+  if (!Sym) {
+    reportBug("Not a symbol", C);
+    return;
+  }
+
+  SymbolExpressor V(C.getState());
+  auto Str = V.Visit(Sym);
+  if (!Str) {
+    reportBug("Unable to express", C);
+    return;
+  }
+
+  reportBug(*Str, C);
+}
+
 void ento::registerExprInspectionChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<ExprInspectionChecker>();
 }
diff --git a/clang/test/Analysis/expr-inspection.cpp b/clang/test/Analysis/expr-inspection.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -x c++ -analyzer-checker=debug.ExprInspection -verify %s
+
+// Self-tests for the debug.ExprInspection checker.
+
+void clang_analyzer_denote(int x, const char *str);
+void clang_analyzer_express(int x);
+
+// Invalid declarations to test sanity checks.
+void clang_analyzer_denote();
+void clang_analyzer_denote(int x);
+void clang_analyzer_express();
+
+void foo(int x, unsigned y) {
+  clang_analyzer_denote(); // expected-warning{{clang_analyzer_denote() requires a symbol and a string literal}}
+  clang_analyzer_express(); // expected-warning{{clang_analyzer_express() requires a symbol}}
+
+  clang_analyzer_denote(x); // expected-warning{{clang_analyzer_denote() requires a symbol and a string literal}}
+  clang_analyzer_express(x); // expected-warning{{Unable to express}}
+
+  clang_analyzer_denote(x, "$x");
+  clang_analyzer_denote(y, "$y");
+  clang_analyzer_express(x + y); // expected-warning{{$x + $y}}
+
+  clang_analyzer_denote(1, "$z");     // expected-warning{{Not a symbol}}
+  clang_analyzer_express(1);     // expected-warning{{Not a symbol}}
+
+  clang_analyzer_denote(x + 1, "$w"); // expected-warning{{Not an atomic symbol}}
+  clang_analyzer_express(x + 1); // expected-warning{{$x + 1}}
+  clang_analyzer_express(y + 1); // expected-warning{{$y + 1U}}
+}