Re: [RFC 1/1] build: support ThreadSanitizer

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 8 Apr 2025 21:05:56 +0800
Dengdui Huang <huangdengdui@huawei.com> 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 <huangdengdui@huawei.com>
> ---

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`.