[lttng-dev] userspace-rcu and ThreadSanitizer

[Ondřej Surý]

Hey,

we use ThreadSanitizer in BIND 9 CI quite extensively and with userspace-rcu
it's lit like an American house on Saturnalia ;).

I have two questions:

1. I've tried to help TSAN by replacing the custom atomics with __atomic gcc
  primitives - that seems to work pretty well.  Note that using C11 stdatomics
  is frankly not possible here because it would require wrapping everything into
  _Atomic().

  Do you want me to contribute this back? And how should I plug this into the
  existing structure?  This touches:

include/urcu/static/pointer.h
include/urcu/system.h
include/urcu/uatomic.h


2. I know there's KTSAN, so it must work somehow, but was there any success
  on using ThreadSanitizer on projects using Userspace-RCU?  It mostly seems
  to highlight the CDS parts of the code.

I can help TSAN to understand some of the code or suppress some of the warnings,
but I do want to prevent the code to be full of stuff like this:

static void
destroy_adbname_rcu_head(struct rcu_head *rcu_head) {
        dns_adbname_t *adbname = caa_container_of(rcu_head, dns_adbname_t,
                                                  rcu_head);

#ifdef __SANITIZE_THREAD__
        SPINLOCK(&adbname->lock);
        SPINUNLOCK(&adbname->lock);
#endif

        destroy_adbname(adbname);
}

I am absolutely sure that the adbname can be destroyed here (because of the
reference counting), but TSAN had a problem with it. Doing the "fake" barrier
with a spinlock here made it stop consider this to be a data race.

I also had to disable the auto_resize of cds_lfht when running under TSAN.

I am also worried that by hiding some code from TSAN, we might miss a legitimate
error.

All I found using Google was this notice from 2014:
https://www.mail-archive.com/valgrind-users@lists.sourceforge.net/msg05121.html

and perhaps this:
https://github.com/google/sanitizers/issues/1415

(Perhaps, I should look into annotating urcu code with TSAN annotations?)

~~~~

3. As an extra bonus, this is going to be needed with clang-17 as noreturn is now
reserved word:

diff --git a/include/urcu/uatomic/generic.h b/include/urcu/uatomic/generic.h
index 89d1cfa..c3762b0 100644
--- a/include/urcu/uatomic/generic.h
+++ b/include/urcu/uatomic/generic.h
@@ -38,7 +38,7 @@ extern "C" {
 #endif

 #if !defined __OPTIMIZE__  || defined UATOMIC_NO_LINK_ERROR
-static inline __attribute__((always_inline, noreturn))
+static inline __attribute__((always_inline, __noreturn__))
 void _uatomic_link_error(void)
 {
 #ifdef ILLEGAL_INSTR
diff --git a/src/urcu-call-rcu-impl.h b/src/urcu-call-rcu-impl.h
index 187727e..cc76f53 100644
--- a/src/urcu-call-rcu-impl.h
+++ b/src/urcu-call-rcu-impl.h
@@ -1055,7 +1055,7 @@ void urcu_register_rculfhash_atfork(struct urcu_atfork *atfork)
  * This unregistration function is deprecated, meant only for internal
  * use by rculfhash.
  */
-__attribute__((noreturn))
+__attribute__((__noreturn__))
 void urcu_unregister_rculfhash_atfork(struct urcu_atfork *atfork __attribute__((unused)))
 {
        urcu_die(EPERM);


Ondrej
--
Ondřej Surý (He/Him)
ondrej at sury.org