allocate runtime strings from their known length instead of probing - #623
Merged
Conversation
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summary
pith_strdup_stringbuilt a rustStringand passed a pointer into itsbuffer to
pith_strdup, whose length lookup probes 16 bytes in front of thepointer for a pith cstring header. rust allocations carry no such header, so
whenever the buffer began its heap block the probe read before the
allocation — the invalid read valgrind reported in docsite and sitegen on
every run, symbolized to
pith_strdup_string ← pith_list_dir ← std_fs_list.the failing magic compare is all that made it harmless.
the helper knows the length already, so it allocates the headered cstring
directly and copies.
process.rshad a private copy of the same helper andnow delegates. the deliberate ptr-16 probe on binary-mapped string literals
is unchanged; the invariant restored is that no production path hands a rust
heap buffer to the prober.
what was tested
valgrind with
PITH_STRUCT_FREELIST=0on both docsite and sitegen: zeroerrors where each previously reported the invalid read; golden checks
unchanged.
cargo test --release -p pith-runtime129 pass. full battery:make teston both backends, both corpora,make memcheck,make leak-check— all green.