From 63575834e69b878106da645c8494a3fc7c09bc98 Mon Sep 17 00:00:00 2001 From: Reuben Brooks Date: Sat, 13 Jun 2026 14:15:41 -0500 Subject: [PATCH 1/2] Fix #2: pr honours *hush* only for the standard output stream The canonical kernel `pr` (kernel/klambda/writer.kl) gates the whole write on `*hush*`, so under `-q` / `(set *hush* true)` even a write to an explicitly-opened file stream was silently dropped, producing zero-byte files. `*hush*` is only meant to silence interactive console chatter. Register a native `pr` override in `register_hot_overrides` (installed after the AOT defun, in both boot paths) that consults `*hush*` ONLY when the target stream is the standard output stream (identified by Rc pointer-equality against the `*stoutput*` global); writes to any other (file) stream always occur. This matches shen-cl / shen-go. Tests: flipped the port-authored cli_launcher assertion that locked in the zero-byte-with-`-q` behavior to assert the file IS written under `-q` (renamed to quiet_does_not_silence_pr_to_file, referencing #2); updated the file-header divergence note. Gates: cargo fmt --check, clippy -D warnings, build, cargo test --workspace (190 passed), and the canonical kernel suite (134/134) all green. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/shen-rust/src/primitives.rs | 63 ++++++++++++++++++++++++++ crates/shen-rust/tests/cli_launcher.rs | 38 ++++++++-------- 2 files changed, 83 insertions(+), 18 deletions(-) diff --git a/crates/shen-rust/src/primitives.rs b/crates/shen-rust/src/primitives.rs index 12e523c..7462ce4 100644 --- a/crates/shen-rust/src/primitives.rs +++ b/crates/shen-rust/src/primitives.rs @@ -90,6 +90,69 @@ pub fn register_hot_overrides(interp: &mut Interp) { interp.register_native("read-file-as-string", 1, hot_read_file_as_string); interp.register_aot_direct("read-file-as-string", hot_read_file_as_string); + + // pr — print a string to a stream. The canonical kernel `pr` gates the + // *whole* write on `*hush*` (see kernel/klambda/writer.kl), so under + // `-q` / `(set *hush* true)` even a write to an explicitly-opened file + // stream is silently dropped, producing zero-byte files (issue #2). + // That gate is only meant to silence interactive console chatter, so we + // consult `*hush*` ONLY when the target is the standard output stream; + // writes to any other (file) stream always occur. See #2. + interp.register_native("pr", 2, hot_pr); + interp.register_aot_direct("pr", hot_pr); +} + +/// pr — write the string `args[0]` to the output stream `args[1]`, honouring +/// `*hush*` only for the standard output stream (issue #2). Returns the +/// original string, matching the kernel contract. +fn hot_pr(interp: &mut Interp, args: &[Value]) -> ShenResult { + let s = match args[0].as_str() { + Some(s) => s.to_string(), + None => { + return Err(ShenError::new(format!( + "pr: arg 1 not a string: {:?}", + args[0] + ))) + } + }; + let target = match args[1].as_stream() { + Some(t) => t, + None => { + return Err(ShenError::new(format!( + "pr: arg 2 not a stream: {:?}", + args[1] + ))) + } + }; + + // Suppress the write only when `*hush*` is set AND the target is the + // standard output stream. File (and other) streams always get written. + let hush_sym = interp.intern("*hush*"); + let hush = matches!( + interp.env.get_global(hush_sym).and_then(|v| v.as_bool()), + Some(true) + ); + if hush { + let stout_sym = interp.intern("*stoutput*"); + let is_stdout = interp + .env + .get_global(stout_sym) + .and_then(|v| v.as_stream()) + .is_some_and(|out| Rc::ptr_eq(&out, &target)); + if is_stdout { + return Ok(args[0]); + } + } + + let mut stream = target.borrow_mut(); + match &mut *stream { + Stream::Out(w) => { + w.write_all(s.as_bytes()) + .map_err(|e| ShenError::new(format!("pr: {e}")))?; + } + _ => return Err(ShenError::new("pr: not an output stream")), + } + Ok(args[0]) } // --- hot-override bodies (DirectFn-compatible plain fns) --- diff --git a/crates/shen-rust/tests/cli_launcher.rs b/crates/shen-rust/tests/cli_launcher.rs index d2bb7cb..4b5a0e3 100644 --- a/crates/shen-rust/tests/cli_launcher.rs +++ b/crates/shen-rust/tests/cli_launcher.rs @@ -3,15 +3,17 @@ //! Mirror of shen-go's `cmd/shen/main_test.go`: spawn the real release //! binary and exercise the launcher protocol — `eval -e EXPR`, `eval -l //! FILE`, `script FILE`, `--version` / `--help`, piping EOF, the `-q` -//! file-write divergence, and adversarial input. +//! file-write behavior, and adversarial input. +//! +//! * `-q` (which sets `*hush*`): `*hush*` only silences chatter on the +//! standard output stream. A `pr` to an explicitly-opened file stream is +//! always written, with or without `-q` — matching shen-cl/shen-go. We +//! assert the file IS written in BOTH cases (issue #2 fixed a prior bug +//! where `-q` dropped the write, producing zero-byte files). //! //! DIVERGENCES from shen-go, locked in with comments (the spec requires the //! port's CORRECT documented behavior, not faked parity): //! -//! * `-q` (which sets `*hush*`): on shen-rust `-q` SILENCES `pr` writes to -//! file streams (zero-byte files), whereas shen-cl/shen-go route `pr` to -//! files regardless. We assert the file IS written WITHOUT `-q`, and is -//! EMPTY (or the process still succeeds) WITH `-q`. //! * Adversarial `eval -e`: shen-rust prints the error to stderr and the //! launcher exits SUCCESS (it does not map an eval error to a nonzero //! code the way shen-go does). We assert the error message is reported @@ -235,12 +237,13 @@ fn piped_stdin_eof_exits_cleanly() { ); } -/// The `-q` / `*hush*` divergence, locked in as shen-rust's CORRECT -/// documented behavior. WITHOUT `-q`, `pr` to a file stream writes the -/// payload. WITH `-q`, `*hush*` silences `pr` to file streams (zero-byte -/// file) — this is the cross-impl divergence the spec calls out. +/// Regression for issue #2: `-q` (`*hush*`) must NOT silence `pr` writes to +/// file streams. `*hush*` only suppresses chatter on the standard output +/// stream; an explicitly-opened file stream is always written, with or +/// without `-q` — matching shen-cl / shen-go. (Previously shen-rust dropped +/// the write under `-q`, producing zero-byte files.) #[test] -fn quiet_silences_pr_to_file_but_default_writes() { +fn quiet_does_not_silence_pr_to_file() { let dir = std::env::temp_dir().join(format!("shen-rust-cli-hush-{}", std::process::id())); std::fs::create_dir_all(&dir).ok(); @@ -260,8 +263,8 @@ fn quiet_silences_pr_to_file_but_default_writes() { "without -q, pr must write the payload to the file" ); - // --- WITH -q: *hush* silences the pr write (documented shen-rust - // divergence). The process still succeeds; the file is empty. --- + // --- WITH -q: the file is STILL written (issue #2 regression). `*hush*` + // only silences the standard output stream, never a file stream. --- let path_b = dir.join("out_q.txt"); let expr_b = format!( r#"(let S (open "{}" out) (do (pr "payload" S) (close S)))"#, @@ -270,12 +273,11 @@ fn quiet_silences_pr_to_file_but_default_writes() { let o = run(&["eval", "-q", "-e", &expr_b], None, TIMEOUT); assert_no_backtrace(&o, "eval -q"); assert!(o.success, "eval -q failed:\n{}", o.combined); - let data_b = std::fs::read(&path_b).unwrap_or_default(); - assert!( - data_b.is_empty(), - "shen-rust -q (*hush*) must silence pr to files; got {} bytes: {:?}", - data_b.len(), - String::from_utf8_lossy(&data_b) + let data_b = std::fs::read(&path_b).expect("output file should exist with -q (issue #2)"); + assert_eq!( + String::from_utf8_lossy(&data_b), + "payload", + "with -q, *hush* must NOT silence pr to a file stream (issue #2)" ); std::fs::remove_file(&path_a).ok(); From 4435b28e7592c29092384d2d368ebc0970942aca Mon Sep 17 00:00:00 2001 From: Reuben Brooks Date: Sun, 14 Jun 2026 14:45:53 -0500 Subject: [PATCH 2/2] Fix #4: cap absvector size so huge requests raise a catchable error (absvector 100000000000) allocated unbounded and OOM-aborted the process (SIGKILL), uncatchable by trap-error. This also broke the debug `cargo test --workspace` gate (Gate 3): reader_fuzz::corpus_never_panics drives that exact input and was OOM-killed in unoptimized builds (release was fast enough to mask it, which is why it passed under --release). Cap absvector at 2^24 slots (~800x the largest vector the kernel allocates) and raise a catchable Shen error above it, mirroring shen-go and shen-cl (#3). Added regression assertions in primitives_coverage.rs (huge -> err, trap-able; boundary 2^24 and small sizes still ok). Full `cargo test --workspace` green in debug; fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/shen-rust/src/primitives.rs | 10 +++++++++- crates/shen-rust/tests/primitives_coverage.rs | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/shen-rust/src/primitives.rs b/crates/shen-rust/src/primitives.rs index 7462ce4..50f1126 100644 --- a/crates/shen-rust/src/primitives.rs +++ b/crates/shen-rust/src/primitives.rs @@ -439,8 +439,13 @@ fn register_core(interp: &mut Interp) { }); // --- vectors (absvectors) --- + // Cap oversized requests so an absurd size raises a CATCHABLE Shen error + // instead of OOM-aborting the whole process (e.g. `(absvector 100000000000)` + // from the reader-fuzz corpus). 2^24 (~16.7M slots) is ~800x the largest + // vector the kernel itself allocates; mirrors shen-go and shen-cl (#3). + const MAX_ABSVECTOR: i64 = 1 << 24; interp.register_native("absvector", 1, |_, args| match args[0].as_int() { - Some(n) if n >= 0 => { + Some(n) if (0..=MAX_ABSVECTOR).contains(&n) => { let len = n as usize; let cells = vec![Value::sym(crate::symbol::SymId(0)); len]; // Will be overwritten with an "uninitialized" sentinel in @@ -449,6 +454,9 @@ fn register_core(interp: &mut Interp) { // WellKnown ordering — harmless for the kernel). Ok(Value::absvector(cells)) } + Some(n) if n > MAX_ABSVECTOR => Err(ShenError::new(format!( + "absvector size {n} out of range (0..{MAX_ABSVECTOR})" + ))), _ => Err(ShenError::new(format!("absvector: bad arg: {:?}", args[0]))), }); interp.register_native("<-address", 2, |_, args| { diff --git a/crates/shen-rust/tests/primitives_coverage.rs b/crates/shen-rust/tests/primitives_coverage.rs index 5fa2fba..3329fd4 100644 --- a/crates/shen-rust/tests/primitives_coverage.rs +++ b/crates/shen-rust/tests/primitives_coverage.rs @@ -248,6 +248,16 @@ fn absvector_out_of_range_errors() { // Reading past the end is a catchable error, not a panic. assert!(run(&mut i, "(<-address (absvector 2) 5)").is_err()); assert!(run(&mut i, "(address-> (absvector 2) 5 1)").is_err()); + // An absurd size is a catchable error (capped), not an OOM abort. + assert!(run(&mut i, "(absvector 100000000000)").is_err()); + assert!(run( + &mut i, + "(trap-error (absvector 100000000000) (lambda E true))" + ) + .is_ok()); + // The cap boundary and legitimate sizes still succeed. + assert!(run(&mut i, "(absvector 16777216)").is_ok()); + assert!(run(&mut i, "(absvector 8)").is_ok()); } // ---------------------------------------------------------------------------