Re: URCU feature request?

[Thobias Knudsen via lttng-dev <lttng-dev@lists.lttng.org>]

[-- Attachment #1: Type: text/plain, Size: 6862 bytes --]

>Since 0.15.0, we've introduce an annotation layer (not part of the
>public API), making TSAN compatible with URCU.  See
>`include/urcu/annotate.h' and the `CMM_SANITIZE_THREAD' macro.
>
>However, IIRC, you also need to compile URCU with the configuration
>option `--enable-compiler-atomic-builtins' so that atomic operations are
>implemented with the configured toolchain's builtin atomics.  I don't
>recall if this was strictly necessary.
>
>If you encounter false positives with TSAN, please send me a minimal
>reproducible example together with:
>
> - the toolchain you’re using
>
> - the version of URCU
>
> - the configuration flags you used
>
>I will be happy to have a look.

Sorry for the late answer. I had to find the time to fix some bugs to get
it all running before switching to urcu v0.15.0 and setting the flags and
macros for thread sanitizer support. Creating a minimum reproducible
example would take some time as I don't have any good idea of the
exact place where the thread sanitizer issues arise because they are
scattered all over tsm.c and test_tsm.c. If you have linux debian family
then you can clone my repo and run it:
https://github.com/ThobiasKnudsen/Logos. ./scripts/build.sh --debug --tsan
&& ./build/bin/test_tsm. I tried running with and without thread sanitizer
support and got the same issues. In the test_tsm when only one thread is
running, the thread sanitizer produces warnings still. I did find a place
where the thread sanitizer said there was a race and the write was within a
node not yet inserted into the cds_lfht. That seems like a false positive.
The read for the same case should be inside rcu_read_lock as everything
within lines 263 to 904 inside test_tsm.c is within a read section.

  Read of size 1 at 0x7b1400085ea1 by thread T6:
    #0 tsm_base_node_is_valid /Logos/src/tsm.c:721 (test_tsm+0xa548)
    #1 _tsm_tsm_type_is_valid /Logos/src/tsm.c:364 (test_tsm+0xb1bf)
    #2 tsm_node_is_valid /Logos/src/tsm.c:1553 (test_tsm+0x9984)
    #3 tsm_node_defer_free /Logos/src/tsm.c:1888 (test_tsm+0xb2fc)
    #4 stress_thread /Logos/src/tests/test_tsm.c:549 (test_tsm+0x46a6)

  Previous write of size 1 at 0x7b1400085ea1 by thread T9:
    #0 tsm_base_node_create /Logos/src/tsm.c:683 (test_tsm+0x6f2a)
    #1 stress_thread /Logos/src/tests/test_tsm.c:843 (test_tsm+0x5ce0)

>I don’t see why that would be a problem for the static-check algorithm I
>described above.  If a pointer needs to be protected by a mutex for
>mutation, that falls outside the scope of RCU, as far as I know.

If I understand correctly the __rcu checks that reads are not done outside
read sections and unsafe writes are not done at all. If this is correct
then if you are using custom concurrency for __rcu protected data outside
the read section or unsafe writes is that allowed? Because if it's not that
would be a limitation for __rcu.

man. 8. sep. 2025 kl. 02:10 skrev Olivier Dion <odion@efficios.com>:

> On Sun, 07 Sep 2025, Thobias Knudsen <thobknu@gmail.com> wrote:
> >> It looks like you want runtime verification for the usage of the API.
> >> Did you know that URCU can now be compiled against ThreadSanitizer
> >> (TSAN)?  If a user misuses the API or makes incorrect assumptions about
> >> the guarantees offered by RCU, TSAN will most likely detect those
> >> issues.  Coupled with the other debug features we already have, this
> >> makes it very hard to not trigger an error path when the API is used
> >> incorrectly.
> >
> > Really?! I've used TSAN and got a bunch of false positives, I believe,
> but
> > maybe they're not false positives? How do you remove the false positives,
> > or know that they're not false positives?
>
> Since 0.15.0, we've introduce an annotation layer (not part of the
> public API), making TSAN compatible with URCU.  See
> `include/urcu/annotate.h' and the `CMM_SANITIZE_THREAD' macro.
>
> However, IIRC, you also need to compile URCU with the configuration
> option `--enable-compiler-atomic-builtins' so that atomic operations are
> implemented with the configured toolchain's builtin atomics.  I don't
> recall if this was stricly necessary.
>
> If you encounter false positives with TSAN, please send me a minimal
> reproducible example together with:
>
>  - the toolchain you’re using
>
>  - the version of URCU
>
>  - the configuration flags you used
>
> I will be happy to have a look.
>
> >> Note that certain kind of errors could actually be flag at compile time
> >> with the proper tooling.  For example, the Linux kernel uses a `__rcu'
> >> attribute that Sparse can understand to flag improper use of
> >> RCU‑protected pointers.  I’d be very open to exposing something similar
> >> (an attribute) for static checkers.
> >
> > wow thanks for the info! I knew compile time checks would be possible but
> > requiring compiler operability which is a higher hanging fruit for me.
>
> I don’t know the details of `__rcu' from the Linux kernel. I think it’s
> just a macro that expands to nothing by default, but Sparse treats it as
> an attribute.  I’m not sure exactly what checks Sparse performs with it,
> but I suspect it involves traversing the program’s control-flow graph
> (CFG), ensuring that pointers marked with the `__rcu' qualifier are:
>
>   - obtained via rcu_dereference
>
>   - only dereferenced under RCU lock protection
>
> > Is '__rcu' compatible with custom concurrency? For example
> > rcu_dereference a pointer then locking a mutex inside the pointer then
> > unlock read then continue using the pointer?
>
> I don’t see why that would be a problem for the static-check algorithm I
> described above.  If a pointer needs to be protected by a mutex for
> mutation, that falls outside the scope of RCU, as far as I know.
>
> > I cant come up with something usefull other than a language rework. Is
> > it much work making the __urcu attribute?
>
> I suppose not.  On top of my head, it would involve adding some pointer
> qualifier and function attributes to the primitives exposed by URCU.
> Users would also need to use the pointer qualifier when working with
> RCU-protected pointers.  The qualifier and the attributes would expand
> to nothing by default, letting static checkers defining them to internal
> values.  I suggest you read `Documentation/RCU/rcu_dereference.rst' in
> the Linux kernel tree if you are interested.
>
> In its current state, this would not be very useful because none of the
> major compilers provide static analysis for RCU.  However, implementing
> such analysis, as a plugin, wouldn’t be overly difficult for someone
> familiar with Clang or GCC, I suppose.
>
> [...]
>
> Thanks,
> Olivier
> --
> Olivier Dion
> EfficiOS Inc.
> https://www.efficios.com
>

[-- Attachment #2: Type: text/html, Size: 7976 bytes --]