Closed
Description
During the investigation of the CI valgrind failures of PR #736, the concern raised in my mind if we correctly instrumented atomic-store-with-release and atomic-load-with-acquire operations.
Current instrumentation in UMF
In utils_concurrency.h
there are two macros:
#define utils_atomic_load_acquire(object, dest) \
do { \
utils_annotate_acquire((void *)object); \
__atomic_load(object, dest, memory_order_acquire); \
} while (0)
#define utils_atomic_store_release(object, desired) \
do { \
__atomic_store_n(object, desired, memory_order_release); \
utils_annotate_release((void *)object); \
} while (0)
The utils_annotate_acquire
and utils_annotate_release
functions defined in the utils_sanitizers.h
as follows:
static inline void utils_annotate_acquire(void *ptr) {
#if __SANITIZE_THREAD__
__tsan_acquire(ptr);
#elif UMF_VG_HELGRIND_ENABLED || UMF_VG_DRD_ENABLED
ANNOTATE_HAPPENS_AFTER(ptr);
#else
(void)ptr;
#endif
}
static inline void utils_annotate_release(void *ptr) {
#if __SANITIZE_THREAD__
__tsan_release(ptr);
#elif UMF_VG_HELGRIND_ENABLED || UMF_VG_DRD_ENABLED
ANNOTATE_HAPPENS_BEFORE(ptr);
#else
(void)ptr;
#endif
}
My understanding of Annotation Placement:
Note: My understanding after searching in internet
- For Valgrind (using
ANNOTATE_HAPPENS_BEFORE
andANNOTATE_HAPPENS_AFTER
):
ANNOTATE_HAPPENS_BEFORE
should be placed immediately before the actual atomic-store-with-release.ANNOTATE_HAPPENS_AFTER
should be placed immediately after the actual atomic_load with acquire semantics.
- For Thread Sanitizer (TSan) (using
__tsan_release
and__tsan_acquire
):
__tsan_release
should be placed immediately after the actual atomic_store with release semantics.__tsan_acquire
should be placed immediately after the actual atomic_load with acquire semantics.