Skip to content

Commit b7c30d1

Browse files
authored
Add "GC Zeal" infrastructure for additional, aggressive GC assertions (#12921)
* Add "GC Zeal" infrastructure for additional, aggressive GC assertions This initial commit just sets up the initial GC Zeal infrastructure, it doesn't actually start using it anywhere yet. * Adds support for `cfg(gc_zeal)` to `crates/wasmtime/build.rs` and `crates/cranelift/build.rs` * Defines `gc_assert!` macro * Defines `wasmtime_environ::gc::POISON` constant for GC heap poisoning * Adds debug assert that `POISON` doesn't overlap any valid `VMGcKind` discriminant * Adds CI job to run GC-related tests with `cfg(gc_zeal)` * Review feedback
1 parent a46300a commit b7c30d1

4 files changed

Lines changed: 84 additions & 18 deletions

File tree

.github/workflows/main.yml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ jobs:
231231
if grep -q pulley names.log; then
232232
echo test-nightly=true >> $GITHUB_OUTPUT
233233
fi
234+
if grep -q gc names.log; then
235+
echo test-gc-zeal=true >> $GITHUB_OUTPUT
236+
fi
234237
fi
235238
matrix="$(node ./ci/build-test-matrix.js ./commits.log ./names.log $run_full)"
236239
echo "test-matrix={\"include\":$(echo $matrix)}" >> $GITHUB_OUTPUT
@@ -244,6 +247,7 @@ jobs:
244247
echo test-capi=true >> $GITHUB_OUTPUT
245248
echo test-nightly=true >> $GITHUB_OUTPUT
246249
echo test-miri=true >> $GITHUB_OUTPUT
250+
echo test-gc-zeal=true >> $GITHUB_OUTPUT
247251
echo audit=true >> $GITHUB_OUTPUT
248252
echo preview1-adapter=true >> $GITHUB_OUTPUT
249253
echo run-dwarf=true >> $GITHUB_OUTPUT
@@ -663,6 +667,29 @@ jobs:
663667
- run: cargo fuzz check --dev --fuzz-dir ./cranelift/assembler-x64/fuzz
664668
- run: cargo fuzz check --dev --fuzz-dir ./crates/wizer/fuzz
665669

670+
# Run GC-related tests with `cfg(gc_zeal)` set
671+
gc_zeal:
672+
needs: determine
673+
if: needs.determine.outputs.test-gc-zeal
674+
name: GC Zeal Tests
675+
runs-on: ubuntu-latest
676+
steps:
677+
- uses: actions/checkout@v6
678+
with:
679+
submodules: true
680+
- uses: ./.github/actions/install-rust
681+
- name: Run GC tests with cfg(gc_zeal) enabled
682+
env:
683+
WASMTIME_TEST_GC_KEYWORDS: "gc drc any eq struct array i31 extern exn ref rec"
684+
RUSTFLAGS: "--cfg gc_zeal"
685+
run: |
686+
set -e
687+
688+
cargo test --test all -- $WASMTIME_TEST_GC_KEYWORDS
689+
690+
# The wast test runner will read `WASMTIME_TEST_GC_KEYWORDS` on its own.
691+
cargo test --test wast
692+
666693
# Perform all tests of the c-api
667694
test_capi:
668695
needs: determine
@@ -1336,6 +1363,7 @@ jobs:
13361363
- build-preview1-component-adapter-provider
13371364
- test-min-platform-example
13381365
- check_js
1366+
- gc_zeal
13391367
if: always()
13401368
steps:
13411369
# Calculate the exit status of the whole CI workflow.

Cargo.toml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,17 @@ unused_import_braces = 'warn'
205205
unused-lifetimes = 'warn'
206206
unused-macro-rules = 'warn'
207207

208-
# Don't warn about unknown cfgs for pulley
208+
# Don't warn about unknown cfgs for our custom cfgs.
209209
[workspace.lints.rust.unexpected_cfgs]
210210
level = "warn"
211211
check-cfg = [
212212
'cfg(pulley_tail_calls)',
213213
'cfg(pulley_assume_llvm_makes_tail_calls)',
214214
'cfg(pulley_disable_interp_simd)',
215+
'cfg(arc_try_new)',
216+
217+
# When enabled, `cfg(gc_zeal)` activates aggressive GC debugging assertions.
218+
'cfg(gc_zeal)',
215219
]
216220

217221
[workspace.lints.clippy]

crates/environ/src/gc.rs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,20 @@ use crate::{
2222
use alloc::sync::Arc;
2323
use core::alloc::Layout;
2424

25+
/// Poison byte written over unallocated GC heap memory when `cfg(gc_zeal)` is
26+
/// enabled.
27+
pub const POISON: u8 = 0b00001111;
28+
29+
/// Assert a condition, but only when `gc_zeal` is enabled.
30+
#[macro_export]
31+
macro_rules! gc_assert {
32+
($($arg:tt)*) => {
33+
if cfg!(gc_zeal) {
34+
assert!($($arg)*);
35+
}
36+
};
37+
}
38+
2539
/// Discriminant to check whether GC reference is an `i31ref` or not.
2640
pub const I31_DISCRIMINANT: u32 = 1;
2741

@@ -485,15 +499,23 @@ impl VMGcKind {
485499
#[inline]
486500
pub fn from_high_bits_of_u32(val: u32) -> VMGcKind {
487501
let masked = val & Self::MASK;
488-
match masked {
502+
let result = match masked {
489503
x if x == Self::ExternRef.as_u32() => Self::ExternRef,
490504
x if x == Self::AnyRef.as_u32() => Self::AnyRef,
491505
x if x == Self::EqRef.as_u32() => Self::EqRef,
492506
x if x == Self::ArrayRef.as_u32() => Self::ArrayRef,
493507
x if x == Self::StructRef.as_u32() => Self::StructRef,
494508
x if x == Self::ExnRef.as_u32() => Self::ExnRef,
495509
_ => panic!("invalid `VMGcKind`: {masked:#032b}"),
496-
}
510+
};
511+
512+
let poison_kind = u32::from_le_bytes([POISON, POISON, POISON, POISON]) & VMGcKind::MASK;
513+
debug_assert_ne!(
514+
masked, poison_kind,
515+
"No valid `VMGcKind` should overlap with the poison pattern"
516+
);
517+
518+
result
497519
}
498520

499521
/// Does this kind match the other kind?

tests/wast.rs

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,37 @@ fn main() {
1818

1919
let mut trials = Vec::new();
2020

21+
// Check for if we are only running GC-related tests.
22+
let gc_keywords = std::env::var("WASMTIME_TEST_GC_KEYWORDS")
23+
.ok()
24+
.map(|s| s.split(" ").map(|s| s.to_string()).collect::<Vec<_>>());
25+
2126
let mut add_trial = |test: &WastTest, config: WastConfig| {
22-
let trial = Trial::test(
23-
format!(
24-
"{:?}/{}{}{}",
25-
config.compiler,
26-
if config.pooling { "pooling/" } else { "" },
27-
if config.collector != Collector::Auto {
28-
format!("{:?}/", config.collector)
29-
} else {
30-
String::new()
31-
},
32-
test.path.to_str().unwrap()
33-
),
34-
{
35-
let test = test.clone();
36-
move || run_wast(&test, config).map_err(|e| format!("{e:?}").into())
27+
let name = format!(
28+
"{:?}/{}{}{}",
29+
config.compiler,
30+
if config.pooling { "pooling/" } else { "" },
31+
if config.collector != Collector::Auto {
32+
format!("{:?}/", config.collector)
33+
} else {
34+
String::new()
3735
},
36+
test.path.to_str().unwrap()
3837
);
3938

39+
// Don't add this trial if we are only running GC-related tests and it
40+
// doesn't look like a GC-related test.
41+
if let Some(ks) = &gc_keywords {
42+
if config.collector == Collector::Auto && !ks.iter().any(|kw| name.contains(kw)) {
43+
return;
44+
}
45+
}
46+
47+
let trial = Trial::test(name, {
48+
let test = test.clone();
49+
move || run_wast(&test, config).map_err(|e| format!("{e:?}").into())
50+
});
51+
4052
trials.push(trial);
4153
};
4254

0 commit comments

Comments
 (0)