Retain string values when ownership is shared#2628
Draft
slepp wants to merge 10 commits into
Draft
Conversation
Contributor
Author
|
Blocked on an extern-string ABI mismatch discovered by Linux package-import CI: imported package functions may return plain malloc C strings, while the retain/drop implementation assumes a Hew refcount header. The full Rust workspace and vertical-slice gates pass; the cross-module trait-method fixture aborts when the headerless result reaches hew_string_drop. I stopped rather than expanding this PR into an unplanned extern ABI migration. |
Emit MIR-driven string retains at genuine co-owner mint points while preserving last-use handoffs and consuming runtime paths. Admit matching string drops from the same ownership decision and cover aggregate, return, local-share, parameter, closure, and duplicate-field cases with leak and sanitizer fixtures.
Move string copy, tuple, record, enum, and array sharing fixtures from the reject corpus to the accept corpus now that each co-owner is retained safely.
Copy malloc-owned strings returned by unclassified extern C symbols into Hew's header-aware refcounted domain before freeing the foreign allocation. Share JIT symbol classification with MIR so runtime and stdlib producers retain their native ownership contract.
Add a total `ExternProvenance` (`Root` / `Module(String)`) to `HirExternFn` and populate it at both extern construction sites from the same `current_module_name` authority the lowering context already sets per item. This replaces downstream inference-by-absence: MIR previously derived a C-ABI string return's ownership from whether the extern was missing from `diagnostic_source_modules`, which conflates a root user extern with a std extern whose attribution was lost. A wrong classification corrupts memory in either direction, so the defining module is now a proven fact carried on the node rather than reconstructed later. The enum is deliberately total (no `Unknown`/`Option`) so every construction site must decide at compile time.
Derive `ExternDecl::malloc_string_return` from the extern's carried `ExternProvenance` instead of absence in `diagnostic_source_modules`. The old heuristic corrupted memory in either direction: adopting a header-aware Hew string frees `base + 16`, while treating a foreign C string as Hew reads a phantom 16-byte header. `classify_extern_string_ownership` makes the provenance the primary authority: standard-library provenance is header-aware; root and user/package provenance adopt the raw malloc-owned C return. The JIT symbol catalog is retained only as a monotonic secondary guard that can suppress adoption for a known runtime producer, never enable it and never override std provenance — it is name-keyed and incomplete for std string producers, so it cannot be the ownership authority. Classification is a total, fail-closed three-state result. An unrepresentable provenance yields `ExternStringOwnership::Unresolved`, which pushes a new `ExternStringOwnershipUnresolved` MIR diagnostic (gating codegen at the CLI) rather than guessing a memory-unsafe direction. The diagnostic is rendered by `dump.rs` and all four `hew-cli` diagnostic match arms.
…losed Add structural codegen coverage for the two ownership directions: - `extern_malloc_string_adoption`: a `malloc_string_return = true` extern at a tracked call dest emits the null-guarded adoption sequence (`hew_string_char_count` / `hew_string_slice_codepoints` through the `extern_string_adopt` block) and frees the raw foreign pointer; the same pipeline with `false` stores the header-aware return without adoption. - `scalar_discard_guard`: a `malloc_string_return = true` string extern discarded in statement position (`dest == None`) must STILL fail closed on the pointer/aggregate backstop — the adoption edge only runs for a tracked dest, so a discard can never silently leak the foreign allocation. Both existing `ExternDecl` fixtures gain the now-required `provenance` field.
Add unit and end-to-end lowering coverage for
`classify_extern_string_ownership` and `ExternDecl::malloc_string_return`:
- standard-library provenance stays header-aware even when the symbol is absent
from the JIT catalog (provenance, not the catalog, is authoritative);
- root and user/package provenance adopt the raw foreign C string;
- a user-package symbol whose name collides with an absent std producer still
adopts on its provenance — the exact case the old absence heuristic could not
distinguish;
- a classified runtime symbol is guarded back to header-aware (the monotonic
secondary guard only ever suppresses adoption);
- an unrepresentable `Module("")` provenance fails closed with an
`ExternStringOwnershipUnresolved` diagnostic and a `false` decl, never a
guessed `true`;
- non-C ABI and non-string returns never adopt.
Recapture the `--dump-mir raw`/`elab` golden corpus to accept the String retain/share ownership drift already landed on this branch. The delta is confined to the intended families: added `string.retain` before shared/returned string moves, added `hew_string_drop` drops in previously empty exit-path drop slots, and removed `agg_alias` statements for strings now tracked by retain. Regenerated only after the ownership classification and targeted extern proofs were confirmed correct; `checked-mir-verify` is byte-identical across all 40 fixtures x 2 stages. The extern-provenance change itself does not render in per-function MIR dumps, so it contributes nothing to this delta.
The recv-loop leak oracles all send string literals, which are immortal rodata with no send-side drop obligation — the owned-payload send seam (a fresh f-string per iteration flowing through the Sender::send wrapper param into the borrow-contract call) had no slope coverage. Add an owned_send shape that sends one owned heap string per frame and asserts the same flat leak-node slope as the literal shapes.
slepp
force-pushed
the
feat/a240-s2-string-cow
branch
from
e1ed22c to
78189cd
Compare
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
StringRetainmarker so genuine string co-owner mints retain throughhew_string_clone, and matching drops are admitted from the same ownership decision.hew_string_drop.hew_vec_set_strownership metadata to match its copy-in behaviour.This closes the remaining string ownership gap: uncertain sole ownership now becomes counted sharing instead of leaking or rejecting the value.
Verification
cargo nextest run --workspace --no-fail-fast— green across the workspacestring_retain_share_leak_oracle— 3/3 with realleaks(1)measurements, flat at 0→0 nodesstring_cow_retain_shareand the bytes ownership fixturescripts/checked-mir-corpus.sh verifyclean, 40 fixtures x 2 stages byte-identicalmake test-pkg-import— passes with MallocScribble and MallocGuardEdgesmake test-vertical-slice— passed after moving newly supported string-sharing fixtures into the accept corpusOut of scope