[analyzer] Fix the "Zombie Symbols" bug.

It's an old bug that consists in stale references to symbols remaining in the
GDM if they disappear from other program state sections as a result of any
operation that isn't the actual dead symbol collection. The most common example
here is:

   FILE *fp = fopen("myfile.txt", "w");
   fp = 0; // leak of file descriptor

In this example the leak were not detected previously because the symbol
disappears from the public part of the program state due to evaluating
the assignment. For that reason the checker never receives a notification
that the symbol is dead, and never reports a leak.

This patch not only causes leak false negatives, but also a number of other
problems, including false positives on some checkers.

What's worse, even though the program state contains a finite number of symbols,
the set of symbols that dies is potentially infinite. This means that is
impossible to compute the set of all dead symbols to pass off to the checkers
for cleaning up their part of the GDM.

No longer compute the dead set at all. Disallow iterating over dead symbols.
Disallow querying if any symbols are dead. Remove the API for marking symbols
as dead, as it is no longer necessary. Update checkers accordingly.

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


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

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

@@ -198,7 +198,7 @@ public:
     auto &CZFactory = State->get_context<ConstraintSMT>();
 
     for (auto I = CZ.begin(), E = CZ.end(); I != E; ++I) {
-      if (SymReaper.maybeDead(I->first))
+      if (SymReaper.isDead(I->first))
         CZ = CZFactory.remove(CZ, *I);
     }
 

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

@@ -558,7 +558,6 @@ class SymbolReaper {
 
   SymbolMapTy TheLiving;
   SymbolSetTy MetadataInUse;
-  SymbolSetTy TheDead;
 
   RegionSetTy RegionRoots;
 
@@ -603,21 +602,6 @@ public:
   /// symbol marking has occurred, i.e. in the MarkLiveSymbols callback.
   void markInUse(SymbolRef sym);
 
-  /// If a symbol is known to be live, marks the symbol as live.
-  ///
-  ///  Otherwise, if the symbol cannot be proven live, it is marked as dead.
-  ///  Returns true if the symbol is dead, false if live.
-  bool maybeDead(SymbolRef sym);
-
-  using dead_iterator = SymbolSetTy::const_iterator;
-
-  dead_iterator dead_begin() const { return TheDead.begin(); }
-  dead_iterator dead_end() const { return TheDead.end(); }
-
-  bool hasDeadSymbols() const {
-    return !TheDead.empty();
-  }
-
   using region_iterator = RegionSetTy::const_iterator;
 
   region_iterator region_begin() const { return RegionRoots.begin(); }
@@ -626,9 +610,9 @@ public:
   /// Returns whether or not a symbol has been confirmed dead.
   ///
   /// This should only be called once all marking of dead symbols has completed.
-  /// (For checkers, this means only in the evalDeadSymbols callback.)
-  bool isDead(SymbolRef sym) const {
-    return TheDead.count(sym);
+  /// (For checkers, this means only in the checkDeadSymbols callback.)
+  bool isDead(SymbolRef sym) {
+    return !isLive(sym);
   }
 
   void markLive(const MemRegion *region);

lib/StaticAnalyzer/Checkers/CStringChecker.cpp

@@ -2385,9 +2385,6 @@ void CStringChecker::checkLiveSymbols(ProgramStateRef state,
 
 void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
     CheckerContext &C) const {
-  if (!SR.hasDeadSymbols())
-    return;
-
   ProgramStateRef state = C.getState();
   CStringLengthTy Entries = state->get<CStringLength>();
   if (Entries.isEmpty())

lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

@@ -123,11 +123,6 @@ void DynamicTypePropagation::checkDeadSymbols(SymbolReaper &SR,
     }
   }
 
-  if (!SR.hasDeadSymbols()) {
-    C.addTransition(State);
-    return;
-  }
-
   MostSpecializedTypeArgsMapTy TyArgMap =
       State->get<MostSpecializedTypeArgsMap>();
   for (MostSpecializedTypeArgsMapTy::iterator I = TyArgMap.begin(),

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

@@ -100,9 +100,6 @@ void MPIChecker::checkUnmatchedWaits(const CallEvent &PreCallEvent,
 
 void MPIChecker::checkMissingWaits(SymbolReaper &SymReaper,
                                    CheckerContext &Ctx) const {
-  if (!SymReaper.hasDeadSymbols())
-    return;
-
   ProgramStateRef State = Ctx.getState();
   const auto &Requests = State->get<RequestMap>();
   if (Requests.isEmpty())

lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

@@ -16,6 +16,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
@@ -29,6 +30,7 @@ namespace {
 class MacOSKeychainAPIChecker : public Checker<check::PreStmt<CallExpr>,
                                                check::PostStmt<CallExpr>,
                                                check::DeadSymbols,
+                                               check::PointerEscape,
                                                eval::Assume> {
   mutable std::unique_ptr<BugType> BT;
 
@@ -58,6 +60,10 @@ public:
   void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
   void checkPostStmt(const CallExpr *S, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
+  ProgramStateRef checkPointerEscape(ProgramStateRef State,
+                                     const InvalidatedSymbols &Escaped,
+                                     const CallEvent *Call,
+                                     PointerEscapeKind Kind) const;
   ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
                              bool Assumption) const;
   void printState(raw_ostream &Out, ProgramStateRef State,
@@ -570,6 +576,44 @@ void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR,
   C.addTransition(State, N);
 }
 
+ProgramStateRef MacOSKeychainAPIChecker::checkPointerEscape(
+    ProgramStateRef State, const InvalidatedSymbols &Escaped,
+    const CallEvent *Call, PointerEscapeKind Kind) const {
+  // FIXME: This branch doesn't make any sense at all, but it is an overfitted
+  // replacement for a previous overfitted code that was making even less sense.
+  if (!Call || Call->getDecl())
+    return State;
+
+  for (auto I : State->get<AllocatedData>()) {
+    SymbolRef Sym = I.first;
+    if (Escaped.count(Sym))
+      State = State->remove<AllocatedData>(Sym);
+
+    // This checker is special. Most checkers in fact only track symbols of
+    // SymbolConjured type, eg. symbols returned from functions such as
+    // malloc(). This checker tracks symbols returned as out-parameters.
+    //
+    // When a function is evaluated conservatively, the out-parameter's pointee
+    // base region gets invalidated with a SymbolConjured. If the base region is
+    // larger than the region we're interested in, the value we're interested in
+    // would be SymbolDerived based on that SymbolConjured. However, such
+    // SymbolDerived will never be listed in the Escaped set when the base
+    // region is invalidated because ExprEngine doesn't know which symbols
+    // were derived from a given symbol, while there can be infinitely many
+    // valid symbols derived from any given symbol.
+    //
+    // Hence the extra boilerplate: remove the derived symbol when its parent
+    // symbol escapes.
+    //
+    if (const auto *SD = dyn_cast<SymbolDerived>(Sym)) {
+      SymbolRef ParentSym = SD->getParentSymbol();
+      if (Escaped.count(ParentSym))
+        State = State->remove<AllocatedData>(Sym);
+    }
+  }
+  return State;
+}
+
 std::shared_ptr<PathDiagnosticPiece>
 MacOSKeychainAPIChecker::SecKeychainBugVisitor::VisitNode(
     const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) {

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -2345,9 +2345,6 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N,
 void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                      CheckerContext &C) const
 {
-  if (!SymReaper.hasDeadSymbols())
-    return;
-
   ProgramStateRef state = C.getState();
   RegionStateTy RS = state->get<RegionState>();
   RegionStateTy::Factory &F = state->get_context<RegionState>();

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

@@ -446,9 +446,6 @@ void NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg,
 /// Cleaning up the program state.
 void NullabilityChecker::checkDeadSymbols(SymbolReaper &SR,
                                           CheckerContext &C) const {
-  if (!SR.hasDeadSymbols())
-    return;
-
   ProgramStateRef State = C.getState();
   NullabilityMapTy Nullabilities = State->get<NullabilityMap>();
   for (NullabilityMapTy::iterator I = Nullabilities.begin(),

lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

@@ -1442,20 +1442,18 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper,
   SmallVector<SymbolRef, 10> Leaked;
 
   // Update counts from autorelease pools
-  for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(),
-       E = SymReaper.dead_end(); I != E; ++I) {
-    SymbolRef Sym = *I;
-    if (const RefVal *T = B.lookup(Sym)){
-      // Use the symbol as the tag.
-      // FIXME: This might not be as unique as we would like.
+  for (const auto &I: state->get<RefBindings>()) {
+    SymbolRef Sym = I.first;
+    if (SymReaper.isDead(Sym)) {
       static CheckerProgramPointTag Tag(this, "DeadSymbolAutorelease");
-      state = handleAutoreleaseCounts(state, Pred, &Tag, C, Sym, *T);
+      const RefVal &V = I.second;
+      state = handleAutoreleaseCounts(state, Pred, &Tag, C, Sym, V);
       if (!state)
         return;
 
       // Fetch the new reference count from the state, and use it to handle
       // this symbol.
-      state = handleSymbolDeath(state, *I, *getRefBinding(state, Sym), Leaked);
+      state = handleSymbolDeath(state, Sym, *getRefBinding(state, Sym), Leaked);
     }
   }
 

lib/StaticAnalyzer/Checkers/StreamChecker.cpp

@@ -383,26 +383,26 @@ ProgramStateRef StreamChecker::CheckDoubleClose(const CallExpr *CE,
 
 void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                      CheckerContext &C) const {
+  ProgramStateRef state = C.getState();
+
   // TODO: Clean up the state.
-  for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(),
-         E = SymReaper.dead_end(); I != E; ++I) {
-    SymbolRef Sym = *I;
-    ProgramStateRef state = C.getState();
-    const StreamState *SS = state->get<StreamMap>(Sym);
-    if (!SS)
+  const StreamMapTy &Map = state->get<StreamMap>();
+  for (const auto &I: Map) {
+    SymbolRef Sym = I.first;
+    const StreamState &SS = I.second;
+    if (!SymReaper.isDead(Sym) || !SS.isOpened())
       continue;
 
-    if (SS->isOpened()) {
-      ExplodedNode *N = C.generateErrorNode();
-      if (N) {
-        if (!BT_ResourceLeak)
-          BT_ResourceLeak.reset(new BuiltinBug(
-              this, "Resource Leak",
-              "Opened File never closed. Potential Resource leak."));
-        C.emitReport(llvm::make_unique<BugReport>(
-            *BT_ResourceLeak, BT_ResourceLeak->getDescription(), N));
-      }
-    }
+    ExplodedNode *N = C.generateErrorNode();
+    if (!N)
+      return;
+
+    if (!BT_ResourceLeak)
+      BT_ResourceLeak.reset(
+          new BuiltinBug(this, "Resource Leak",
+                         "Opened File never closed. Potential Resource leak."));
+    C.emitReport(llvm::make_unique<BugReport>(
+        *BT_ResourceLeak, BT_ResourceLeak->getDescription(), N));
   }
 }
 

lib/StaticAnalyzer/Core/Environment.cpp

@@ -193,11 +193,6 @@ EnvironmentManager::removeDeadBindings(Environment Env,
 
       // Mark all symbols in the block expr's value live.
       RSScaner.scan(X);
-      continue;
-    } else {
-      SymExpr::symbol_iterator SI = X.symbol_begin(), SE = X.symbol_end();
-      for (; SI != SE; ++SI)
-        SymReaper.maybeDead(*SI);
     }
   }
 

lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -675,44 +675,35 @@ void ExprEngine::removeDead(ExplodedNode *Pred, ExplodedNodeSet &Out,
   // Process any special transfer function for dead symbols.
   // A tag to track convenience transitions, which can be removed at cleanup.
   static SimpleProgramPointTag cleanupTag(TagProviderName, "Clean Node");
-  if (!SymReaper.hasDeadSymbols()) {
-    // Generate a CleanedNode that has the environment and store cleaned
-    // up. Since no symbols are dead, we can optimize and not clean out
-    // the constraint manager.
-    StmtNodeBuilder Bldr(Pred, Out, *currBldrCtx);
-    Bldr.generateNode(DiagnosticStmt, Pred, CleanedState, &cleanupTag, K);
-
-  } else {
-    // Call checkers with the non-cleaned state so that they could query the
-    // values of the soon to be dead symbols.
-    ExplodedNodeSet CheckedSet;
-    getCheckerManager().runCheckersForDeadSymbols(CheckedSet, Pred, SymReaper,
-                                                  DiagnosticStmt, *this, K);
-
-    // For each node in CheckedSet, generate CleanedNodes that have the
-    // environment, the store, and the constraints cleaned up but have the
-    // user-supplied states as the predecessors.
-    StmtNodeBuilder Bldr(CheckedSet, Out, *currBldrCtx);
-    for (const auto I : CheckedSet) {
-      ProgramStateRef CheckerState = I->getState();
-
-      // The constraint manager has not been cleaned up yet, so clean up now.
-      CheckerState = getConstraintManager().removeDeadBindings(CheckerState,
-                                                               SymReaper);
-
-      assert(StateMgr.haveEqualEnvironments(CheckerState, Pred->getState()) &&
-        "Checkers are not allowed to modify the Environment as a part of "
-        "checkDeadSymbols processing.");
-      assert(StateMgr.haveEqualStores(CheckerState, Pred->getState()) &&
-        "Checkers are not allowed to modify the Store as a part of "
-        "checkDeadSymbols processing.");
-
-      // Create a state based on CleanedState with CheckerState GDM and
-      // generate a transition to that state.
-      ProgramStateRef CleanedCheckerSt =
+  // Call checkers with the non-cleaned state so that they could query the
+  // values of the soon to be dead symbols.
+  ExplodedNodeSet CheckedSet;
+  getCheckerManager().runCheckersForDeadSymbols(CheckedSet, Pred, SymReaper,
+                                                DiagnosticStmt, *this, K);
+
+  // For each node in CheckedSet, generate CleanedNodes that have the
+  // environment, the store, and the constraints cleaned up but have the
+  // user-supplied states as the predecessors.
+  StmtNodeBuilder Bldr(CheckedSet, Out, *currBldrCtx);
+  for (const auto I : CheckedSet) {
+    ProgramStateRef CheckerState = I->getState();
+
+    // The constraint manager has not been cleaned up yet, so clean up now.
+    CheckerState =
+        getConstraintManager().removeDeadBindings(CheckerState, SymReaper);
+
+    assert(StateMgr.haveEqualEnvironments(CheckerState, Pred->getState()) &&
+           "Checkers are not allowed to modify the Environment as a part of "
+           "checkDeadSymbols processing.");
+    assert(StateMgr.haveEqualStores(CheckerState, Pred->getState()) &&
+           "Checkers are not allowed to modify the Store as a part of "
+           "checkDeadSymbols processing.");
+
+    // Create a state based on CleanedState with CheckerState GDM and
+    // generate a transition to that state.
+    ProgramStateRef CleanedCheckerSt =
         StateMgr.getPersistentStateWithGDM(CleanedState, CheckerState);
-      Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, &cleanupTag, K);
-    }
+    Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, &cleanupTag, K);
   }
 }
 

lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

@@ -399,7 +399,7 @@ RangeConstraintManager::removeDeadBindings(ProgramStateRef State,
 
   for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) {
     SymbolRef Sym = I.getKey();
-    if (SymReaper.maybeDead(Sym)) {
+    if (SymReaper.isDead(Sym)) {
       Changed = true;
       CR = CRFactory.remove(CR, Sym);
     }

lib/StaticAnalyzer/Core/RegionStore.cpp

@@ -2571,24 +2571,9 @@ StoreRef RegionStoreManager::removeDeadBindings(Store store,
     const MemRegion *Base = I.getKey();
 
     // If the cluster has been visited, we know the region has been marked.
-    if (W.isVisited(Base))
-      continue;
-
-    // Remove the dead entry.
-    B = B.remove(Base);
-
-    if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(Base))
-      SymReaper.maybeDead(SymR->getSymbol());
-
-    // Mark all non-live symbols that this binding references as dead.
-    const ClusterBindings &Cluster = I.getData();
-    for (ClusterBindings::iterator CI = Cluster.begin(), CE = Cluster.end();
-         CI != CE; ++CI) {
-      SVal X = CI.getData();
-      SymExpr::symbol_iterator SI = X.symbol_begin(), SE = X.symbol_end();
-      for (; SI != SE; ++SI)
-        SymReaper.maybeDead(*SI);
-    }
+    // Otherwise, remove the dead entry.
+    if (!W.isVisited(Base))
+      B = B.remove(Base);
   }
 
   return StoreRef(B.asStore(), *this);

lib/StaticAnalyzer/Core/SymbolManager.cpp

@@ -401,7 +401,6 @@ void SymbolReaper::markDependentsLive(SymbolRef sym) {
 
 void SymbolReaper::markLive(SymbolRef sym) {
   TheLiving[sym] = NotProcessed;
-  TheDead.erase(sym);
   markDependentsLive(sym);
 }
 
@@ -426,14 +425,6 @@ void SymbolReaper::markInUse(SymbolRef sym) {
     MetadataInUse.insert(sym);
 }
 
-bool SymbolReaper::maybeDead(SymbolRef sym) {
-  if (isLive(sym))
-    return false;
-
-  TheDead.insert(sym);
-  return true;
-}
-
 bool SymbolReaper::isLiveRegion(const MemRegion *MR) {
   if (RegionRoots.count(MR))
     return true;

test/Analysis/MisusedMovedObject.cpp

@@ -688,3 +688,25 @@ void reportSuperClass() {
   c.foo();             // expected-warning {{Method call on a 'moved-from' object 'c'}} expected-note {{Method call on a 'moved-from' object 'c'}}
   C c2 = c;            // no-warning
 }
+
+struct Empty {};
+
+Empty inlinedCall() {
+  // Used to warn because region 'e' failed to be cleaned up because no symbols
+  // have ever died during the analysis and the checkDeadSymbols callback
+  // was skipped entirely.
+  Empty e{};
+  return e; // no-warning
+}
+
+void checkInlinedCallZombies() {
+  while (true)
+    inlinedCall();
+}
+
+void checkLoopZombies() {
+  while (true) {
+    Empty e{};
+    Empty f = std::move(e); // no-warning
+  }
+}

test/Analysis/keychainAPI.m

@@ -212,7 +212,7 @@ int foo(CFTypeRef keychainOrArray, SecProtocolType protocol,
     if (st == noErr)
       SecKeychainItemFreeContent(ptr, outData[3]);
   }
-  if (length) { // TODO: We do not report a warning here since the symbol is no longer live, but it's not marked as dead.
+  if (length) { // expected-warning{{Allocated data is not released: missing a call to 'SecKeychainItemFreeContent'}}
     length++;
   }
   return 0;
@@ -454,3 +454,15 @@ int radar_19196494() {
   }
   return 0;
 }
+int radar_19196494_v2() {
+  @autoreleasepool {
+    AuthorizationValue login_password = {};
+    OSStatus err = SecKeychainFindGenericPassword(0, 0, "", 0, "", (UInt32 *)&login_password.length, (void**)&login_password.data, 0);
+    if (!login_password.data) return 0;
+    cb.SetContextVal(&login_password);
+    if (err == noErr) {
+      SecKeychainItemFreeContent(0, login_password.data);
+    }
+  }
+  return 0;
+}

test/Analysis/loop-block-counts.c

@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void callee(void **p) {
+  int x;
+  *p = &x;
+}
+
+void loop() {
+  void *arr[2];
+  for (int i = 0; i < 2; ++i)
+    callee(&arr[i]);
+  // FIXME: Should be UNKNOWN.
+  clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{TRUE}}
+}
+
+void loopWithCall() {
+  void *arr[2];
+  for (int i = 0; i < 2; ++i) {
+    int x;
+    arr[i] = &x;
+  }
+  // FIXME: Should be UNKNOWN.
+  clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{TRUE}}
+}

test/Analysis/pr22954.c

@@ -585,7 +585,7 @@ int f28(int i, int j, int k, int l) {
   m28[j].s3[k] = 1;
   struct ll * l28 = (struct ll*)(&m28[1]);
   l28->s1[l] = 2;
-  char input[] = {'a', 'b', 'c', 'd'};
+  char input[] = {'a', 'b', 'c', 'd'}; // expected-warning{{Potential leak of memory pointed to by field 's4'}}
   memcpy(l28->s1, input, 4);
   clang_analyzer_eval(m28[0].s3[0] == 1); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(m28[0].s3[1] == 1); // expected-warning{{UNKNOWN}}

test/Analysis/retain-release-cpp-classes.cpp

@@ -0,0 +1,33 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-output=text -verify %s
+
+// expected-no-diagnostics
+
+typedef void *CFTypeRef;
+typedef struct _CFURLCacheRef *CFURLCacheRef;
+
+CFTypeRef CustomCFRetain(CFTypeRef);
+void invalidate(void *);
+struct S1 {
+  CFTypeRef s;
+  CFTypeRef returnFieldAtPlus0() {
+    return s;
+  }
+};
+struct S2 {
+  S1 *s1;
+};
+void foo(S1 *s1) {
+  invalidate(s1);
+  S2 s2;
+  s2.s1 = s1;
+  CustomCFRetain(s1->returnFieldAtPlus0());
+
+  // Definitely no leak end-of-path note here. The retained pointer
+  // is still accessible through s1 and s2.
+  ((void) 0); // no-warning
+
+  // FIXME: Ideally we need to warn after this invalidate(). The per-function
+  // retain-release contract is violated: the programmer should release
+  // the symbol after it was retained, within the same function.
+  invalidate(&s2);
+}

test/Analysis/self-assign.cpp

@@ -32,13 +32,14 @@ StringUsed& StringUsed::operator=(const StringUsed &rhs) { // expected-note{{Ass
   clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
   free(str); // expected-note{{Memory is released}}
   str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}  expected-note{{Use of memory after it is freed}}
+// expected-note@-1{{Memory is allocated}}
   return *this;
 }
 
 StringUsed& StringUsed::operator=(StringUsed &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
   clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
   str = rhs.str;
-  rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+  rhs.str = nullptr; // expected-warning{{Potential memory leak}} expected-note{{Potential memory leak}}
   return *this;
 }
 
@@ -83,7 +84,7 @@ StringUnused::operator const char*() const {
 
 int main() {
   StringUsed s1 ("test"), s2;
-  s2 = s1;
-  s2 = std::move(s1);
+  s2 = s1; // expected-note{{Calling copy assignment operator for 'StringUsed'}} // expected-note{{Returned allocated memory}}
+  s2 = std::move(s1); // expected-note{{Calling move assignment operator for 'StringUsed'}}
   return 0;
 }

test/Analysis/simple-stream-checks.c

@@ -89,3 +89,8 @@ void testPassToSystemHeaderFunctionIndirectly() {
   fs.p = fopen("myfile.txt", "w");
   fakeSystemHeaderCall(&fs); // invalidates fs, making fs.p unreachable
 }  // no-warning
+
+void testOverwrite() {
+  FILE *fp = fopen("myfile.txt", "w");
+  fp = 0;
+} // expected-warning {{Opened file is never closed; potential resource leak}}

test/Analysis/unions.cpp

@@ -90,9 +90,8 @@ namespace PR17596 {
     char str[] = "abc";
     vv.s = str;
 
-    // FIXME: This is a leak of uu.s.
     uu = vv;
-  }
+  } // expected-warning{{leak}}
 
   void testIndirectInvalidation() {
     IntOrString uu;