Skip to content

Commit c8eae59

Browse files
myrrcmyrrc
authored
Add tsan for vortex-ffi, fix asan instrumentation (#7157)
- Add proper instrumentation for vortex-ffi (didn't check memory leaks before due to Mimalloc). - Add memory sanitizer and thread sanitizer runtimes for Rust. - Remove `verbosity=1` from sanitizer output as it's too noisy. - Remove `VX_DUCKDB_DEBUG` since we don't check duckdb code in these jobs. - Make address sanitizer fail on error to fail Github action job. - Add cross Rust-C address and thread sanitizers. - Fix existing sanitizer errors (memory leak in vx_sink). - Allow building FFI C part library with Rust's target triple via `TARGET_TRIPLE` CMake option. Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
1 parent 86c3568 commit c8eae59

5 files changed

Lines changed: 237 additions & 65 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 108 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -379,51 +379,132 @@ jobs:
379379
flags: ${{ matrix.suite }}
380380
use_oidc: true
381381

382-
rust-test:
383-
name: "Rust tests (sanitizer)"
384-
timeout-minutes: 40
382+
rust-test-sanitizer:
383+
strategy:
384+
fail-fast: false
385+
matrix:
386+
include:
387+
- sanitizer: asan
388+
sanitizer_flags: "-Zsanitizer=address -Zsanitize=leak"
389+
- sanitizer: msan
390+
sanitizer_flags: "-Zsanitizer=memory"
391+
- sanitizer: tsan
392+
sanitizer_flags: "-Zsanitizer=thread"
393+
name: "Rust tests (${{ matrix.sanitizer }})"
385394
runs-on: >-
386395
${{ github.repository == 'vortex-data/vortex'
387396
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=rust-test-sanitizer', github.run_id)
388397
|| 'ubuntu-latest' }}
398+
timeout-minutes: 40
389399
env:
390-
# Add debug symbols and enable ASAN/LSAN with better output
391-
ASAN_OPTIONS: "symbolize=1:print_stats=1:check_initialization_order=1:detect_leaks=1:halt_on_error=0:verbosity=1:leak_check_at_exit=1"
392-
LSAN_OPTIONS: "verbosity=1:report_objects=1"
400+
ASAN_OPTIONS: "symbolize=1:check_initialization_order=1:detect_leaks=1:leak_check_at_exit=1"
401+
LSAN_OPTIONS: "report_objects=1"
393402
ASAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
394-
# Link against DuckDB debug build
395-
VX_DUCKDB_DEBUG: "1"
396-
# Keep frame pointers for better stack traces
397-
CARGO_PROFILE_DEV_DEBUG: "true"
398-
CARGO_PROFILE_TEST_DEBUG: "true"
399-
# Skip slow tests that are too expensive under sanitizer
403+
MSAN_OPTIONS: "symbolize=1"
404+
MSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
405+
TSAN_OPTIONS: "symbolize=1"
406+
TSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
400407
VORTEX_SKIP_SLOW_TESTS: "1"
408+
# -Cunsafe-allow-abi-mismatch=sanitizer: libraries like compiler_builtins
409+
# unset -Zsanitizer flag and we should allow that.
410+
RUSTFLAGS: "-A warnings -Cunsafe-allow-abi-mismatch=sanitizer --cfg disable_loom --cfg vortex_nightly -C debuginfo=2 -C opt-level=0 -C strip=none"
401411
steps:
402412
- uses: runs-on/action@v2
403413
if: github.repository == 'vortex-data/vortex'
404414
with:
405415
sccache: s3
406416
- uses: actions/checkout@v6
407417
- uses: ./.github/actions/setup-prebuild
408-
- name: Install nightly for sanitizer
418+
- name: Install Rust nightly toolchain
409419
run: |
410420
rustup toolchain install $NIGHTLY_TOOLCHAIN
411421
rustup component add --toolchain $NIGHTLY_TOOLCHAIN rust-src rustfmt clippy llvm-tools-preview
412-
- name: Rust Tests
413-
env:
414-
RUSTFLAGS: "-A warnings -Zsanitizer=address -Zsanitizer=leak --cfg disable_loom --cfg vortex_nightly -C debuginfo=2 -C opt-level=0 -C strip=none"
422+
export RUSTFLAGS="${RUSTFLAGS} ${{ matrix.sanitizer_flags }}"
423+
- name: Build tests with sanitizer
415424
run: |
416-
# Build with full debug info first (helps with caching)
417-
cargo +$NIGHTLY_TOOLCHAIN build --locked --all-features \
418-
--target x86_64-unknown-linux-gnu \
419-
-p vortex-buffer -p vortex-ffi -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
420-
# Run tests with sanitizers and debug output
421-
cargo +$NIGHTLY_TOOLCHAIN nextest run \
422-
--locked \
423-
--all-features \
424-
--no-fail-fast \
425-
--target x86_64-unknown-linux-gnu \
426-
-p vortex-buffer -p vortex-ffi -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
425+
cargo +$NIGHTLY_TOOLCHAIN build --locked --all-features \
426+
--target x86_64-unknown-linux-gnu -Zbuild-std \
427+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
428+
429+
- name: Run tests with sanitizer
430+
run: |
431+
cargo +$NIGHTLY_TOOLCHAIN nextest run --locked --all-features \
432+
--target x86_64-unknown-linux-gnu --no-fail-fast -Zbuild-std \
433+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
434+
435+
# vortex-ffi requires --no-default-features as otherwise we pull in
436+
# Mimalloc which interferes with sanitizers
437+
# cargo nextest reports less sanitizer issues than cargo test
438+
# TODO(myrrc): remove --no-default-features once we make Mimalloc opt-in
439+
- name: Run vortex-ffi tests with sanitizer
440+
run: |
441+
cargo +$NIGHTLY_TOOLCHAIN test --locked --no-default-features \
442+
--target x86_64-unknown-linux-gnu --no-fail-fast -Zbuild-std \
443+
-p vortex-ffi -- --no-capture
444+
445+
rust-ffi-test-sanitizer:
446+
strategy:
447+
fail-fast: false
448+
matrix:
449+
include:
450+
# We don't run memory sanitizer as it's clang-only and provides many
451+
# false positives for Catch2
452+
- sanitizer: asan
453+
sanitizer_flags: "-Zsanitizer=address -Zsanitize=leak"
454+
- sanitizer: tsan
455+
sanitizer_flags: "-Zsanitizer=thread"
456+
name: "Rust/C++ FFI tests (${{ matrix.sanitizer }})"
457+
timeout-minutes: 40
458+
env:
459+
ASAN_OPTIONS: "symbolize=1:check_initialization_order=1:detect_leaks=1:leak_check_at_exit=1"
460+
LSAN_OPTIONS: "report_objects=1"
461+
ASAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
462+
MSAN_OPTIONS: "symbolize=1"
463+
MSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
464+
TSAN_OPTIONS: "symbolize=1"
465+
TSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
466+
VORTEX_SKIP_SLOW_TESTS: "1"
467+
# -Cunsafe-allow-abi-mismatch=sanitizer: libraries like compiler_builtins
468+
# unset -Zsanitizer flag and we should allow that.
469+
runs-on: >-
470+
${{ github.repository == 'vortex-data/vortex'
471+
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=rust-ffi-test-sanitizer', github.run_id)
472+
|| 'ubuntu-latest' }}
473+
steps:
474+
- uses: runs-on/action@v2
475+
if: github.repository == 'vortex-data/vortex'
476+
with:
477+
sccache: s3
478+
- uses: actions/checkout@v6
479+
- uses: ./.github/actions/setup-prebuild
480+
- name: Install rustfilt
481+
run: |
482+
cargo install rustfilt
483+
- name: Install Rust nightly toolchain
484+
run: |
485+
rustup toolchain install $NIGHTLY_TOOLCHAIN
486+
rustup component add --toolchain $NIGHTLY_TOOLCHAIN rust-src rustfmt clippy llvm-tools-preview
487+
488+
# Export flags here so that rustfilt won't be built with sanitizers
489+
export RUSTFLAGS="-A warnings -Cunsafe-allow-abi-mismatch=sanitizer \
490+
--cfg disable_loom --cfg vortex_nightly -C debuginfo=2 \
491+
-C opt-level=0 -C strip=none -Zexternal-clangrt \
492+
${{ matrix.sanitizer_flags }}"
493+
- name: Build FFI library
494+
run: |
495+
# TODO(myrrc): remove --no-default-features
496+
cargo +$NIGHTLY_TOOLCHAIN build --locked --no-default-features \
497+
--target x86_64-unknown-linux-gnu -Zbuild-std \
498+
-p vortex-ffi
499+
- name: Build FFI library tests
500+
run: |
501+
cd vortex-ffi
502+
cmake -Bbuild -DBUILD_TESTS=1 -DSANITIZER=${{ matrix.sanitizer }} -DTARGET_TRIPLE="x86_64-unknown-linux-gnu"
503+
cmake --build build -j
504+
- name: Run tests
505+
run: |
506+
set -o pipefail
507+
./vortex-ffi/build/test/vortex_ffi_test 2>&1 | rustfilt -i-
427508
428509
cuda-build-lint:
429510
if: github.repository == 'vortex-data/vortex'

‎vortex-ffi/CMakeLists.txt‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,40 @@ cmake_minimum_required(VERSION 3.10)
55
project(VortexFFI
66
VERSION 0.0.1
77
LANGUAGES C)
8-
set(CMAKE_C_STANDARD 17)
9-
set(CMAKE_CXX_STANDARD 20)
10-
set(CMAKE_CXX_STANDARD_REQUIRED ON)
118
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
9+
set(CMAKE_C_STANDARD 17)
1210
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror -Wextra -Wpedantic")
13-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -Wextra -Wpedantic")
1411

15-
option(BUILD_TESTING "Build tests" ON)
12+
option(BUILD_TESTS "Build tests" OFF)
13+
14+
set(SANITIZER "" CACHE STRING "Build with sanitizers")
15+
set(TARGET_TRIPLE "" CACHE STRING "Rust target triple for FFI library")
1616

1717
if (NOT CMAKE_BUILD_TYPE)
1818
set(CMAKE_BUILD_TYPE Debug)
1919
endif()
2020

21+
if (NOT SANITIZER STREQUAL "")
22+
message(NOTICE "Sanitizer: ${SANITIZER}")
23+
if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
24+
message(FATAL_ERROR "Only debug build is supported for sanitizer builds")
25+
endif()
26+
27+
if (SANITIZER STREQUAL "asan")
28+
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address,undefined,leak")
29+
if (BUILD_TESTS)
30+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address,undefined,leak")
31+
endif()
32+
elseif (SANITIZER STREQUAL "tsan")
33+
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread")
34+
if (BUILD_TESTS)
35+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread")
36+
endif()
37+
else()
38+
message(FATAL_ERROR "Unknown sanitizer ${SANITIZER}")
39+
endif()
40+
endif()
41+
2142
message(NOTICE "Build type: ${CMAKE_BUILD_TYPE}")
2243
if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND
2344
NOT CMAKE_BUILD_TYPE STREQUAL "Release" AND
@@ -26,11 +47,11 @@ if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND
2647
endif()
2748

2849
if (CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
29-
set(LIBRARY_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../target/release_debug")
50+
set(CMAKE_BUILD_TYPE_LOWER "release_debug")
3051
else()
3152
string(TOLOWER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE_LOWER)
32-
set(LIBRARY_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../target/${CMAKE_BUILD_TYPE_LOWER}")
3353
endif()
54+
set(LIBRARY_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../target/${TARGET_TRIPLE}/${CMAKE_BUILD_TYPE_LOWER}")
3455

3556
if(WIN32)
3657
set(LIBRARY_PATH "${LIBRARY_DIR}/libvortex_ffi.lib")
@@ -71,8 +92,12 @@ set_target_properties(vortex_ffi_shared PROPERTIES
7192
INTERFACE_LINK_OPTIONS "LINKER:-rpath,${LIBRARY_DIR}"
7293
)
7394

74-
if (BUILD_TESTING)
95+
if (BUILD_TESTS)
7596
enable_language(CXX)
97+
set(CMAKE_CXX_STANDARD 20)
98+
set(CMAKE_CXX_STANDARD_REQUIRED ON)
99+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -Wextra -Wpedantic")
100+
76101
enable_testing()
77102
add_subdirectory(test)
78103
endif()

‎vortex-ffi/README.md‎

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,32 +26,90 @@ item cannot be referenced in the documentation if it does not have a documentati
2626

2727
## Updating Headers
2828

29-
To rebuild the header file (requires nightly toolchain):
29+
To rebuild the header file:
3030

31-
```shell
31+
```sh
3232
cargo +nightly build -p vortex-ffi
3333
```
3434

3535
The header generation uses cbindgen's macro expansion feature which requires nightly.
3636
Stable builds use the checked-in header file at `cinclude/vortex.h`.
3737

38-
### Development Workflow
39-
40-
- **For header changes**: Use nightly toolchain to regenerate headers after modifying FFI code
41-
- **For regular development**: Stable toolchain builds work with existing checked-in headers
42-
- **CI validation**: Automated checks verify header freshness using nightly toolchain
43-
44-
### Testing
38+
### Testing C part
4539

4640
Build the test library
4741

48-
```
42+
```sh
4943
cmake -Bbuild
5044
cmake --build build -j $(nproc)
5145
```
5246

5347
Run the tests
5448

55-
```
49+
```sh
5650
ctest --test-dir build -j $(nproc)
5751
```
52+
53+
You would need C++ compiler toolchain to run the tests since they use Catch2.
54+
55+
### Testing Rust part with sanitizers
56+
57+
AddressSanitizer:
58+
59+
```sh
60+
# inside vortex-ffi
61+
RUSTFLAGS="-Z sanitizer=address" \
62+
cargo +nightly test -Zbuild-std \
63+
--no-default-features --target <target triple> \
64+
-- --no-capture
65+
```
66+
67+
MemorySanitizer:
68+
69+
```sh
70+
RUSTFLAGS="-Z sanitizer=memory -Cunsafe-allow-abi-mismatch=sanitizer" \
71+
cargo +nightly test -Zbuild-std \
72+
--no-default-features --target <target triple> \
73+
-- --no-capture
74+
```
75+
76+
ThreadSanitizer:
77+
78+
```sh
79+
RUSTFLAGS="-Z sanitizer=thread -Cunsafe-allow-abi-mismatch=sanitizer" \
80+
cargo +nightly test -Zbuild-std \
81+
--no-default-features --target <target triple> \
82+
-- --no-capture
83+
```
84+
85+
- `-Zbuild-std` is needed as memory and thread sanitizers report std errors
86+
otherwise.
87+
- `--no-default-features` is needed as we use Mimalloc otherwise which interferes
88+
with sanitizers.
89+
- `allow-abi-mismatch` is safe because in our dependency graph only crates like
90+
`compiler_builtins` unset sanitization, and they do it on purpose.
91+
- Make sure to use `cargo test` and not `cargo nextest` as nextest reports less
92+
leaks.
93+
- If you want stack trace symbolization, install `llvm-symbolizer`.
94+
95+
### Testing Rust and C with sanitizers
96+
97+
1. Build FFI library with external sanitizer runtime:
98+
99+
```sh
100+
RUSTFLAGS="-Zsanitizer=address -Zexternal-clangrt" \
101+
cargo +nightly build -Zbuild-std --target=<target triple> \
102+
--no-default-features -p vortex-ffi
103+
```
104+
105+
2. Build tests with target triple
106+
107+
```sh
108+
cmake -Bbuild -DWITH_ASAN=1 -DTARGET_TRIPLE=<target triple>
109+
```
110+
111+
3. Run the tests (ctest doesn't output failures in detail):
112+
113+
```
114+
./build/test/vortex_ffi_test 2>& 1 | rustfilt -i-
115+
```

0 commit comments

Comments
 (0)