I forgot to say Thanks, Thobias
fre. 19. sep. 2025 kl. 22:39 skrev Thobias Knudsen <thob...@gmail.com>: > >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 <od...@efficios.com>: > >> On Sun, 07 Sep 2025, Thobias Knudsen <thob...@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 >> >
