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

[NoQ]

Summary

If a pointer cast fails (evaluates to an UnknownVal) and such cast is the last use of the pointer, the pointer is no longer referenced by the program state and a leak is (mis-)diagnosed. Produce pointer escape (but not invalidation) when the cast fails to avoid such false positives.

[xazax.hun]

I am in favour of this approach. This is what I suggested back than in https://reviews.llvm.org/D23014 but it was somehow overlooked.

[NoQ]

Hmm, yeah, indeed, i must have overlooked it, nice one :) I'll see if i can fix the other place as well.

[NoQ]

In D45698#1069121, @xazax.hun wrote:

I am in favour of this approach. This is what I suggested back than in https://reviews.llvm.org/D23014 but it was somehow overlooked.

I'll try to follow-up on this.

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

protected:
   ProgramStateRef notifyCheckersOfPointerEscape(
                            ProgramStateRef State,
                            const InvalidatedSymbols *Invalidated,
                            ArrayRef<const MemRegion *> ExplicitRegions,
                            ArrayRef<const MemRegion *> Regions,
                            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.
   // FIXME: Comment on the meaning of the arguments, when 'St' may not
   // be the same as Pred->state, and when 'location' may not be the
   // same as state->getLValue(Ex).
   /// Simulate a read of the result of Ex.
   void evalLoad(ExplodedNodeSet &Dst,

lib/StaticAnalyzer/Core/ExprEngine.cpp

if (!State->contains<InitializedTemporaries>(Key)) {
       // those, we currently skip the CXXBindTemporaryExpr but rely on adding
       // temporary destructor nodes.
       State = State->set<InitializedTemporaries>(Key, nullptr);
     }
     StmtBldr.generateNode(BTE, Node, State);
   }
 }
 
-namespace {
-
+ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
+                                        PointerEscapeKind K) const {
   class CollectReachableSymbolsCallback final : public SymbolVisitor {
     InvalidatedSymbols Symbols;
 
   public:
     explicit CollectReachableSymbolsCallback(ProgramStateRef State) {}
 
     const InvalidatedSymbols &getSymbols() const { return Symbols; }
 
     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) {
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 S->getLocStart(),
                                 "Error evaluating statement");
   ExplodedNodeSet Dst;
   StmtNodeBuilder Bldr(Pred, DstTop, *currBldrCtx);
case Expr::ObjCBoxedExprClass: {
 
         // Escape pointers passed into the list, unless it's an ObjC boxed
         // expression which is not a boxable C structure.
         if (!(isa<ObjCBoxedExpr>(Ex) &&
               !cast<ObjCBoxedExpr>(Ex)->getSubExpr()
                                       ->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);
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
       Bldr.addNodes(Dst);
       break;
ProgramStateRef ExprEngine::processPointerEscapedOnBind(ProgramStateRef State,
   // If our store can represent the binding and we aren't storing to something
   // that doesn't have local storage then just return and have the simulation
   // state continue as is.
   if (!escapes)
     return 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;
 }
 
 ProgramStateRef
 ExprEngine::notifyCheckersOfPointerEscape(ProgramStateRef State,
     const InvalidatedSymbols *Invalidated,
     ArrayRef<const MemRegion *> ExplicitRegions,
     ArrayRef<const MemRegion *> Regions,

lib/StaticAnalyzer/Core/ExprEngineC.cpp

void ExprEngine::VisitBlockExpr(const BlockExpr *BE, ExplodedNode *Pred,
   getCheckerManager().runCheckersForPostStmt(Dst, Tmp, BE, *this);
 }
 
 ProgramStateRef ExprEngine::handleLValueBitCast(
     ProgramStateRef state, const Expr* Ex, const LocationContext* LCtx,
     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;
 }
 
 ProgramStateRef ExprEngine::handleLVectorSplat(
     ProgramStateRef state, const LocationContext* LCtx, const CastExpr* CastE,
     StmtNodeBuilder &Bldr, ExplodedNode* Pred) {

test/Analysis/malloc.mm

void test12365078_nested(unichar *characters) {
   }
 }
 
 void test12365078_check_positive() {
   unichar *characters = (unichar*)malloc(12);
   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;
+}

test/Analysis/pr22954.c

int f29(int i, int j, int k, int l, int m) {
   clang_analyzer_eval(m29[1].s3[0] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[1].s3[1] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[1].s3[2] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[1].s3[3] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m29[i].s3[0] == 1); // expected-warning{{UNKNOWN}}
   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;
 }
 
 // Test unions' fields.
 union uu {
   char x;
   char s1[4];
 };