Author: Aaron Puchert Date: 2021-05-03T14:03:17+02:00 New Revision: daca6edb31efae048a420595fae9750aaa91c733
URL: https://github.com/llvm/llvm-project/commit/daca6edb31efae048a420595fae9750aaa91c733 DIFF: https://github.com/llvm/llvm-project/commit/daca6edb31efae048a420595fae9750aaa91c733.diff LOG: Thread safety analysis: Fix false negative on break We weren't modifying the lock set when intersecting with one coming from a break-terminated block. This is inconsistent, since break isn't a back edge, and it leads to false negatives with scoped locks. We usually don't warn for those when joining locksets aren't the same, we just silently remove locks that are not in the intersection. But not warning and not removing them isn't right. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D101202 Added: Modified: clang/lib/Analysis/ThreadSafety.cpp clang/test/PCH/thread-safety-attrs.cpp clang/test/SemaCXX/warn-thread-safety-analysis.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index bb3a4f1616fca..4377fc58a1d55 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -2467,11 +2467,10 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { PrevBlock, CurrBlock); // Do not update EntrySet. - intersectAndWarn(CurrBlockInfo->EntrySet, PrevLockset, - PrevBlockInfo->ExitLoc, - IsLoop ? LEK_LockedSomeLoopIterations - : LEK_LockedSomePredecessors, - false); + intersectAndWarn( + CurrBlockInfo->EntrySet, PrevLockset, PrevBlockInfo->ExitLoc, + IsLoop ? LEK_LockedSomeLoopIterations : LEK_LockedSomePredecessors, + !IsLoop); } } diff --git a/clang/test/PCH/thread-safety-attrs.cpp b/clang/test/PCH/thread-safety-attrs.cpp index ae2a413af31c8..3e0c081f134f9 100644 --- a/clang/test/PCH/thread-safety-attrs.cpp +++ b/clang/test/PCH/thread-safety-attrs.cpp @@ -311,7 +311,8 @@ void sls_fun_bad_12() { } sls_mu.Lock(); } - sls_mu.Unlock(); + sls_mu.Unlock(); // \ + // expected-warning{{releasing mutex 'sls_mu' that was not held}} } #endif diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index b837206138a67..369952eb397a2 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -337,7 +337,8 @@ void sls_fun_bad_12() { } sls_mu.Lock(); } - sls_mu.Unlock(); + sls_mu.Unlock(); // \ + // expected-warning{{releasing mutex 'sls_mu' that was not held}} } //-----------------------------------------// @@ -2582,6 +2583,7 @@ class Foo { void test3(); void test4(); void test5(); + void test6(); }; @@ -2620,6 +2622,18 @@ void Foo::test5() { rlock.Release(); // expected-warning {{releasing mutex 'mu_' that was not held}} } +void Foo::test6() { + ReleasableMutexLock rlock(&mu_); + do { + if (c) { + rlock.Release(); + break; + } + } while (c); + // No warning on join point because the lock will be released by the scope object anyway + a = 1; // expected-warning {{writing variable 'a' requires holding mutex 'mu_' exclusively}} +} + } // end namespace ReleasableScopedLock _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
