Skip to content

Commit 1e9270b

Browse files
mondainmondain
committed
Merge chore/tsan-ci: ThreadSanitizer CI job and ROQR_SANITIZE option
2 parents ae9e3d0 + 5e86233 commit 1e9270b

3 files changed

Lines changed: 106 additions & 0 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,35 @@ jobs:
5252
run: cmake --build build --parallel
5353
- name: Test
5454
run: ctest --test-dir build --output-on-failure
55+
56+
tsan:
57+
name: thread-sanitizer (clang)
58+
runs-on: ubuntu-latest
59+
steps:
60+
- uses: actions/checkout@v4
61+
- name: Install dependencies
62+
run: sudo apt-get update && sudo apt-get install -y
63+
cmake clang ninja-build libssl-dev ffmpeg
64+
- name: Build picoquic deps
65+
run: eval "$(scripts/setup_picoquic_deps.sh)"
66+
- name: Configure (TSAN; ROQR targets instrumented, picoquic is not)
67+
env:
68+
CC: clang
69+
CXX: clang++
70+
run: |
71+
eval "$(scripts/setup_picoquic_deps.sh)"
72+
cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug \
73+
-DROQR_SANITIZE=thread \
74+
-DROQR_BUILD_QUIC=ON -DROQR_BUILD_RTMP=ON \
75+
-DROQR_BUILD_TOOLS=ON -DROQR_BUILD_EXAMPLES=ON \
76+
-DROQR_BUILD_TESTS=ON -DROQR_BUILD_JNI=OFF
77+
- name: Build
78+
run: cmake --build build --parallel
79+
# halt_on_error=0 lets a single test report every race it hits; TSAN still
80+
# exits non-zero if any (unsuppressed) race is found, failing the job. The
81+
# suppressions file covers only picoquic's internal wake-up-pipe fd races
82+
# (see cmake/tsan.supp); races in ROQR code are unsuppressed and fail CI.
83+
- name: Test (TSAN)
84+
env:
85+
TSAN_OPTIONS: "halt_on_error=0 history_size=7 second_deadlock_stack=1 suppressions=${{ github.workspace }}/cmake/tsan.supp"
86+
run: ctest --test-dir build --output-on-failure --timeout 300

‎CMakeLists.txt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,17 @@ option(ROQR_BUILD_EXAMPLES "Build gateway library and example apps" ON)
2424
option(ROQR_BUILD_FFI "Build the C FFI shared library" ON)
2525
option(ROQR_BUILD_JNI "Build JNI bindings (needs a JDK)" OFF)
2626

27+
# Sanitizer selection (thread, address, undefined; empty = none). The flags are
28+
# applied per-target at the end of this file to ROQR's own code ONLY — never the
29+
# vendored picoquic, whose internal network-thread and PRNG data races we cannot
30+
# fix and which would otherwise drown out races in our code. See the
31+
# ROQR_SANITIZE block at the bottom.
32+
set(ROQR_SANITIZE "" CACHE STRING
33+
"Enable a sanitizer: thread, address, or undefined (empty = none)")
34+
if(ROQR_SANITIZE)
35+
message(STATUS "Building ROQR targets with -fsanitize=${ROQR_SANITIZE}")
36+
endif()
37+
2738
# The SHARED roqr-ffi library links the static libraries (and picoquic), so
2839
# they must be position-independent — but only when FFI is actually built.
2940
# Setting this directory-scoped variable before add_subdirectory propagates
@@ -92,3 +103,38 @@ if(ROQR_BUILD_TESTS)
92103
enable_testing()
93104
add_subdirectory(tests)
94105
endif()
106+
107+
# Apply the selected sanitizer to ROQR's own targets only. Instrumenting the
108+
# vendored picoquic (compiled from source under .deps) is deliberately avoided:
109+
# its network-thread teardown and global-PRNG paths have internal data races we
110+
# cannot fix from here, and instrumenting it drowns the signal in third-party
111+
# noise. Because our code — including the on_message/on_closed callbacks that
112+
# run on picoquic's network thread — stays instrumented, TSAN still detects real
113+
# races in ROQR (both racing accesses land in our instrumented code); picoquic's
114+
# uninstrumented internals simply aren't tracked. Fetched deps (Catch2, under
115+
# the build tree) are skipped for the same reason.
116+
if(ROQR_SANITIZE)
117+
function(_roqr_collect_targets dir out_var)
118+
get_property(_subs DIRECTORY "${dir}" PROPERTY SUBDIRECTORIES)
119+
get_property(_acc DIRECTORY "${dir}" PROPERTY BUILDSYSTEM_TARGETS)
120+
foreach(_sub IN LISTS _subs)
121+
_roqr_collect_targets("${_sub}" _child)
122+
list(APPEND _acc ${_child})
123+
endforeach()
124+
set(${out_var} ${_acc} PARENT_SCOPE)
125+
endfunction()
126+
127+
_roqr_collect_targets("${CMAKE_SOURCE_DIR}" _roqr_all_targets)
128+
set(_roqr_san_flags -fsanitize=${ROQR_SANITIZE} -fno-omit-frame-pointer -g)
129+
foreach(_tgt IN LISTS _roqr_all_targets)
130+
get_target_property(_type ${_tgt} TYPE)
131+
get_target_property(_src_dir ${_tgt} SOURCE_DIR)
132+
if(_type MATCHES "STATIC_LIBRARY|SHARED_LIBRARY|MODULE_LIBRARY|OBJECT_LIBRARY|EXECUTABLE"
133+
AND _src_dir MATCHES "^${CMAKE_SOURCE_DIR}"
134+
AND NOT _src_dir MATCHES "\\.deps"
135+
AND NOT _src_dir MATCHES "^${CMAKE_BINARY_DIR}")
136+
target_compile_options(${_tgt} PRIVATE ${_roqr_san_flags})
137+
target_link_options(${_tgt} PRIVATE -fsanitize=${ROQR_SANITIZE})
138+
endif()
139+
endforeach()
140+
endif()

‎cmake/tsan.supp‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# ThreadSanitizer suppressions for the libroqr TSAN CI job.
2+
#
3+
# Scope: suppress races that originate ENTIRELY inside the vendored picoquic
4+
# and that libroqr cannot fix. ROQR's own code (core/ quic/ rtmp/ gateway/
5+
# tools/ ffi/) is instrumented and unsuppressed, so real races in our code
6+
# fail CI. picoquic itself is built uninstrumented, so its internal memory
7+
# races are not tracked -- but TSAN's syscall interceptors (close/read/pipe)
8+
# still flag fd-lifecycle races in picoquic's socket loop regardless of
9+
# instrumentation, which is what the entries below cover.
10+
#
11+
# Format: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions
12+
#
13+
# picoquic manages a wake-up pipe for its network thread: the loop thread
14+
# reads it (picoquic_packet_loop_poll) while teardown closes it
15+
# (picoquic_close_network_wake_up, from picoquic_delete_network_thread) and
16+
# setup creates it (picoquic_open_network_wake_up, from
17+
# picoquic_start_custom_network_thread_qmux). TSAN reports an fd race across
18+
# these because the happens-before edge lives inside picoquic's uninstrumented
19+
# thread join. We drive this only through the documented Client lifecycle
20+
# (connect / destroy) and never touch the fd ourselves, so it is not fixable
21+
# from libroqr. These functions are pipe-fd-specific and never appear as the
22+
# accessing frame of a genuine race in ROQR code, so suppressing them cannot
23+
# mask a real libroqr race. Note picoquic_packet_loop_poll is deliberately NOT
24+
# suppressed: it appears in the stack of our on_message/on_closed callbacks
25+
# (which run on picoquic's network thread), so suppressing it could hide a
26+
# real race in those callbacks.
27+
race:picoquic_close_network_wake_up
28+
race:picoquic_open_network_wake_up

0 commit comments

Comments
 (0)