Skip to content

Commit b9a88a4

Browse files
committed
introduce sanitizers in CI
Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
1 parent 41b997d commit b9a88a4

6 files changed

Lines changed: 257 additions & 96 deletions

File tree

.github/workflows/ci.yml

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

382-
rust-test:
383-
name: "Rust tests (sanitizer)"
382+
rust-test-asan:
383+
name: "Rust tests (address sanitizer)"
384384
timeout-minutes: 40
385385
runs-on: >-
386386
${{ github.repository == 'vortex-data/vortex'
387-
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=rust-test-sanitizer', github.run_id)
387+
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=rust-test-address-sanitizer', github.run_id)
388388
|| 'ubuntu-latest' }}
389389
env:
390390
# 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"
391+
ASAN_OPTIONS: "symbolize=1:print_stats=1:check_initialization_order=1:detect_leaks=1:verbosity=1:leak_check_at_exit=1"
392392
LSAN_OPTIONS: "verbosity=1:report_objects=1"
393393
ASAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
394394
# Link against DuckDB debug build
@@ -416,14 +416,124 @@ jobs:
416416
# Build with full debug info first (helps with caching)
417417
cargo +$NIGHTLY_TOOLCHAIN build --locked --all-features \
418418
--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
419+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
420+
420421
# Run tests with sanitizers and debug output
421422
cargo +$NIGHTLY_TOOLCHAIN nextest run \
422423
--locked \
423424
--all-features \
424425
--no-fail-fast \
425426
--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
427+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
428+
429+
# vortex-ffi requires --no-default-features as otherwise we pull in
430+
# Mimalloc which interferes with sanitizers
431+
# cargo nextest reports less leaks than cargo test
432+
cargo +$NIGHTLY_TOOLCHAIN test --locked --no-default-features \
433+
-Zbuild-std --no-fail-fast --target x86_64-unknown-linux-gnu \
434+
-p vortex-ffi -- --no-capture
435+
436+
rust-test-msan:
437+
name: "Rust tests (memory sanitizer)"
438+
timeout-minutes: 40
439+
runs-on: >-
440+
${{ github.repository == 'vortex-data/vortex'
441+
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=rust-test-memory-sanitizer', github.run_id)
442+
|| 'ubuntu-latest' }}
443+
env:
444+
# Add debug symbols and enable ASAN/LSAN with better output
445+
MSAN_OPTIONS: "symbolize=1:print_stats=1:verbosity=1"
446+
MSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
447+
# Link against DuckDB debug build
448+
VX_DUCKDB_DEBUG: "1"
449+
# Keep frame pointers for better stack traces
450+
CARGO_PROFILE_DEV_DEBUG: "true"
451+
CARGO_PROFILE_TEST_DEBUG: "true"
452+
# Skip slow tests that are too expensive under sanitizer
453+
VORTEX_SKIP_SLOW_TESTS: "1"
454+
steps:
455+
- uses: runs-on/action@v2
456+
if: github.repository == 'vortex-data/vortex'
457+
with:
458+
sccache: s3
459+
- uses: actions/checkout@v6
460+
- uses: ./.github/actions/setup-prebuild
461+
- name: Install nightly for sanitizer
462+
run: |
463+
rustup toolchain install $NIGHTLY_TOOLCHAIN
464+
rustup component add --toolchain $NIGHTLY_TOOLCHAIN rust-src rustfmt clippy llvm-tools-preview
465+
- name: Rust Tests
466+
env:
467+
RUSTFLAGS: "-A warnings -Zsanitizer=memory -Cunsafe-allow-abi-mismatch=sanitizer --cfg disable_loom --cfg vortex_nightly -C debuginfo=2 -C opt-level=0 -C strip=none"
468+
run: |
469+
# Build with full debug info first (helps with caching)
470+
cargo +$NIGHTLY_TOOLCHAIN build --locked --all-features \
471+
-Zbuild-std --target x86_64-unknown-linux-gnu \
472+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
473+
474+
# Run tests with sanitizers and debug output
475+
cargo +$NIGHTLY_TOOLCHAIN nextest run \
476+
--locked --all-features --no-fail-fast \
477+
-Zbuild-std --target x86_64-unknown-linux-gnu \
478+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
479+
480+
# vortex-ffi requires --no-default-features as otherwise we pull in
481+
# Mimalloc which interferes with sanitizers
482+
# cargo nextest reports less leaks than cargo test
483+
cargo +$NIGHTLY_TOOLCHAIN test --locked --no-default-features \
484+
-Zbuild-std --no-fail-fast --target x86_64-unknown-linux-gnu \
485+
-p vortex-ffi -- --no-capture
486+
487+
rust-test-tsan:
488+
name: "Rust tests (thread sanitizer)"
489+
timeout-minutes: 40
490+
runs-on: >-
491+
${{ github.repository == 'vortex-data/vortex'
492+
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=rust-test-thread-sanitizer', github.run_id)
493+
|| 'ubuntu-latest' }}
494+
env:
495+
# Add debug symbols and enable ASAN/LSAN with better output
496+
TSAN_OPTIONS: "symbolize=1:verbosity=1"
497+
TSAN_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer"
498+
# Link against DuckDB debug build
499+
VX_DUCKDB_DEBUG: "1"
500+
# Keep frame pointers for better stack traces
501+
CARGO_PROFILE_DEV_DEBUG: "true"
502+
CARGO_PROFILE_TEST_DEBUG: "true"
503+
# Skip slow tests that are too expensive under sanitizer
504+
VORTEX_SKIP_SLOW_TESTS: "1"
505+
steps:
506+
- uses: runs-on/action@v2
507+
if: github.repository == 'vortex-data/vortex'
508+
with:
509+
sccache: s3
510+
- uses: actions/checkout@v6
511+
- uses: ./.github/actions/setup-prebuild
512+
- name: Install nightly for sanitizer
513+
run: |
514+
rustup toolchain install $NIGHTLY_TOOLCHAIN
515+
rustup component add --toolchain $NIGHTLY_TOOLCHAIN rust-src rustfmt clippy llvm-tools-preview
516+
- name: Rust Tests
517+
env:
518+
RUSTFLAGS: "-A warnings -Zsanitizer=thread -Cunsafe-allow-abi-mismatch=sanitizer --cfg disable_loom --cfg vortex_nightly -C debuginfo=2 -C opt-level=0 -C strip=none"
519+
run: |
520+
# Build with full debug info first (helps with caching)
521+
cargo +$NIGHTLY_TOOLCHAIN build --locked --all-features \
522+
-Zbuild-std --target x86_64-unknown-linux-gnu \
523+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
524+
525+
# Run tests with sanitizers and debug output
526+
cargo +$NIGHTLY_TOOLCHAIN nextest run \
527+
-Zbuild-std --locked --all-features \
528+
--no-fail-fast --target x86_64-unknown-linux-gnu \
529+
-p vortex-buffer -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-array
530+
531+
# vortex-ffi requires --no-default-features as otherwise we pull in
532+
# Mimalloc which interferes with sanitizers
533+
# cargo nextest reports less issues than cargo test
534+
cargo +$NIGHTLY_TOOLCHAIN test --locked --no-default-features \
535+
-Zbuild-std --no-fail-fast --target x86_64-unknown-linux-gnu \
536+
-p vortex-ffi -- --no-capture
427537
428538
cuda-build-lint:
429539
if: github.repository == 'vortex-data/vortex'

vortex-ffi/README.md

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ 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

@@ -41,17 +41,61 @@ Stable builds use the checked-in header file at `cinclude/vortex.h`.
4141
- **For regular development**: Stable toolchain builds work with existing checked-in headers
4242
- **CI validation**: Automated checks verify header freshness using nightly toolchain
4343

44-
### Testing
44+
### Testing C part
4545

4646
Build the test library
4747

48-
```
48+
```sh
4949
cmake -Bbuild
5050
cmake --build build -j $(nproc)
5151
```
5252

5353
Run the tests
5454

55-
```
55+
```sh
5656
ctest --test-dir build -j $(nproc)
5757
```
58+
59+
You would need C++ compiler toolchain to run the tests since they use Catch2.
60+
61+
### Testing Rust part with sanitizers
62+
63+
AddressSanitizer:
64+
65+
```sh
66+
# inside vortex-ffi
67+
RUSTFLAGS="-Z sanitizer=address" \
68+
cargo +nightly test -Zbuild-std \
69+
--no-default-features --target <target triple> \
70+
-- --no-capture
71+
```
72+
73+
MemorySanitizer:
74+
75+
```sh
76+
RUSTFLAGS="-Z sanitizer=memory -Cunsafe-allow-abi-mismatch=sanitizer" \
77+
cargo +nightly test -Zbuild-std \
78+
--no-default-features --target <target triple> \
79+
-- --no-capture
80+
```
81+
82+
ThreadSanitizer:
83+
84+
```sh
85+
RUSTFLAGS="-Z sanitizer=thread -Cunsafe-allow-abi-mismatch=sanitizer" \
86+
cargo +nightly test -Zbuild-std \
87+
--no-default-features --target <target triple> \
88+
-- --no-capture
89+
```
90+
91+
- `-Zbuild-std` is needed as memory and thread sanitizers report std errors
92+
otherwise.
93+
- `--no-default-features` is needed as we use Mimalloc otherwise which interferes
94+
with sanitizers.
95+
- `allow-abi-mismatch` is safe because in our dependency graph only crates like
96+
`compiler_builtins` unset sanitization, and they do it on purpose.
97+
- Make sure to use `cargo test` and not `cargo nextest` as nextest reports less
98+
leaks.
99+
- If you want stack trace symbolization, install `llvm-symbolizer`.
100+
101+
### Testing Rust and C part with sanitizers

vortex-ffi/build.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
#![allow(clippy::unwrap_used)]
5-
#![allow(clippy::panic)]
5+
#![allow(clippy::exit)]
66
use std::env;
77
use std::path::PathBuf;
88
use std::process::Command;
9+
use std::process::exit;
910

1011
fn main() {
1112
// Set up dependency tracking
@@ -65,21 +66,17 @@ fn main() {
6566
);
6667
}
6768
}
68-
Err(e) => {
69+
Err(err) => {
6970
// Check if this might be a sanitizer-related incompatibility
70-
let error_msg = e.to_string();
71+
let err = err.to_string();
7172
let rustflags = env::var("RUSTFLAGS").unwrap_or_default();
7273

73-
if rustflags.contains("sanitizer") || error_msg.contains("sanitizer") {
74-
println!(
75-
"cargo:warning=Skipping header generation due to sanitizer incompatibility"
76-
);
77-
println!("cargo:warning=Error: {}", e);
74+
if rustflags.contains("sanitizer") || err.contains("sanitizer") {
75+
println!("cargo:info=Skipping header generation due to sanitizers");
7876
return;
7977
}
80-
81-
// For non-sanitizer errors, fail hard as these indicate real problems
82-
panic!("Failed to generate header with cbindgen: {}", e);
78+
println!("cargo:error=Failed to generate header with cbindgen: {err}");
79+
exit(1);
8380
}
8481
}
8582
}

vortex-ffi/cinclude/vortex.h

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,6 @@ typedef struct vx_array_iterator vx_array_iterator;
266266
* The `sink` interface is used to collect array chunks and place them into a resource
267267
* (e.g. an array stream or file (`vx_array_sink_open_file`)).
268268
*
269-
* ## Thread Safety
270-
*
271269
* This struct is **not** thread-safe for concurrent operations. While the underlying
272270
* `Sender` is thread-safe, the FFI wrapper should only be accessed from a single thread
273271
* to avoid race conditions between `push` and `close` operations. The `close` operation
@@ -742,37 +740,45 @@ void vx_session_free(vx_session *ptr);
742740

743741
/**
744742
* Create a new Vortex session.
745-
*
746-
* The caller is responsible for freeing the session with [`vx_session_free`].
743+
* The session is owned and must be freed by the caller.
747744
*/
748745
vx_session *vx_session_new(void);
749746

750747
/**
751-
* Clone a Vortex session, returning an owned copy.
752-
*
753-
* The caller is responsible for freeing the session with [`vx_session_free`].
748+
* Clone a Vortex session.
749+
* The cloned copy is owned and must be freed by the caller.
754750
*/
755751
vx_session *vx_session_clone(const vx_session *session);
756752

753+
/**
754+
* Free an owned [`vx_array_sink`] object.
755+
*/
756+
void vx_array_sink_free(vx_array_sink *ptr);
757+
757758
/**
758759
* Opens a writable array stream, where sink is used to push values into the stream.
759-
* To close the stream close the sink with `vx_array_sink_close`.
760+
* path must not be NULL.
761+
* This method does not take ownership of dtype.
762+
* If an error is encountered, NULL is returned, and error field is populated
763+
* with an owned error which must be freed by the caller.
764+
* Returned sink is owned and must be freed by the caller
760765
*/
761-
vx_array_sink *vx_array_sink_open_file(const vx_session *session,
762-
const char *path,
763-
const vx_dtype *dtype,
764-
vx_error **error_out);
766+
vx_array_sink *
767+
vx_array_sink_open_file(const vx_session *session, const char *path, const vx_dtype *dtype, vx_error **error);
765768

766769
/**
767-
* Pushed a single array chunk into a file sink.
770+
* Push a single array into a file sink.
771+
* Does not take ownership of array.
768772
*/
769-
void vx_array_sink_push(vx_array_sink *sink, const vx_array *array, vx_error **error_out);
773+
void vx_array_sink_push(vx_array_sink *sink, const vx_array *array, vx_error **error);
770774

771775
/**
772-
* Closes an array sink, must be called to ensure all the values pushed to the sink are written
776+
* Close an array sink.
777+
* Takes ownership of sink and frees it.
778+
* Must be called to ensure all the values pushed to the sink are written
773779
* to the external resource.
774780
*/
775-
void vx_array_sink_close(vx_array_sink *sink, vx_error **error_out);
781+
void vx_array_sink_close(vx_array_sink *sink, vx_error **error);
776782

777783
/**
778784
* Clone a borrowed [`vx_string`], returning an owned [`vx_string`].

vortex-ffi/src/session.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,16 @@ box_wrapper!(
1616
);
1717

1818
/// Create a new Vortex session.
19-
///
20-
/// The caller is responsible for freeing the session with [`vx_session_free`].
19+
/// The session is owned and must be freed by the caller.
2120
#[unsafe(no_mangle)]
2221
pub unsafe extern "C-unwind" fn vx_session_new() -> *mut vx_session {
2322
vx_session::new(Box::new(
2423
VortexSession::default().with_handle(RUNTIME.handle()),
2524
))
2625
}
2726

28-
/// Clone a Vortex session, returning an owned copy.
29-
///
30-
/// The caller is responsible for freeing the session with [`vx_session_free`].
27+
/// Clone a Vortex session.
28+
/// The cloned copy is owned and must be freed by the caller.
3129
#[unsafe(no_mangle)]
3230
pub unsafe extern "C-unwind" fn vx_session_clone(session: *const vx_session) -> *mut vx_session {
3331
let session = vx_session::as_ref(session);

0 commit comments

Comments
 (0)