Thread safety analysis:

 * When we detect that a CFG block has inconsistent lock sets, point the
   diagnostic at the location where we found the inconsistency, and point a note
   at somewhere the inconsistently-locked mutex was locked.
 * Fix the wording of the normal (non-loop, non-end-of-function) case of this
   diagnostic to not suggest that the mutex is going out of scope.
 * Fix the diagnostic emission code to keep a warning and its note together when
   sorting the diagnostics into source location order.


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

include/clang/Analysis/Analyses/ThreadSafety.h

@@ -93,10 +93,14 @@ public:
   /// 3. or when a mutex is locked but not unlocked inside a function.
   /// \param LockName -- A StringRef name for the lock expression, to be printed
   /// in the error message.
-  /// \param Loc -- The location of the lock expression where the mutex is
+  /// \param LocLocked -- The location of the lock expression where the mutex is
   ///               locked
+  /// \param LocEndOfScope -- The location of the end of the scope where the
+  ///               mutex is no longer held
   /// \param LEK -- which of the three above cases we should warn for
-  virtual void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc,
+  virtual void handleMutexHeldEndOfScope(Name LockName,
+                                         SourceLocation LocLocked,
+                                         SourceLocation LocEndOfScope,
                                          LockErrorKind LEK){}
 
   /// Warn when a mutex is held exclusively and shared at the same point. For

include/clang/Basic/DiagnosticSemaKinds.td

@@ -1642,12 +1642,13 @@ def warn_no_unlock : Warning<
   "mutex '%0' is still locked at the end of function">,
   InGroup<ThreadSafety>, DefaultIgnore;
 // FIXME: improve the error message about locks not in scope
-def warn_lock_at_end_of_scope : Warning<
-  "mutex '%0' is still locked at the end of its scope">,
+def warn_lock_some_predecessors : Warning<
+  "mutex '%0' is not locked on every path through here">,
   InGroup<ThreadSafety>, DefaultIgnore;
 def warn_expecting_lock_held_on_loop : Warning<
   "expecting mutex '%0' to be locked at start of each loop">,
   InGroup<ThreadSafety>, DefaultIgnore;
+def note_locked_here : Note<"mutex acquired here">;
 def warn_lock_exclusive_and_shared : Warning<
   "mutex '%0' is locked exclusively and shared in the same scope">,
   InGroup<ThreadSafety>, DefaultIgnore;

lib/Analysis/ThreadSafety.cpp

@@ -290,6 +290,8 @@ typedef llvm::ImmutableMap<NamedDecl*, unsigned> LocalVarContext;
 
 class LocalVariableMap;
 
+/// A side (entry or exit) of a CFG node.
+enum CFGBlockSide { CBS_Entry, CBS_Exit };
 
 /// CFGBlockInfo is a struct which contains all the information that is
 /// maintained for each block in the CFG.  See LocalVariableMap for more
@@ -299,8 +301,17 @@ struct CFGBlockInfo {
   Lockset ExitSet;              // Lockset held at exit from block
   LocalVarContext EntryContext; // Context held at entry to block
   LocalVarContext ExitContext;  // Context held at exit from block
+  SourceLocation EntryLoc;      // Location of first statement in block
+  SourceLocation ExitLoc;       // Location of last statement in block.
   unsigned EntryIndex;          // Used to replay contexts later
 
+  const Lockset &getSet(CFGBlockSide Side) const {
+    return Side == CBS_Entry ? EntrySet : ExitSet;
+  }
+  SourceLocation getLocation(CFGBlockSide Side) const {
+    return Side == CBS_Entry ? EntryLoc : ExitLoc;
+  }
+
 private:
   CFGBlockInfo(Lockset EmptySet, LocalVarContext EmptyCtx)
     : EntrySet(EmptySet), ExitSet(EmptySet),
@@ -760,6 +771,51 @@ void LocalVariableMap::traverseCFG(CFG *CFGraph,
   saveContext(0, BlockInfo[exitID].ExitContext);
 }
 
+/// Find the appropriate source locations to use when producing diagnostics for
+/// each block in the CFG.
+static void findBlockLocations(CFG *CFGraph,
+                               PostOrderCFGView *SortedGraph,
+                               std::vector<CFGBlockInfo> &BlockInfo) {
+  for (PostOrderCFGView::iterator I = SortedGraph->begin(),
+       E = SortedGraph->end(); I!= E; ++I) {
+    const CFGBlock *CurrBlock = *I;
+    CFGBlockInfo *CurrBlockInfo = &BlockInfo[CurrBlock->getBlockID()];
+
+    // Find the source location of the last statement in the block, if the
+    // block is not empty.
+    if (const Stmt *S = CurrBlock->getTerminator()) {
+      CurrBlockInfo->EntryLoc = CurrBlockInfo->ExitLoc = S->getLocStart();
+    } else {
+      for (CFGBlock::const_reverse_iterator BI = CurrBlock->rbegin(),
+           BE = CurrBlock->rend(); BI != BE; ++BI) {
+        // FIXME: Handle other CFGElement kinds.
+        if (const CFGStmt *CS = dyn_cast<CFGStmt>(&*BI)) {
+          CurrBlockInfo->ExitLoc = CS->getStmt()->getLocStart();
+          break;
+        }
+      }
+    }
+
+    if (!CurrBlockInfo->ExitLoc.isInvalid()) {
+      // This block contains at least one statement. Find the source location
+      // of the first statement in the block.
+      for (CFGBlock::const_iterator BI = CurrBlock->begin(),
+           BE = CurrBlock->end(); BI != BE; ++BI) {
+        // FIXME: Handle other CFGElement kinds.
+        if (const CFGStmt *CS = dyn_cast<CFGStmt>(&*BI)) {
+          CurrBlockInfo->EntryLoc = CS->getStmt()->getLocStart();
+          break;
+        }
+      }
+    } else if (CurrBlock->pred_size() == 1 && *CurrBlock->pred_begin() &&
+               CurrBlock != &CFGraph->getExit()) {
+      // The block is empty, and has a single predecessor. Use its exit
+      // location.
+      CurrBlockInfo->EntryLoc = CurrBlockInfo->ExitLoc =
+          BlockInfo[(*CurrBlock->pred_begin())->getBlockID()].ExitLoc;
+    }
+  }
+}
 
 /// \brief Class which implements the core thread safety analysis routines.
 class ThreadSafetyAnalyzer {
@@ -772,7 +828,8 @@ class ThreadSafetyAnalyzer {
 public:
   ThreadSafetyAnalyzer(ThreadSafetyHandler &H) : Handler(H) {}
 
-  Lockset intersectAndWarn(const Lockset LSet1, const Lockset LSet2,
+  Lockset intersectAndWarn(const CFGBlockInfo &Block1, CFGBlockSide Side1,
+                           const CFGBlockInfo &Block2, CFGBlockSide Side2,
                            LockErrorKind LEK);
 
   Lockset addLock(Lockset &LSet, Expr *MutexExp, const NamedDecl *D,
@@ -1304,9 +1361,14 @@ void BuildLockset::VisitDeclStmt(DeclStmt *S) {
 /// A; if () then B; else C; D; we need to check that the lockset after B and C
 /// are the same. In the event of a difference, we use the intersection of these
 /// two locksets at the start of D.
-Lockset ThreadSafetyAnalyzer::intersectAndWarn(const Lockset LSet1,
-                                               const Lockset LSet2,
+Lockset ThreadSafetyAnalyzer::intersectAndWarn(const CFGBlockInfo &Block1,
+                                               CFGBlockSide Side1,
+                                               const CFGBlockInfo &Block2,
+                                               CFGBlockSide Side2,
                                                LockErrorKind LEK) {
+  Lockset LSet1 = Block1.getSet(Side1);
+  Lockset LSet2 = Block2.getSet(Side2);
+
   Lockset Intersection = LSet1;
   for (Lockset::iterator I = LSet2.begin(), E = LSet2.end(); I != E; ++I) {
     const MutexID &LSet2Mutex = I.getKey();
@@ -1322,7 +1384,8 @@ Lockset ThreadSafetyAnalyzer::intersectAndWarn(const Lockset LSet1,
       }
     } else {
       Handler.handleMutexHeldEndOfScope(LSet2Mutex.getName(),
-                                        LSet2LockData.AcquireLoc, LEK);
+                                        LSet2LockData.AcquireLoc,
+                                        Block1.getLocation(Side1), LEK);
     }
   }
 
@@ -1331,7 +1394,8 @@ Lockset ThreadSafetyAnalyzer::intersectAndWarn(const Lockset LSet1,
       const MutexID &Mutex = I.getKey();
       const LockData &MissingLock = I.getData();
       Handler.handleMutexHeldEndOfScope(Mutex.getName(),
-                                        MissingLock.AcquireLoc, LEK);
+                                        MissingLock.AcquireLoc,
+                                        Block2.getLocation(Side2), LEK);
       Intersection = LocksetFactory.remove(Intersection, Mutex);
     }
   }
@@ -1377,6 +1441,9 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
   // Compute SSA names for local variables
   LocalVarMap.traverseCFG(CFGraph, SortedGraph, BlockInfo);
 
+  // Fill in source locations for all CFGBlocks.
+  findBlockLocations(CFGraph, SortedGraph, BlockInfo);
+
   // Add locks from exclusive_locks_required and shared_locks_required
   // to initial lockset.
   if (!SortedGraph->empty() && D->hasAttrs()) {
@@ -1456,7 +1523,8 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         LocksetInitialized = true;
       } else {
         CurrBlockInfo->EntrySet =
-          intersectAndWarn(CurrBlockInfo->EntrySet, PrevBlockInfo->ExitSet,
+          intersectAndWarn(*CurrBlockInfo, CBS_Entry,
+                           *PrevBlockInfo, CBS_Exit,
                            LEK_LockedSomePredecessors);
       }
     }
@@ -1482,7 +1550,7 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         bool IsLoop = Terminator && isa<ContinueStmt>(Terminator);
 
         // Do not update EntrySet.
-        intersectAndWarn(CurrBlockInfo->EntrySet, PrevBlockInfo->ExitSet,
+        intersectAndWarn(*CurrBlockInfo, CBS_Entry, *PrevBlockInfo, CBS_Exit,
                          IsLoop ? LEK_LockedSomeLoopIterations
                                 : LEK_LockedSomePredecessors);
       }
@@ -1541,17 +1609,19 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
         continue;
 
       CFGBlock *FirstLoopBlock = *SI;
-      Lockset PreLoop = BlockInfo[FirstLoopBlock->getBlockID()].EntrySet;
-      Lockset LoopEnd = BlockInfo[CurrBlockID].ExitSet;
-      intersectAndWarn(LoopEnd, PreLoop, LEK_LockedSomeLoopIterations);
+      CFGBlockInfo &PreLoop = BlockInfo[FirstLoopBlock->getBlockID()];
+      CFGBlockInfo &LoopEnd = BlockInfo[CurrBlockID];
+      intersectAndWarn(LoopEnd, CBS_Exit, PreLoop, CBS_Entry,
+                       LEK_LockedSomeLoopIterations);
     }
   }
 
-  Lockset InitialLockset = BlockInfo[CFGraph->getEntry().getBlockID()].EntrySet;
-  Lockset FinalLockset = BlockInfo[CFGraph->getExit().getBlockID()].ExitSet;
+  CFGBlockInfo &Initial = BlockInfo[CFGraph->getEntry().getBlockID()];
+  CFGBlockInfo &Final = BlockInfo[CFGraph->getExit().getBlockID()];
 
   // FIXME: Should we call this function for all blocks which exit the function?
-  intersectAndWarn(InitialLockset, FinalLockset, LEK_LockedAtEndOfFunction);
+  intersectAndWarn(Initial, CBS_Entry, Final, CBS_Exit,
+                   LEK_LockedAtEndOfFunction);
 }
 
 } // end anonymous namespace

lib/Sema/AnalysisBasedWarnings.cpp

@@ -592,7 +592,8 @@ private:
 //===----------------------------------------------------------------------===//
 namespace clang {
 namespace thread_safety {
-typedef std::pair<SourceLocation, PartialDiagnostic> DelayedDiag;
+typedef llvm::SmallVector<PartialDiagnosticAt, 1> OptionalNotes;
+typedef std::pair<PartialDiagnosticAt, OptionalNotes> DelayedDiag;
 typedef llvm::SmallVector<DelayedDiag, 4> DiagList;
 
 struct SortDiagBySourceLocation {
@@ -602,8 +603,8 @@ struct SortDiagBySourceLocation {
   bool operator()(const DelayedDiag &left, const DelayedDiag &right) {
     // Although this call will be slow, this is only called when outputting
     // multiple warnings.
-    return S.getSourceManager().isBeforeInTranslationUnit(left.first,
-                                                          right.first);
+    return S.getSourceManager().isBeforeInTranslationUnit(left.first.first,
+                                                          right.first.first);
   }
 };
 
@@ -611,7 +612,7 @@ namespace {
 class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
   Sema &S;
   DiagList Warnings;
-  SourceLocation FunLocation;
+  SourceLocation FunLocation, FunEndLocation;
 
   // Helper functions
   void warnLockMismatch(unsigned DiagID, Name LockName, SourceLocation Loc) {
@@ -619,13 +620,13 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
     // precise source location.
     if (!Loc.isValid())
       Loc = FunLocation;
-    PartialDiagnostic Warning = S.PDiag(DiagID) << LockName;
-    Warnings.push_back(DelayedDiag(Loc, Warning));
+    PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << LockName);
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
 
  public:
-  ThreadSafetyReporter(Sema &S, SourceLocation FL)
-    : S(S), FunLocation(FL) {}
+  ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL)
+    : S(S), FunLocation(FL), FunEndLocation(FEL) {}
 
   /// \brief Emit all buffered diagnostics in order of sourcelocation.
   /// We need to output diagnostics produced while iterating through
@@ -635,13 +636,18 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
     SortDiagBySourceLocation SortDiagBySL(S);
     sort(Warnings.begin(), Warnings.end(), SortDiagBySL);
     for (DiagList::iterator I = Warnings.begin(), E = Warnings.end();
-         I != E; ++I)
-      S.Diag(I->first, I->second);
+         I != E; ++I) {
+      S.Diag(I->first.first, I->first.second);
+      const OptionalNotes &Notes = I->second;
+      for (unsigned NoteI = 0, NoteN = Notes.size(); NoteI != NoteN; ++NoteI)
+        S.Diag(Notes[NoteI].first, Notes[NoteI].second);
+    }
   }
 
   void handleInvalidLockExp(SourceLocation Loc) {
-    PartialDiagnostic Warning = S.PDiag(diag::warn_cannot_resolve_lock) << Loc;
-    Warnings.push_back(DelayedDiag(Loc, Warning));
+    PartialDiagnosticAt Warning(Loc,
+                                S.PDiag(diag::warn_cannot_resolve_lock) << Loc);
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
   void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) {
     warnLockMismatch(diag::warn_unlock_but_no_lock, LockName, Loc);
@@ -651,12 +657,13 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
     warnLockMismatch(diag::warn_double_lock, LockName, Loc);
   }
 
-  void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc,
+  void handleMutexHeldEndOfScope(Name LockName, SourceLocation LocLocked,
+                                 SourceLocation LocEndOfScope,
                                  LockErrorKind LEK){
     unsigned DiagID = 0;
     switch (LEK) {
       case LEK_LockedSomePredecessors:
-        DiagID = diag::warn_lock_at_end_of_scope;
+        DiagID = diag::warn_lock_some_predecessors;
         break;
       case LEK_LockedSomeLoopIterations:
         DiagID = diag::warn_expecting_lock_held_on_loop;
@@ -665,18 +672,22 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
         DiagID = diag::warn_no_unlock;
         break;
     }
-    warnLockMismatch(DiagID, LockName, Loc);
+    if (LocEndOfScope.isInvalid())
+      LocEndOfScope = FunEndLocation;
+
+    PartialDiagnosticAt Warning(LocEndOfScope, S.PDiag(DiagID) << LockName);
+    PartialDiagnosticAt Note(LocLocked, S.PDiag(diag::note_locked_here));
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note)));
   }
 
 
   void handleExclusiveAndShared(Name LockName, SourceLocation Loc1,
                                 SourceLocation Loc2) {
-    PartialDiagnostic Warning =
-      S.PDiag(diag::warn_lock_exclusive_and_shared) << LockName;
-    PartialDiagnostic Note =
-      S.PDiag(diag::note_lock_exclusive_and_shared) << LockName;
-    Warnings.push_back(DelayedDiag(Loc1, Warning));
-    Warnings.push_back(DelayedDiag(Loc2, Note));
+    PartialDiagnosticAt Warning(
+      Loc1, S.PDiag(diag::warn_lock_exclusive_and_shared) << LockName);
+    PartialDiagnosticAt Note(
+      Loc2, S.PDiag(diag::note_lock_exclusive_and_shared) << LockName);
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes(1, Note)));
   }
 
   void handleNoMutexHeld(const NamedDecl *D, ProtectedOperationKind POK,
@@ -686,9 +697,9 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
     unsigned DiagID = POK == POK_VarAccess?
                         diag::warn_variable_requires_any_lock:
                         diag::warn_var_deref_requires_any_lock;
-    PartialDiagnostic Warning = S.PDiag(DiagID)
-      << D->getName() << getLockKindFromAccessKind(AK);
-    Warnings.push_back(DelayedDiag(Loc, Warning));
+    PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID)
+      << D->getName() << getLockKindFromAccessKind(AK));
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
 
   void handleMutexNotHeld(const NamedDecl *D, ProtectedOperationKind POK,
@@ -705,15 +716,15 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
         DiagID = diag::warn_fun_requires_lock;
         break;
     }
-    PartialDiagnostic Warning = S.PDiag(DiagID)
-      << D->getName() << LockName << LK;
-    Warnings.push_back(DelayedDiag(Loc, Warning));
+    PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID)
+      << D->getName() << LockName << LK);
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
 
   void handleFunExcludesLock(Name FunName, Name LockName, SourceLocation Loc) {
-    PartialDiagnostic Warning =
-      S.PDiag(diag::warn_fun_excludes_mutex) << FunName << LockName;
-    Warnings.push_back(DelayedDiag(Loc, Warning));
+    PartialDiagnosticAt Warning(Loc,
+      S.PDiag(diag::warn_fun_excludes_mutex) << FunName << LockName);
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
 };
 }
@@ -897,7 +908,8 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
   // Check for thread safety violations
   if (P.enableThreadSafetyAnalysis) {
     SourceLocation FL = AC.getDecl()->getLocation();
-    thread_safety::ThreadSafetyReporter Reporter(S, FL);
+    SourceLocation FEL = AC.getDecl()->getLocEnd();
+    thread_safety::ThreadSafetyReporter Reporter(S, FL, FEL);
     thread_safety::runThreadSafetyAnalysis(AC, Reporter);
     Reporter.emitDiagnostics();
   }

test/SemaCXX/warn-thread-safety-analysis.cpp

@@ -172,30 +172,29 @@ void sls_fun_bad_2() {
 }
 
 void sls_fun_bad_3() {
-  sls_mu.Lock(); // \
-    // expected-warning{{mutex 'sls_mu' is still locked at the end of function}}
-}
+  sls_mu.Lock(); // expected-note {{mutex acquired here}}
+} // expected-warning{{mutex 'sls_mu' is still locked at the end of function}}
 
 void sls_fun_bad_4() {
   if (getBool())
     sls_mu.Lock(); // \
-      // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}}
+  expected-warning{{mutex 'sls_mu2' is not locked on every path through here}} \
+  expected-note{{mutex acquired here}}
+
   else
     sls_mu2.Lock(); // \
-      // expected-warning{{mutex 'sls_mu2' is still locked at the end of its scope}}
-}
+  expected-note{{mutex acquired here}}
+} // expected-warning{{mutex 'sls_mu' is not locked on every path through here}}
 
 void sls_fun_bad_5() {
-  sls_mu.Lock(); // \
-    // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}}
+  sls_mu.Lock(); // expected-note {{mutex acquired here}}
   if (getBool())
     sls_mu.Unlock();
-}
+} // expected-warning{{mutex 'sls_mu' is not locked on every path through here}}
 
 void sls_fun_bad_6() {
   if (getBool()) {
-    sls_mu.Lock(); // \
-      // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}}
+    sls_mu.Lock(); // expected-note {{mutex acquired here}}
   } else {
     if (getBool()) {
       getBool(); // EMPTY
@@ -204,7 +203,8 @@ void sls_fun_bad_6() {
     }
   }
   sls_mu.Unlock(); // \
-    // expected-warning{{unlocking 'sls_mu' that was not locked}}
+    expected-warning{{mutex 'sls_mu' is not locked on every path through here}}\
+    expected-warning{{unlocking 'sls_mu' that was not locked}}
 }
 
 void sls_fun_bad_7() {
@@ -213,19 +213,20 @@ void sls_fun_bad_7() {
     sls_mu.Unlock();
     if (getBool()) {
       if (getBool()) {
-        continue;
+        continue; // \
+        expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
       }
     }
-    sls_mu.Lock(); // \
-      // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+    sls_mu.Lock(); // expected-note {{mutex acquired here}}
   }
   sls_mu.Unlock();
 }
 
 void sls_fun_bad_8() {
-  sls_mu.Lock(); // \
-    // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
-  do {
+  sls_mu.Lock(); // expected-note{{mutex acquired here}}
+
+  // FIXME: TERRIBLE SOURCE LOCATION!
+  do { // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
     sls_mu.Unlock();
   } while (getBool());
 }
@@ -233,37 +234,35 @@ void sls_fun_bad_8() {
 void sls_fun_bad_9() {
   do {
     sls_mu.Lock(); // \
-      // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+      // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}} \
+      // expected-note{{mutex acquired here}}
   } while (getBool());
   sls_mu.Unlock();
 }
 
 void sls_fun_bad_10() {
-  sls_mu.Lock(); // \
-    // expected-warning{{mutex 'sls_mu' is still locked at the end of function}} \
-    // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+  sls_mu.Lock(); // expected-note 2{{mutex acquired here}}
   while(getBool()) {
-    sls_mu.Unlock();
+    sls_mu.Unlock(); // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
   }
-}
+} // expected-warning{{mutex 'sls_mu' is still locked at the end of function}}
 
 void sls_fun_bad_11() {
-  while (getBool()) {
-    sls_mu.Lock(); // \
-      // expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+  while (getBool()) { // \
+   expected-warning{{expecting mutex 'sls_mu' to be locked at start of each loop}}
+    sls_mu.Lock(); // expected-note {{mutex acquired here}}
   }
   sls_mu.Unlock(); // \
     // expected-warning{{unlocking 'sls_mu' that was not locked}}
 }
 
 void sls_fun_bad_12() {
-  sls_mu.Lock(); // \
-    // expected-warning{{mutex 'sls_mu' is still locked at the end of its scope}}
+  sls_mu.Lock(); // expected-note {{mutex acquired here}}
   while (getBool()) {
     sls_mu.Unlock();
     if (getBool()) {
       if (getBool()) {
-        break;
+        break; // expected-warning{{mutex 'sls_mu' is not locked on every path through here}}
       }
     }
     sls_mu.Lock();
@@ -303,9 +302,8 @@ void aa_fun_bad_2() {
 }
 
 void aa_fun_bad_3() {
-  glock.globalLock(); // \
-    // expected-warning{{mutex 'aa_mu' is still locked at the end of function}}
-}
+  glock.globalLock(); // expected-note{{mutex acquired here}}
+} // expected-warning{{mutex 'aa_mu' is still locked at the end of function}}
 
 //--------------------------------------------------//
 // Regression tests for unusual method names
@@ -316,22 +314,18 @@ Mutex wmu;
 // Test diagnostics for other method names.
 class WeirdMethods {
   WeirdMethods() {
-    wmu.Lock(); // \
-      // expected-warning {{mutex 'wmu' is still locked at the end of function}}
-  }
+    wmu.Lock(); // expected-note {{mutex acquired here}}
+  } // expected-warning {{mutex 'wmu' is still locked at the end of function}}
   ~WeirdMethods() {
-    wmu.Lock(); // \
-      // expected-warning {{mutex 'wmu' is still locked at the end of function}}
-  }
+    wmu.Lock(); // expected-note {{mutex acquired here}}
+  } // expected-warning {{mutex 'wmu' is still locked at the end of function}}
   void operator++() {
-    wmu.Lock(); // \
-      // expected-warning {{mutex 'wmu' is still locked at the end of function}}
-  }
+    wmu.Lock(); // expected-note {{mutex acquired here}}
+  } // expected-warning {{mutex 'wmu' is still locked at the end of function}}
   operator int*() {
-    wmu.Lock(); // \
-      // expected-warning {{mutex 'wmu' is still locked at the end of function}}
+    wmu.Lock(); // expected-note {{mutex acquired here}}
     return 0;
-  }
+  } // expected-warning {{mutex 'wmu' is still locked at the end of function}}
 };
 
 //-----------------------------------------------//
@@ -1484,11 +1478,10 @@ namespace substitution_test {
 
     void bar3(MyData* d1, MyData* d2) {
       DataLocker dlr;
-      dlr.lockData(d1);   // \
-        // expected-warning {{mutex 'mu' is still locked at the end of function}}
+      dlr.lockData(d1);   // expected-note {{mutex acquired here}}
       dlr.unlockData(d2); // \
         // expected-warning {{unlocking 'mu' that was not locked}}
-    }
+    } // expected-warning {{mutex 'mu' is still locked at the end of function}}
 
     void bar4(MyData* d1, MyData* d2) {
       DataLocker dlr;
@@ -1914,3 +1907,32 @@ void test() {
 }
 
 };
+
+namespace GoingNative {
+
+  struct __attribute__((lockable)) mutex {
+    void lock() __attribute__((exclusive_lock_function));
+    void unlock() __attribute__((unlock_function));
+    // ...
+  };
+  bool foo();
+  bool bar();
+  mutex m;
+  void test() {
+    m.lock();
+    while (foo()) {
+      m.unlock();
+      // ...
+      if (bar()) {
+        // ...
+        if (foo())
+          continue; // expected-warning {{expecting mutex 'm' to be locked at start of each loop}}
+        //...
+      }
+      // ...
+      m.lock(); // expected-note {{mutex acquired here}}
+    }
+    m.unlock();
+  }
+
+}