[Openmp-commits] [openmp] [Archer] Detect OpenMP lock/unlock misuse (PR #138690)
via Openmp-commits
openmp-commits at lists.llvm.org
Wed May 7 00:14:17 PDT 2025
https://github.com/felilxtomski updated https://github.com/llvm/llvm-project/pull/138690
>From c9a78a9ee1a0ba7679dc31435b9a85cce0cfa0a9 Mon Sep 17 00:00:00 2001
From: "felix.tomski" <tomski at itc.rwth-aachen.de>
Date: Tue, 6 May 2025 15:47:44 +0200
Subject: [PATCH 1/2] [Archer] Detect OpenMP lock/unlock misuse
---
openmp/tools/archer/ompt-tsan.cpp | 33 ++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/openmp/tools/archer/ompt-tsan.cpp b/openmp/tools/archer/ompt-tsan.cpp
index bb60fc6b603f4..456f357f4ef86 100644
--- a/openmp/tools/archer/ompt-tsan.cpp
+++ b/openmp/tools/archer/ompt-tsan.cpp
@@ -166,6 +166,8 @@ DECLARE_TSAN_FUNCTION(AnnotateNewMemory, const char *, int,
const volatile void *, size_t)
DECLARE_TSAN_FUNCTION(__tsan_func_entry, const void *)
DECLARE_TSAN_FUNCTION(__tsan_func_exit)
+DECLARE_TSAN_FUNCTION(AnnotateRWLockReleased, const char *, int,
+ const volatile void *, size_t)
}
// This marker is used to define a happens-before arc. The race detector will
@@ -190,6 +192,9 @@ DECLARE_TSAN_FUNCTION(__tsan_func_exit)
AnnotateNewMemory(__FILE__, __LINE__, addr, size)
#define TsanFreeMemory(addr, size) \
AnnotateNewMemory(__FILE__, __LINE__, addr, size)
+
+#define TsanRWLockRelease(mutex, isw) \
+ AnnotateRWLockReleased(__FILE__, __LINE__, mutex, isw)
#endif
// Function entry/exit
@@ -598,7 +603,11 @@ static inline TaskData *ToTaskData(ompt_data_t *task_data) {
}
/// Store a mutex for each wait_id to resolve race condition with callbacks.
-static std::unordered_map<ompt_wait_id_t, std::mutex> Locks;
+struct LockInfo {
+ std::mutex mu;
+ uint64_t thread_id;
+};
+static std::unordered_map<ompt_wait_id_t, LockInfo> Locks;
static std::mutex LocksMutex;
static void ompt_tsan_thread_begin(ompt_thread_t thread_type,
@@ -1109,22 +1118,31 @@ static void ompt_tsan_mutex_acquired(ompt_mutex_t kind, ompt_wait_id_t wait_id,
// Acquire our own lock to make sure that
// 1. the previous release has finished.
// 2. the next acquire doesn't start before we have finished our release.
+ TsanFuncEntry(codeptr_ra);
LocksMutex.lock();
- std::mutex &Lock = Locks[wait_id];
+ std::mutex &Lock = Locks[wait_id].mu;
+ Locks[wait_id].thread_id = ompt_get_thread_data()->value;
LocksMutex.unlock();
Lock.lock();
TsanHappensAfter(&Lock);
+ TsanFuncExit();
}
static void ompt_tsan_mutex_released(ompt_mutex_t kind, ompt_wait_id_t wait_id,
const void *codeptr_ra) {
+ TsanFuncEntry(codeptr_ra);
LocksMutex.lock();
- std::mutex &Lock = Locks[wait_id];
+ std::mutex &Lock = Locks[wait_id].mu;
+ auto lock_owner = Locks[wait_id].thread_id;
LocksMutex.unlock();
TsanHappensBefore(&Lock);
+ if (lock_owner != ompt_get_thread_data()->value)
+ TsanRWLockRelease(&wait_id, 1);
+
Lock.unlock();
+ TsanFuncExit();
}
// callback , signature , variable to store result , required support level
@@ -1179,6 +1197,12 @@ static int ompt_tsan_initialize(ompt_function_lookup_t lookup, int device_num,
exit(1);
}
+ if (ompt_get_thread_data == NULL) {
+ fprintf(stderr, "Could not get inquiry function 'ompt_get_thread_data', "
+ "exiting...\n");
+ exit(1);
+ }
+
findTsanFunction(AnnotateHappensAfter,
(void (*)(const char *, int, const volatile void *)));
findTsanFunction(AnnotateHappensBefore,
@@ -1190,6 +1214,9 @@ static int ompt_tsan_initialize(ompt_function_lookup_t lookup, int device_num,
(void (*)(const char *, int, const volatile void *, size_t)));
findTsanFunction(__tsan_func_entry, (void (*)(const void *)));
findTsanFunction(__tsan_func_exit, (void (*)(void)));
+ findTsanFunction(
+ AnnotateRWLockReleased,
+ (void (*)(const char *, int, const volatile void *, size_t)));
SET_CALLBACK(thread_begin);
SET_CALLBACK(thread_end);
>From cf92eaf09a7e94332f982cd4cf3cc1227b7bbe6a Mon Sep 17 00:00:00 2001
From: "felix.tomski" <tomski at itc.rwth-aachen.de>
Date: Wed, 7 May 2025 09:14:03 +0200
Subject: [PATCH 2/2] Detect lock/unlock issues through TSan annotations
---
openmp/tools/archer/ompt-tsan.cpp | 49 ++++++++++++++++++++++++++++---
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/openmp/tools/archer/ompt-tsan.cpp b/openmp/tools/archer/ompt-tsan.cpp
index 456f357f4ef86..d1c757e4dc753 100644
--- a/openmp/tools/archer/ompt-tsan.cpp
+++ b/openmp/tools/archer/ompt-tsan.cpp
@@ -166,6 +166,12 @@ DECLARE_TSAN_FUNCTION(AnnotateNewMemory, const char *, int,
const volatile void *, size_t)
DECLARE_TSAN_FUNCTION(__tsan_func_entry, const void *)
DECLARE_TSAN_FUNCTION(__tsan_func_exit)
+DECLARE_TSAN_FUNCTION(AnnotateRWLockCreate, const char *, int,
+ const volatile void *)
+DECLARE_TSAN_FUNCTION(AnnotateRWLockDestroy, const char *, int,
+ const volatile void *)
+DECLARE_TSAN_FUNCTION(AnnotateRWLockAcquired, const char *, int,
+ const volatile void *, size_t)
DECLARE_TSAN_FUNCTION(AnnotateRWLockReleased, const char *, int,
const volatile void *, size_t)
}
@@ -193,7 +199,14 @@ DECLARE_TSAN_FUNCTION(AnnotateRWLockReleased, const char *, int,
#define TsanFreeMemory(addr, size) \
AnnotateNewMemory(__FILE__, __LINE__, addr, size)
-#define TsanRWLockRelease(mutex, isw) \
+// Locks
+#define TsanRWLockCreate(mutex) \
+ AnnotateRWLockCreate(__FILE__, __LINE__, mutex)
+#define TsanRWLockDestroy(mutex) \
+ AnnotateRWLockDestroy(__FILE__, __LINE__, mutex)
+#define TsanRWLockAcquired(mutex, isw) \
+ AnnotateRWLockAcquired(__FILE__, __LINE__, mutex, isw)
+#define TsanRWLockReleased(mutex, isw) \
AnnotateRWLockReleased(__FILE__, __LINE__, mutex, isw)
#endif
@@ -1111,6 +1124,24 @@ static void ompt_tsan_dependences(ompt_data_t *task_data,
}
}
+static void ompt_tsan_lock_init(ompt_mutex_t kind, unsigned int hint, unsigned int impl, ompt_wait_id_t wait_id, const void * codeptr_ra) {
+ TsanFuncEntry(codeptr_ra);
+ LocksMutex.lock();
+ std::mutex &Lock = Locks[wait_id].mu;
+ LocksMutex.unlock();
+ TsanRWLockCreate(&Lock);
+ TsanFuncExit();
+}
+
+static void ompt_tsan_lock_destroy(ompt_mutex_t kind, ompt_wait_id_t wait_id, const void * codeptr_ra) {
+ TsanFuncEntry(codeptr_ra);
+ LocksMutex.lock();
+ std::mutex &Lock = Locks[wait_id].mu;
+ LocksMutex.unlock();
+ TsanRWLockDestroy(&Lock);
+ TsanFuncExit();
+}
+
/// OMPT event callbacks for handling locking.
static void ompt_tsan_mutex_acquired(ompt_mutex_t kind, ompt_wait_id_t wait_id,
const void *codeptr_ra) {
@@ -1126,6 +1157,7 @@ static void ompt_tsan_mutex_acquired(ompt_mutex_t kind, ompt_wait_id_t wait_id,
Lock.lock();
TsanHappensAfter(&Lock);
+ TsanRWLockAcquired(&Lock, 1);
TsanFuncExit();
}
@@ -1137,9 +1169,7 @@ static void ompt_tsan_mutex_released(ompt_mutex_t kind, ompt_wait_id_t wait_id,
auto lock_owner = Locks[wait_id].thread_id;
LocksMutex.unlock();
TsanHappensBefore(&Lock);
-
- if (lock_owner != ompt_get_thread_data()->value)
- TsanRWLockRelease(&wait_id, 1);
+ TsanRWLockReleased(&Lock, 1);
Lock.unlock();
TsanFuncExit();
@@ -1214,6 +1244,15 @@ static int ompt_tsan_initialize(ompt_function_lookup_t lookup, int device_num,
(void (*)(const char *, int, const volatile void *, size_t)));
findTsanFunction(__tsan_func_entry, (void (*)(const void *)));
findTsanFunction(__tsan_func_exit, (void (*)(void)));
+ findTsanFunction(
+ AnnotateRWLockCreate,
+ (void (*)(const char *, int, const volatile void *)));
+ findTsanFunction(
+ AnnotateRWLockDestroy,
+ (void (*)(const char *, int, const volatile void *)));
+ findTsanFunction(
+ AnnotateRWLockAcquired,
+ (void (*)(const char *, int, const volatile void *, size_t)));
findTsanFunction(
AnnotateRWLockReleased,
(void (*)(const char *, int, const volatile void *, size_t)));
@@ -1231,6 +1270,8 @@ static int ompt_tsan_initialize(ompt_function_lookup_t lookup, int device_num,
SET_CALLBACK_T(mutex_acquired, mutex);
SET_CALLBACK_T(mutex_released, mutex);
+ SET_CALLBACK_T(lock_init, mutex_acquire);
+ SET_CALLBACK_T(lock_destroy, mutex);
SET_OPTIONAL_CALLBACK_T(reduction, sync_region, hasReductionCallback,
ompt_set_never);
More information about the Openmp-commits
mailing list