[analyzer] When we fail to evaluate a pointer cast, escape the pointer.

If a pointer cast fails (evaluates to an UnknownVal, i.e. not implemented in the
analyzer) and such cast is in fact the last use of the pointer, the pointer
symbol is no longer referenced by the program state and a leak is
(mis-)diagnosed.

"Escape" the pointer upon a failed cast, i.e. inform the checker that we can no
longer reliably track it.

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@330380 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 7d3f117eb2157d9c95c22191c9df24033c9a2575..ba42d816d8e5ef0d40ae87dbafde46d885f155b1 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -604,6 +604,11 @@ protected:
                            const CallEvent *Call,
                            RegionAndSymbolInvalidationTraits &ITraits) override;
 
+  /// A simple wrapper when you only need to notify checkers of pointer-escape
+  /// of a single value.
+  ProgramStateRef escapeValue(ProgramStateRef State, SVal V,
+                              PointerEscapeKind K) const;
+
 public:
   // FIXME: 'tag' should be removed, and a LocationContext should be used
   // instead.

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 51c6f25e98c65460fec4eb217ac5646b8d1bd4ef..1e0bfcea2d74089e706c70f732e6332bbe095900 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1231,23 +1231,27 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
   }
 }
 
-namespace {
+ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
+                                        PointerEscapeKind K) const {
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+    InvalidatedSymbols Symbols;
 
-class CollectReachableSymbolsCallback final : public SymbolVisitor {
-  InvalidatedSymbols Symbols;
+  public:
+    explicit CollectReachableSymbolsCallback(ProgramStateRef State) {}
 
-public:
-  explicit CollectReachableSymbolsCallback(ProgramStateRef State) {}
+    const InvalidatedSymbols &getSymbols() const { return Symbols; }
 
-  const InvalidatedSymbols &getSymbols() const { return Symbols; }
-
-  bool VisitSymbol(SymbolRef Sym) override {
-    Symbols.insert(Sym);
-    return true;
-  }
-};
+    bool VisitSymbol(SymbolRef Sym) override {
+      Symbols.insert(Sym);
+      return true;
+    }
+  };
 
-} // namespace
+  const CollectReachableSymbolsCallback &Scanner =
+      State->scanReachableSymbols<CollectReachableSymbolsCallback>(V);
+  return getCheckerManager().runCheckersForPointerEscape(
+      State, Scanner.getSymbols(), /*CallEvent*/ nullptr, K, nullptr);
+}
 
 void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
                        ExplodedNodeSet &DstTop) {
@@ -1529,17 +1533,8 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
                                       ->getType()->isRecordType()))
           for (auto Child : Ex->children()) {
             assert(Child);
-
             SVal Val = State->getSVal(Child, LCtx);
-
-            CollectReachableSymbolsCallback Scanner =
-                State->scanReachableSymbols<CollectReachableSymbolsCallback>(
-                    Val);
-            const InvalidatedSymbols &EscapedSymbols = Scanner.getSymbols();
-
-            State = getCheckerManager().runCheckersForPointerEscape(
-                State, EscapedSymbols,
-                /*CallEvent*/ nullptr, PSK_EscapeOther, nullptr);
+            State = escapeValue(State, Val, PSK_EscapeOther);
           }
 
         Bldr2.generateNode(S, N, State);
@@ -2759,15 +2754,7 @@ ProgramStateRef ExprEngine::processPointerEscapedOnBind(ProgramStateRef State,
 
   // Otherwise, find all symbols referenced by 'val' that we are tracking
   // and stop tracking them.
-  CollectReachableSymbolsCallback Scanner =
-      State->scanReachableSymbols<CollectReachableSymbolsCallback>(Val);
-  const InvalidatedSymbols &EscapedSymbols = Scanner.getSymbols();
-  State = getCheckerManager().runCheckersForPointerEscape(State,
-                                                          EscapedSymbols,
-                                                          /*CallEvent*/ nullptr,
-                                                          PSK_EscapeOnBind,
-                                                          nullptr);
-
+  State = escapeValue(State, Val, PSK_EscapeOnBind);
   return State;
 }
 

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 55ee2cefc9156294cefe6e8f878ecee64614421e..5a306a5c5f22905a3396a13bc8a53be715e3bd07 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -258,12 +258,15 @@ ProgramStateRef ExprEngine::handleLValueBitCast(
     QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr,
     ExplodedNode* Pred) {
   // Delegate to SValBuilder to process.
-  SVal V = state->getSVal(Ex, LCtx);
-  V = svalBuilder.evalCast(V, T, ExTy);
+  SVal OrigV = state->getSVal(Ex, LCtx);
+  SVal V = svalBuilder.evalCast(OrigV, T, ExTy);
   // Negate the result if we're treating the boolean as a signed i1
   if (CastE->getCastKind() == CK_BooleanToSignedIntegral)
     V = evalMinus(V);
   state = state->BindExpr(CastE, LCtx, V);
+  if (V.isUnknown() && !OrigV.isUnknown()) {
+    state = escapeValue(state, OrigV, PSK_EscapeOther);
+  }
   Bldr.generateNode(CastE, Pred, state);
 
   return state;

diff --git a/test/Analysis/malloc.mm b/test/Analysis/malloc.mm
index e3daa858be87ed10689cc6faecec8f97d9425736..d7bfbf3f34f33a96bbb31ca957263caba5486103 100644 (file)
--- a/test/Analysis/malloc.mm
+++ b/test/Analysis/malloc.mm
@@ -320,3 +320,13 @@ void test12365078_check_positive() {
   NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
   if (string) free(characters); // expected-warning{{Attempt to free non-owned memory}}
 }
+
+void *test_reinterpret_cast_to_block() {
+  // Used to leak because the pointer was disappearing
+  // during the reinterpret_cast.
+  using BlockPtrTy = void (^)();
+  struct Block {};
+  Block* block = static_cast<Block*>(malloc(sizeof(Block)));
+  BlockPtrTy blockPtr = reinterpret_cast<BlockPtrTy>(block); // no-warning
+  return blockPtr;
+}

diff --git a/test/Analysis/pr22954.c b/test/Analysis/pr22954.c
index 64a00c5ec0877d78de7dbb2155c435ec8d81aeb1..b4273c0a8943bd033b5863e2a62d66066d7b6b49 100644 (file)
--- a/test/Analysis/pr22954.c
+++ b/test/Analysis/pr22954.c
@@ -624,9 +624,10 @@ int f29(int i, int j, int k, int l, int m) {
   clang_analyzer_eval(m29[i].s3[1] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[i].s3[2] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[i].s3[3] == 1); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(m29[j].s3[k] == 1); // expected-warning{{TRUE}}\
-  expected-warning{{Potential leak of memory pointed to by field 's4'}}
+  clang_analyzer_eval(m29[j].s3[k] == 1); // expected-warning{{TRUE}}
   clang_analyzer_eval(l29->s1[m] == 2); // expected-warning{{UNKNOWN}}
+  // FIXME: Should warn that m29[i].s4 leaks. But not on the previous line,
+  // because l29 and m29 alias.
   return 0;
 }