From b643f9dee8a0808f596d80b0dce59863f96e8dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 19:23:39 +0200 Subject: [PATCH 1/2] fix(gc): PERRY_GC_STACKMAP_TRACE must parse its value, not its presence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lint` is red on main, which blocks every open PR: it is a required check and `check_gc_env_knobs` fails on PERRY_GC_STACKMAP_TRACE: read for presence (var_os(..).is_some()) in crates/perry-runtime/src/gc/roots/stack_maps.rs; 'PERRY_GC_STACKMAP_TRACE=0' would ENABLE it. The knob arrived in #8131 reading `var_os(..).is_some()`. Presence-testing inverts the one spelling a reader is most likely to try: `=0` sets the variable, so it turns the trace ON. Every other GC knob routes through the shared parser for exactly this reason, and the audit exists to keep that uniform. Use `gc::env_flag_enabled`, the default-OFF parser (`policy.rs`'s `PERRY_GC_TRACE` is the same shape). It fails toward the knob's documented default, so a typo leaves the instrument off rather than silently arming it. Behaviour is unchanged for the spellings that already worked — `=1`/`on`/`true` enable it, unset leaves it off — and `=0`/`off`/`false`/`no` now disable it instead of enabling it. `python3 scripts/check_gc_env_knobs.py` goes from one failure to "30 claimed knobs, 197 live env parsers, 0 presence-only GC reads". --- crates/perry-runtime/src/gc/roots/stack_maps.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/perry-runtime/src/gc/roots/stack_maps.rs b/crates/perry-runtime/src/gc/roots/stack_maps.rs index 8d7c0b75f8..cb417138ae 100644 --- a/crates/perry-runtime/src/gc/roots/stack_maps.rs +++ b/crates/perry-runtime/src/gc/roots/stack_maps.rs @@ -1114,7 +1114,11 @@ mod unwind { fn walk_trace_enabled() -> bool { static ON: std::sync::OnceLock = std::sync::OnceLock::new(); - *ON.get_or_init(|| std::env::var_os("PERRY_GC_STACKMAP_TRACE").is_some()) + // `env_flag_enabled`, not `var_os(..).is_some()`: presence-testing makes + // `PERRY_GC_STACKMAP_TRACE=0` ENABLE the trace, which is the opposite of + // what every other GC knob does and what anyone typing `=0` means. The + // shared parser fails toward the knob's documented default (OFF here). + *ON.get_or_init(|| crate::gc::env_flag_enabled("PERRY_GC_STACKMAP_TRACE")) } unsafe extern "C" fn walk_frame( From fc4c4a65ffe2d242ca9b31819acb44bed6501c4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 15 Aug 2026 19:23:51 +0200 Subject: [PATCH 2/2] chore(changelog): add fragment for #8164 --- changelog.d/8164-stackmap-trace-knob-polarity.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 changelog.d/8164-stackmap-trace-knob-polarity.md diff --git a/changelog.d/8164-stackmap-trace-knob-polarity.md b/changelog.d/8164-stackmap-trace-knob-polarity.md new file mode 100644 index 0000000000..8d9eef5850 --- /dev/null +++ b/changelog.d/8164-stackmap-trace-knob-polarity.md @@ -0,0 +1,12 @@ +`PERRY_GC_STACKMAP_TRACE` parses its value instead of its presence, restoring +the `lint` gate. The knob arrived in #8131 reading `var_os(..).is_some()`, which +inverts the spelling a reader is most likely to try: `PERRY_GC_STACKMAP_TRACE=0` +still sets the variable, so it turned the trace ON. `check_gc_env_knobs` — a +required-check audit that exists to keep every GC knob on the shared parser — +failed on it, and a red `lint` on main blocks every open PR. + +It now uses `gc::env_flag_enabled`, the default-OFF parser (the same shape as +`policy.rs`'s `PERRY_GC_TRACE`), which fails toward the knob's documented +default so a typo leaves the instrument off rather than silently arming it. +`=1`/`on`/`true` still enable it and unset still leaves it off; `=0`/`off`/ +`false`/`no` now disable it.