From be162957bac4a3300af796b7d9e4be69c14b6804 Mon Sep 17 00:00:00 2001 From: Kacy Fortner Date: Thu, 30 Jul 2026 04:46:52 +0000 Subject: [PATCH 1/2] allocate runtime strings from their known length instead of probing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pith_strdup_string built a rust string and handed a pointer into its buffer to pith_strdup, whose length lookup probes 16 bytes in front of the pointer for a pith header. a rust allocation has no header, so whenever the buffer sat at the start of its heap block the probe read memory the process does not own — valgrind flagged it in docsite and sitegen on every run (pith_list_dir was the caller), and the magic compare failing is all that made it benign. process.rs carried its own copy of the same helper. the helper already knows the length, so it now allocates the headered cstring directly and copies — no probe, no rust round-trip, one fewer format allocation. the ptr-16 probe on string literals in mapped binary data remains the documented practical-guard design; what changed is that no production path hands a rust heap buffer to it any more. valgrind on docsite and sitegen with the struct freelist disabled: the invalid read is gone, zero errors, goldens unchanged. --- cranelift/runtime/src/process.rs | 5 +++-- cranelift/runtime/src/runtime_core.rs | 12 ++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cranelift/runtime/src/process.rs b/cranelift/runtime/src/process.rs index b9d90b07..06ea73bf 100644 --- a/cranelift/runtime/src/process.rs +++ b/cranelift/runtime/src/process.rs @@ -146,8 +146,9 @@ fn pith_store_process_handle(mut child: Child) -> i64 { } fn pith_strdup_string(text: &str) -> *mut i8 { - let owned = format!("{}\0", text); - unsafe { crate::pith_strdup(owned.as_ptr() as *const i8) } + // the shared helper allocates from the known length without probing the + // rust buffer for a header it cannot have (see runtime_core). + crate::runtime_core::pith_strdup_string(text) } unsafe fn pith_build_command( diff --git a/cranelift/runtime/src/runtime_core.rs b/cranelift/runtime/src/runtime_core.rs index 2121a2bd..415c66c9 100644 --- a/cranelift/runtime/src/runtime_core.rs +++ b/cranelift/runtime/src/runtime_core.rs @@ -4,8 +4,16 @@ use crate::string; use std::alloc::{alloc, Layout}; pub(crate) fn pith_strdup_string(text: &str) -> *mut i8 { - let owned = format!("{}\0", text); - unsafe { pith_strdup(owned.as_ptr() as *const i8) } + // the length is already known, so allocate the headered cstring directly. + // routing through pith_strdup would probe 16 bytes in front of the rust + // string's buffer looking for a header no rust allocation has — a read of + // memory this process does not own whenever the buffer starts its heap + // block. pith_alloc_cstring writes the nul terminator itself. + unsafe { + let result = pith_alloc_cstring(text.len()); + std::ptr::copy_nonoverlapping(text.as_ptr() as *const i8, result, text.len()); + result + } } #[no_mangle] From c5c4242982f8e4b6941eaae502b6a7b5d808c975 Mon Sep 17 00:00:00 2001 From: Kacy Fortner Date: Thu, 30 Jul 2026 05:08:32 +0000 Subject: [PATCH 2/2] retry transient dials in the concurrent resumption case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the case counts successful dials and prints an exact total, but a dial had one attempt — a starved accept loop or a refused connect on a loaded runner broke the count for reasons unrelated to the session-store lock it tests. it flaked twice in ci in three days that way. each dial now retries up to five times with a short growing backoff. a retried dial exercises the stores exactly as well, and the count stays exact. verified five for five with both cores pinned by busy loops, where the single-attempt version was flaking. --- .../cases/test_tls_concurrent_resumption.pith | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/cases/test_tls_concurrent_resumption.pith b/tests/cases/test_tls_concurrent_resumption.pith index bbf5e749..ba50ed14 100644 --- a/tests/cases/test_tls_concurrent_resumption.pith +++ b/tests/cases/test_tls_concurrent_resumption.pith @@ -14,6 +14,7 @@ # resumed. a torn store shows up as a failed dial, a wrong echo, or a hang. import std.net.tls as tls +import std.time as time CERT_PATH := "tests/live/fixtures/localhost.crt" KEY_PATH := "tests/live/fixtures/localhost.key" @@ -60,11 +61,26 @@ fn dial_once(cfg: tls.Config, tag: String) -> Bool: return false return got_r.ok == "echo:" + tag +# a dial that fails transiently — a starved accept loop on a loaded box, a +# refused connect while the backlog drains — is retried a few times with a +# short backoff. the property under test is the session stores staying +# coherent under concurrent handshakes, and a retried dial exercises them +# exactly as well; without the retry, one transient socket failure on a busy +# ci runner breaks the exact-count output for reasons unrelated to the lock. +fn dial_with_retry(cfg: tls.Config, tag: String) -> Bool: + mut attempt := 0 + while attempt < 5: + if dial_once(cfg, tag): + return true + attempt = attempt + 1 + time.delay(20 * attempt) + return false + fn worker(id: Int, cfg: tls.Config, done: Channel[Int]): mut ok := 0 mut i := 0 while i < DIALS_PER_WORKER: - if dial_once(cfg, "w" + id.to_string() + "-" + i.to_string()): + if dial_with_retry(cfg, "w" + id.to_string() + "-" + i.to_string()): ok = ok + 1 i = i + 1 done.send(ok)