From 388eb2538ce2809aaf5159f0d36a87d0f21224fb Mon Sep 17 00:00:00 2001 From: Johan511 Date: Wed, 7 Feb 2024 14:37:30 +0530 Subject: [PATCH] adding comments to explain tsan annotations, updating change log --- Changes | 5 +++++ runtime/caml/tsan.h | 9 +++++---- runtime/major_gc.c | 8 +++++++- runtime/shared_heap.c | 4 ++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Changes b/Changes index 0e81f2fa70d9..b929cc75dbb6 100644 --- a/Changes +++ b/Changes @@ -68,6 +68,11 @@ _______________ ### Bug fixes: +- #11040: Silences false data race observed between caml_shared_try_alloc + and oldify. Introduces macros to call tsan annotations which help annotate + a ``happens before'' relationship. + (Olivier Nicole, Hari Hara Naveen S) + - #12888: fix printing of uncaught exceptions in `.cmo` files passed on the command-line of the toplevel. (Nicolás Ojeda Bär, review by Florian Angeletti, report by Daniel Bünzli) diff --git a/runtime/caml/tsan.h b/runtime/caml/tsan.h index 3ae011f01054..2f1df2f4fa56 100644 --- a/runtime/caml/tsan.h +++ b/runtime/caml/tsan.h @@ -40,8 +40,9 @@ # endif #endif -/* TSAN annotates a release operation on encountering ANNOTATE_HAPPENS_BEFORE - * and similarly an acquire operation on encountering ANNOTATE_HAPPENS_AFTER */ +/* TSan records a release operation on encountering ANNOTATE_HAPPENS_BEFORE + * and similarly an acquire operation on encountering ANNOTATE_HAPPENS_AFTER. + These annotations are used to eliminate false positives. */ #define CAML_TSAN_ANNOTATE_HAPPENS_BEFORE(addr) #define CAML_TSAN_ANNOTATE_HAPPENS_AFTER(addr) @@ -49,9 +50,9 @@ # undef CAML_TSAN_ANNOTATE_HAPPENS_BEFORE # undef CAML_TSAN_ANNOTATE_HAPPENS_AFTER -# define CAML_TSAN_ANNOTATE_HAPPENS_BEFORE(addr) \ +# define CAML_TSAN_ANNOTATE_HAPPENS_BEFORE(addr) \ AnnotateHappensBefore(__FILE__, __LINE__, (void *)(addr)); -# define CAML_TSAN_ANNOTATE_HAPPENS_AFTER(addr) \ +# define CAML_TSAN_ANNOTATE_HAPPENS_AFTER(addr) \ AnnotateHappensAfter(__FILE__, __LINE__, (void *)(addr)); extern void AnnotateHappensBefore(const char *f, int l, void *addr); diff --git a/runtime/major_gc.c b/runtime/major_gc.c index 4680ce97e2e9..78ccf358d310 100644 --- a/runtime/major_gc.c +++ b/runtime/major_gc.c @@ -946,6 +946,9 @@ static void mark_slice_darken(struct mark_stack* stk, value child, /* This part of the code is duplicated in do_some_marking for performance * reasons. * Changes here should probably be reflected in do_some_marking. */ + /* Annotating an acquire barrier on the header because TSan does not see the + * happens-before relationship established by address dependencies with + * initializing writes in shared_heap.c allocation (#12894) */ CAML_TSAN_ANNOTATE_HAPPENS_AFTER(Hp_val(child)); chd = Hd_val(child); if (Tag_hd(chd) == Infix_tag) { @@ -1007,7 +1010,10 @@ Caml_noinline static intnat do_some_marking(struct mark_stack* stk, /* This part of the code is a duplicate of mark_slice_darken for * performance reasons. - * Changes here should probably be reflected here in mark_slice_darken. */ + * Changes here should probably be reflected here in mark_slice_darken.*/ + /* Annotating an acquire barrier on the header because TSan does not see + * the happens-before relationship established by address dependencies + * with initializing writes in shared_heap.c allocation (#12894) */ CAML_TSAN_ANNOTATE_HAPPENS_AFTER(Hp_val(block)); header_t hd = Hd_val(block); diff --git a/runtime/shared_heap.c b/runtime/shared_heap.c index 6589fd2d6adb..44d4aa5f9703 100644 --- a/runtime/shared_heap.c +++ b/runtime/shared_heap.c @@ -453,6 +453,10 @@ value* caml_shared_try_alloc(struct caml_heap_state* local, mlsize_t wosize, } colour = caml_global_heap_state.MARKED; Hd_hp (p) = Make_header_with_reserved(wosize, tag, colour, reserved); + /* Annotating a release barrier on `p` because TSan does not see the + * happens-before relationship established by address dependencies + * between the initializing writes here and the read in major_gc.c + * marking (#12894) */ CAML_TSAN_ANNOTATE_HAPPENS_BEFORE(p); #ifdef DEBUG {