[compiler-rt] r201576 - [sanitizer] when reporting a deadlock also report the lock cycle
Kostya Serebryany
kcc at google.com
Tue Feb 18 06:56:20 PST 2014
Author: kcc
Date: Tue Feb 18 08:56:19 2014
New Revision: 201576
URL: http://llvm.org/viewvc/llvm-project?rev=201576&view=rev
Log:
[sanitizer] when reporting a deadlock also report the lock cycle
Modified:
compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h
compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h
compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc
compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_deadlock_detector_test.cc
compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_mutex.cc
compiler-rt/trunk/test/tsan/deadlock_detector_stress_test.cc
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h?rev=201576&r1=201575&r2=201576&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_bvgraph.h Tue Feb 18 08:56:19 2014
@@ -129,6 +129,15 @@ class BVGraph {
return 0;
}
+ // Same as findPath, but finds a shortest path.
+ uptr findShortestPath(uptr from, const BV &targets, uptr *path,
+ uptr path_size) {
+ for (uptr p = 1; p <= path_size; p++)
+ if (findPath(from, targets, path, p) == p)
+ return p;
+ return 0;
+ }
+
private:
void check(uptr idx1, uptr idx2) const {
CHECK_LT(idx1, size());
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h?rev=201576&r1=201575&r2=201576&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_deadlock_detector.h Tue Feb 18 08:56:19 2014
@@ -137,11 +137,31 @@ class DeadlockDetector {
return is_reachable;
}
+ // Finds a path between the lock 'cur_node' (which is currently held in dtls)
+ // and some other currently held lock, returns the length of the path
+ // or 0 on failure.
+ uptr findPathToHeldLock(DeadlockDetectorTLS<BV> *dtls, uptr cur_node,
+ uptr *path, uptr path_size) {
+ tmp_bv_.copyFrom(dtls->getLocks());
+ uptr idx = nodeToIndex(cur_node);
+ CHECK(tmp_bv_.clearBit(idx));
+ uptr res = g_.findShortestPath(idx, tmp_bv_, path, path_size);
+ for (uptr i = 0; i < res; i++)
+ path[i] = indexToNode(path[i]);
+ if (res)
+ CHECK_EQ(path[0], cur_node);
+ return res;
+ }
+
// Handle the unlock event.
void onUnlock(DeadlockDetectorTLS<BV> *dtls, uptr node) {
dtls->removeLock(nodeToIndex(node), current_epoch_);
}
+ bool isHeld(DeadlockDetectorTLS<BV> *dtls, uptr node) const {
+ return dtls->getLocks().getBit(nodeToIndex(node));
+ }
+
uptr testOnlyGetEpoch() const { return current_epoch_; }
void Print() {
@@ -178,6 +198,7 @@ class DeadlockDetector {
uptr current_epoch_;
BV available_nodes_;
BV recycled_nodes_;
+ BV tmp_bv_;
BVGraph<BV> g_;
uptr data_[BV::kSize];
};
Modified: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc?rev=201576&r1=201575&r2=201576&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_bvgraph_test.cc Tue Feb 18 08:56:19 2014
@@ -212,10 +212,10 @@ void Test_isReachable() {
}
TEST(BVGraph, isReachable) {
- Test_isReachable<BasicBitVector<u8> >();
- Test_isReachable<BasicBitVector<> >();
- Test_isReachable<TwoLevelBitVector<> >();
- Test_isReachable<TwoLevelBitVector<3, BasicBitVector<u8> > >();
+ Test_isReachable<BV1>();
+ Test_isReachable<BV2>();
+ Test_isReachable<BV3>();
+ Test_isReachable<BV4>();
}
template <class BV>
@@ -256,8 +256,48 @@ void LongCycle() {
}
TEST(BVGraph, LongCycle) {
- LongCycle<BasicBitVector<u8> >();
- LongCycle<BasicBitVector<> >();
- LongCycle<TwoLevelBitVector<> >();
- LongCycle<TwoLevelBitVector<2, BasicBitVector<u8> > >();
+ LongCycle<BV1>();
+ LongCycle<BV2>();
+ LongCycle<BV3>();
+ LongCycle<BV4>();
+}
+
+template <class BV>
+void ShortestPath() {
+ uptr path[8];
+ BVGraph<BV> g;
+ g.clear();
+ BV t7;
+ t7.clear();
+ t7.setBit(7);
+ // 1=>2=>3=>4=>5=>6=>7
+ // 1=>7
+ g.addEdge(1, 2);
+ g.addEdge(2, 3);
+ g.addEdge(3, 4);
+ g.addEdge(4, 5);
+ g.addEdge(5, 6);
+ g.addEdge(6, 7);
+ g.addEdge(1, 7);
+ EXPECT_TRUE(g.isReachable(1, t7));
+ // No path of length 1.
+ EXPECT_EQ(0U, g.findPath(1, t7, path, 1));
+ // Trying to find a path of len 2..6 gives path of len 2.
+ EXPECT_EQ(2U, g.findPath(1, t7, path, 2));
+ EXPECT_EQ(2U, g.findPath(1, t7, path, 3));
+ EXPECT_EQ(2U, g.findPath(1, t7, path, 4));
+ EXPECT_EQ(2U, g.findPath(1, t7, path, 5));
+ EXPECT_EQ(2U, g.findPath(1, t7, path, 6));
+ // Trying to find a path of len 7 gives path of len 7, because this is DFS.
+ EXPECT_EQ(7U, g.findPath(1, t7, path, 7));
+ // But findShortestPath will find the shortest path.
+ EXPECT_EQ(2U, g.findShortestPath(1, t7, path, 2));
+ EXPECT_EQ(2U, g.findShortestPath(1, t7, path, 7));
+}
+
+TEST(BVGraph, ShortestPath) {
+ ShortestPath<BV1>();
+ ShortestPath<BV2>();
+ ShortestPath<BV3>();
+ ShortestPath<BV4>();
}
Modified: compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_deadlock_detector_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_deadlock_detector_test.cc?rev=201576&r1=201575&r2=201576&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_deadlock_detector_test.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_deadlock_detector_test.cc Tue Feb 18 08:56:19 2014
@@ -43,7 +43,8 @@ struct ScopedDD {
};
template <class BV>
-void BasicTest() {
+void RunBasicTest() {
+ uptr path[10];
ScopedDD<BV> sdd;
DeadlockDetector<BV> &d = *sdd.dp;
DeadlockDetectorTLS<BV> &dtls = sdd.dtls;
@@ -82,6 +83,13 @@ void BasicTest() {
EXPECT_FALSE(d.onLock(&dtls, n2));
EXPECT_TRUE(d.onLock(&dtls, n1));
+ EXPECT_EQ(0U, d.findPathToHeldLock(&dtls, n1, path, 1));
+ EXPECT_EQ(2U, d.findPathToHeldLock(&dtls, n1, path, 10));
+ EXPECT_EQ(2U, d.findPathToHeldLock(&dtls, n1, path, 2));
+ EXPECT_EQ(path[0], n1);
+ EXPECT_EQ(path[1], n2);
+ EXPECT_EQ(d.getData(n1), 1U);
+ EXPECT_EQ(d.getData(n2), 2U);
d.onUnlock(&dtls, n1);
d.onUnlock(&dtls, n2);
}
@@ -106,20 +114,28 @@ void BasicTest() {
EXPECT_FALSE(d.onLock(&dtls, n3));
EXPECT_TRUE(d.onLock(&dtls, n1));
+ EXPECT_EQ(0U, d.findPathToHeldLock(&dtls, n1, path, 2));
+ EXPECT_EQ(3U, d.findPathToHeldLock(&dtls, n1, path, 10));
+ EXPECT_EQ(path[0], n1);
+ EXPECT_EQ(path[1], n2);
+ EXPECT_EQ(path[2], n3);
+ EXPECT_EQ(d.getData(n1), 1U);
+ EXPECT_EQ(d.getData(n2), 2U);
+ EXPECT_EQ(d.getData(n3), 3U);
d.onUnlock(&dtls, n1);
d.onUnlock(&dtls, n3);
}
}
TEST(DeadlockDetector, BasicTest) {
- BasicTest<BV1>();
- BasicTest<BV2>();
- BasicTest<BV3>();
- BasicTest<BV4>();
+ RunBasicTest<BV1>();
+ RunBasicTest<BV2>();
+ RunBasicTest<BV3>();
+ RunBasicTest<BV4>();
}
template <class BV>
-void RemoveNodeTest() {
+void RunRemoveNodeTest() {
ScopedDD<BV> sdd;
DeadlockDetector<BV> &d = *sdd.dp;
DeadlockDetectorTLS<BV> &dtls = sdd.dtls;
@@ -218,14 +234,14 @@ void RemoveNodeTest() {
}
TEST(DeadlockDetector, RemoveNodeTest) {
- RemoveNodeTest<BV1>();
- RemoveNodeTest<BV2>();
- RemoveNodeTest<BV3>();
- RemoveNodeTest<BV4>();
+ RunRemoveNodeTest<BV1>();
+ RunRemoveNodeTest<BV2>();
+ RunRemoveNodeTest<BV3>();
+ RunRemoveNodeTest<BV4>();
}
template <class BV>
-void MultipleEpochsTest() {
+void RunMultipleEpochsTest() {
ScopedDD<BV> sdd;
DeadlockDetector<BV> &d = *sdd.dp;
DeadlockDetectorTLS<BV> &dtls = sdd.dtls;
@@ -259,10 +275,10 @@ void MultipleEpochsTest() {
}
TEST(DeadlockDetector, MultipleEpochsTest) {
- MultipleEpochsTest<BV1>();
- MultipleEpochsTest<BV2>();
- MultipleEpochsTest<BV3>();
- MultipleEpochsTest<BV4>();
+ RunMultipleEpochsTest<BV1>();
+ RunMultipleEpochsTest<BV2>();
+ RunMultipleEpochsTest<BV3>();
+ RunMultipleEpochsTest<BV4>();
}
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_mutex.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_mutex.cc?rev=201576&r1=201575&r2=201576&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_mutex.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_mutex.cc Tue Feb 18 08:56:19 2014
@@ -22,12 +22,11 @@
namespace __tsan {
-static __sanitizer::DeadlockDetector<DDBV> g_deadlock_detector;
+static __sanitizer::DeadlockDetector<DDBV> g_dd;
static void EnsureDeadlockDetectorID(ThreadState *thr, SyncVar *s) {
- if (!g_deadlock_detector.nodeBelongsToCurrentEpoch(s->deadlock_detector_id))
- s->deadlock_detector_id =
- g_deadlock_detector.newNode(reinterpret_cast<uptr>(s));
+ if (!g_dd.nodeBelongsToCurrentEpoch(s->deadlock_detector_id))
+ s->deadlock_detector_id = g_dd.newNode(reinterpret_cast<uptr>(s->addr));
}
void MutexCreate(ThreadState *thr, uptr pc, uptr addr,
@@ -62,8 +61,8 @@ void MutexDestroy(ThreadState *thr, uptr
if (s == 0)
return;
if (common_flags()->detect_deadlocks) {
- if (g_deadlock_detector.nodeBelongsToCurrentEpoch(s->deadlock_detector_id))
- g_deadlock_detector.removeNode(s->deadlock_detector_id);
+ if (g_dd.nodeBelongsToCurrentEpoch(s->deadlock_detector_id))
+ g_dd.removeNode(s->deadlock_detector_id);
s->deadlock_detector_id = 0;
}
if (IsAppMem(addr)) {
@@ -121,11 +120,26 @@ void MutexLock(ThreadState *thr, uptr pc
thr->mset.Add(s->GetId(), true, thr->fast_state.epoch());
if (common_flags()->detect_deadlocks) {
EnsureDeadlockDetectorID(thr, s);
+ if (g_dd.isHeld(&thr->deadlock_detector_tls, s->deadlock_detector_id)) {
+ // FIXME: add tests, handle the real recursive locks.
+ Printf("ThreadSanitizer: reursive-lock\n");
+ }
// Printf("MutexLock: %zx\n", s->deadlock_detector_id);
- bool has_deadlock = g_deadlock_detector.onLock(&thr->deadlock_detector_tls,
- s->deadlock_detector_id);
- if (has_deadlock)
+ bool has_deadlock =
+ g_dd.onLock(&thr->deadlock_detector_tls, s->deadlock_detector_id);
+ if (has_deadlock) {
Printf("ThreadSanitizer: lock-order-inversion (potential deadlock)\n");
+ uptr path[10];
+ uptr len = g_dd.findPathToHeldLock(&thr->deadlock_detector_tls,
+ s->deadlock_detector_id, path,
+ ARRAY_SIZE(path));
+ CHECK_GT(len, 0U); // Hm.. cycle of 10 locks? I'd like to see that.
+ Printf(" path: ");
+ for (uptr i = 0; i < len; i++) {
+ Printf("%p => ", g_dd.getData(path[i]));
+ }
+ Printf("%p\n", g_dd.getData(path[0]));
+ }
}
s->mtx.Unlock();
}
@@ -166,7 +180,7 @@ int MutexUnlock(ThreadState *thr, uptr p
if (common_flags()->detect_deadlocks) {
EnsureDeadlockDetectorID(thr, s);
// Printf("MutexUnlock: %zx\n", s->deadlock_detector_id);
- g_deadlock_detector.onUnlock(&thr->deadlock_detector_tls,
+ g_dd.onUnlock(&thr->deadlock_detector_tls,
s->deadlock_detector_id);
}
s->mtx.Unlock();
Modified: compiler-rt/trunk/test/tsan/deadlock_detector_stress_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/deadlock_detector_stress_test.cc?rev=201576&r1=201575&r2=201576&view=diff
==============================================================================
--- compiler-rt/trunk/test/tsan/deadlock_detector_stress_test.cc (original)
+++ compiler-rt/trunk/test/tsan/deadlock_detector_stress_test.cc Tue Feb 18 08:56:19 2014
@@ -33,13 +33,21 @@ class LockTest {
locks_[i].unlock();
}
+ void *A(size_t i) {
+ assert(i < n_);
+ return &locks_[i];
+ }
+
// Simple lock order onversion.
void Test1() {
fprintf(stderr, "Starting Test1\n");
// CHECK: Starting Test1
+ fprintf(stderr, "Expecting lock inversion: %p %p\n", A(0), A(1));
+ // CHECK: Expecting lock inversion: [[A1:0x[a-f0-9]*]] [[A2:0x[a-f0-9]*]]
L(0); L(1); U(0); U(1);
L(1); L(0); U(0); U(1);
// CHECK: ThreadSanitizer: lock-order-inversion (potential deadlock)
+ // CHECK-NEXT: path: [[A1]] => [[A2]] => [[A1]]
// CHECK-NOT: ThreadSanitizer:
}
More information about the llvm-commits
mailing list