On Tue, 8 Apr 2025 21:05:56 +0800 Dengdui Huang <[email protected]> wrote:
> ThreadSanitizer is a tool that detects data races. It consists of a > compiler instrumentation module and a run-time library. > Typical slowdown introduced by ThreadSanitizer is about 5x-15x. > Typical memory overhead introduced by ThreadSanitizer is about 5x-10x. > > UBSan is integrated with gcc and clang and can be enabled via a meson > option: -Db_sanitize=thread. > > Signed-off-by: Dengdui Huang <[email protected]> > --- This didn't look right to me, so asked AI for review. Please address these comments. Review: [RFC 1/1] build: support ThreadSanitizer Patch 1/1 - build: support ThreadSanitizer Commit message: Warning: The commit message says "UBSan is integrated with gcc and clang and can be enabled via a meson option: -Db_sanitize=thread." This is confusing -- UBSan is the Undefined Behavior Sanitizer, not the Thread Sanitizer. This sentence appears to be copy-pasted from the UBSan block and should say "TSan is integrated with gcc and clang..." Error: The patch uses `get_option('b_sanitize') == 'thread'` (exact string match) but upstream ASan and UBSan blocks use `.contains()`. The meson `b_sanitize` option accepts comma-separated values like `'address,undefined'`. Using `== 'thread'` means the TSan block will NOT trigger if someone passes `-Db_sanitize=thread,undefined` or any compound value. Must use `.contains('thread')` to match the existing pattern: if get_option('b_sanitize').contains('thread') Warning: The ASan and UBSan blocks for GCC both add the library to link arguments and to `dpdk_extra_ldflags` so that downstream consumers (pkg-config, static linking) also get the flag. The TSan block is missing both: add_project_link_arguments('-ltsan', language: 'c') dpdk_extra_ldflags += '-ltsan' Without these, the DPDK build itself may link (meson's `b_sanitize` adds `-fsanitize=thread` to the compiler), but applications linking against DPDK's static libraries via pkg-config will fail to resolve TSan symbols. Info: TSan and ASan are mutually exclusive -- they cannot be used together. Consider adding a check at the top of the TSan block: if get_option('b_sanitize').contains('address') error('TSan and ASan cannot be used together') endif This would give a clear error message instead of cryptic linker failures. Info: It may be worth adding a `dpdk_conf.set10('RTE_THREAD_SANITIZER', true)` define (analogous to `RTE_MALLOC_ASAN`) so that DPDK code can use `#if defined(RTE_THREAD_SANITIZER)` to annotate custom synchronization primitives with TSan annotations (`__tsan_acquire`, `__tsan_release`, etc.). Without these annotations, TSan will produce many false positives on DPDK's custom locking (rte_spinlock, rte_rwlock, rte_ring) since these use inline atomics that TSan cannot automatically recognize as synchronization. This would be the main value of build-level TSan support beyond what the user gets from just passing `-Db_sanitize=thread`.
