Skip to content

Valgrind and Thread Sanitizer (TSan) instrumentation #886

Closed
@vinser52

Description

@vinser52

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

  1. For Valgrind (using ANNOTATE_HAPPENS_BEFORE and ANNOTATE_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.
  1. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions