fix(alloc): keep free_inline's asm goto out of MSVC cleanup funclets (2.2.5)

crates/rusty_alloc/CHANGELOG.md

@@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **2.2.1–2.2.4 failed to build in an MSVC consumer that unwinds, with LTO.**
+  `free_inline`'s fast path is an `asm!` goto (LLVM `callbr`), and since
+  2.2.1 `GlobalAlloc::dealloc` inlines it into every Rust deallocation,
+  including the drops a panic runs. On MSVC targets those are EH cleanup
+  funclets, which cannot hold a `callbr`: LLVM rejected the module ("Bogus
+  funclet pad use ... Broken module found"). Reported by rusty_sloth (thin
+  LTO, one codegen unit, `panic = "unwind"`). On `target_env = "msvc"` with
+  `panic = "unwind"`, `free_inline` now uses the plain-Rust decrement that
+  aarch64 and Miri already use. The fix sits in `free_inline` itself, so it
+  covers every caller, `rusty_alloc::alloc::free` and
+  `rusty_alloc_api::Heap::dealloc` included, not just `dealloc`. MSVC builds
+  with `panic = "abort"` and every other target are unchanged.
+  Cost, on affected builds only: +3 instructions per local free (callgrind,
+  the same code forced on Linux: small 52.26 -> 55.26 Ir/op); Rust
+  `GlobalAlloc` workloads +0.30 % to +3.56 % whole-program, output identical.
+
 ## [2.2.4](https://github.com/Remade-With-Rust/rusty_alloc/compare/rusty_alloc-v2.2.3...rusty_alloc-v2.2.4) - 2026-10-08
 
 ### Semver note (read before upgrading)

crates/rusty_alloc/src/alloc.rs

@@ -1135,7 +1135,25 @@ pub unsafe fn free_inline(p: *mut u8) {
                 {
                     (*owner_heap(pg)).stats.frees += 1;
                 }
-                #[cfg(all(target_arch = "x86_64", not(miri)))]
+                // NOT on an MSVC target that unwinds. This function inlines
+                // into `GlobalAlloc::dealloc`, so into every Rust drop,
+                // including the drops a panic runs; on MSVC those are EH
+                // cleanup FUNCLETS, and LLVM cannot place an `asm!` goto
+                // (`callbr`) in a funclet: its successors lose the funclet
+                // colouring and the verifier rejects the module ("Bogus
+                // funclet pad use"). 2.2.1-2.2.4 failed to build in an
+                // unwinding MSVC consumer with LTO that way (rusty_sloth;
+                // docs/plans/msvc-funclet-asm-goto.md). Those builds take the
+                // plain-Rust decrement below (+3 Ir per local free, opps #6),
+                // which reaches every caller, not just `dealloc`. An MSVC
+                // build with `panic = "abort"` has no funclets and keeps the
+                // asm. Regression gate: the corpus's rusty_sloth row, built
+                // `--release` (no small consumer reproduces it).
+                #[cfg(all(
+                    target_arch = "x86_64",
+                    not(miri),
+                    not(all(target_env = "msvc", panic = "unwind"))
+                ))]
                 {
                     // SAFETY: `pg` is a live page of this thread; `USED_OFFSET`
                     // is `offset_of!(Page, used)`, checked against the real
@@ -1180,7 +1198,11 @@ pub unsafe fn free_inline(p: *mut u8) {
                         options(nostack),
                     );
                 }
-                #[cfg(not(all(target_arch = "x86_64", not(miri))))]
+                #[cfg(not(all(
+                    target_arch = "x86_64",
+                    not(miri),
+                    not(all(target_env = "msvc", panic = "unwind"))
+                )))]
                 {
                     let u = (*pg).used.wrapping_sub(1);
                     (*pg).used = u;

crates/rusty_alloc_api/CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **2.2.1–2.2.4 failed to build in an MSVC consumer that unwinds, with LTO**
+  ("Bogus funclet pad use"): `GlobalAlloc::dealloc` inlined the core's
+  `asm!` goto into EH cleanup funclets. Fixed in `rusty_alloc` (see its
+  changelog); this crate's code is unchanged.
+
 ## [2.2.1](https://github.com/Remade-With-Rust/rusty_alloc/compare/rusty_alloc-api-v2.2.0...rusty_alloc-api-v2.2.1) - 2026-09-25
 
 ### Performance

docs/plans/msvc-funclet-asm-goto.md

@@ -1,6 +1,6 @@
 # 2.2.1–2.2.4 do not build in an unwinding MSVC consumer with LTO
 
-**Status:** fix written and verified in a consumer; **not merged, not released**. To do: §6.
+**Status:** fixed differently from §3, on branch `fix/msvc-funclet-asm-goto`, for 2.2.5. See §8.
 **Date:** 2026-10-07 · **Affects:** rusty_alloc-api 2.2.1, 2.2.2, 2.2.3, 2.2.4 ·
 **Platform:** `x86_64-pc-windows-msvc`, consumer built with `panic = "unwind"` (the default),
 `lto = "thin"`, `codegen-units = 1` ·
@@ -30,6 +30,39 @@ error: could not compile `rusty_sloth-cli` (bin "sloth")
 The consumer's release profile is `opt-level = 3`, `lto = "thin"`, `codegen-units = 1`, and the
 default `panic = "unwind"`. 2.2.0 builds in the same profile.
 
+### Reproduce (re-run 2026-10-07 from a clean rusty_sloth `main`, no patch)
+
+The consumer's `Cargo.lock` pins **2.2.0**. Cargo keeps a locked version until it is told to
+move. Without step 2 the build compiles 2.2.0 from crates.io and tests nothing; a `[patch]`
+entry is likewise left unused (cargo only warns "Patch … was not used in the crate graph").
+
+```sh
+cd rusty_sloth                                    # F:/coding/rusty_sloth
+cargo add rusty_alloc-api@2.2.4 -p rusty_sloth-alloc
+cargo update -p rusty_alloc-api -p rusty_alloc    # REQUIRED: moves the lock off 2.2.0
+cargo tree -i rusty_alloc-api -e normal           # must print "rusty_alloc-api v2.2.4"
+cargo build --release -p rusty_sloth-cli --features cuda
+```
+
+**Expected:** `Compiling rusty_alloc v2.2.4`, then four "Bogus funclet pad use" and `rustc-LLVM
+ERROR: Broken module found`. Reproduced with the published 2.2.4 (`source =
+"registry+…crates.io-index"` in the lock). The LTO link of the `sloth` binary is where it fails;
+building only the library crates passes.
+
+**To test a rusty_alloc tree instead** (the fix, or `main`), put the two entries in rusty_sloth's
+**existing** `[patch.crates-io]` table. It already holds `serde_json` and `tokenizers`, and a
+second `[patch.crates-io]` header is a duplicate-key error:
+
+```toml
+rusty_alloc = { path = "<tree>/crates/rusty_alloc" }
+rusty_alloc-api = { path = "<tree>/crates/rusty_alloc_api" }
+```
+
+Then the same `cargo add` (the requirement must admit the tree's version), `cargo update -p
+rusty_alloc-api -p rusty_alloc`, and check that the lock's two entries have **no `source =`
+line** (a path dependency) before building. Restore rusty_sloth afterwards (`git checkout
+Cargo.toml Cargo.lock crates/rusty_sloth-alloc/Cargo.toml`).
+
 ## 2. Mechanism
 
 - `free_inline` (`crates/rusty_alloc/src/alloc.rs`, `#[inline(always)]`) has the free fast path
@@ -165,3 +198,59 @@ workspace (`tests/msvc-unwind-consumer/`, with `path` dependencies) built with
 - Would an LLVM / rustc issue be worth filing? A `callbr` inlined into a funclet should either be
   handled or refused at inline time, not left for the verifier. A minimal reproducer is an
   `asm!` goto in an `#[inline(always)]` function called from a `Drop` on MSVC with LTO.
+
+## 8. Resolution (2026-10-07, rusty_alloc session)
+
+**The fix is in `free_inline`, not in `dealloc`.** `7037d8d` outlined only
+`GlobalAlloc::dealloc`, so the other callers were still exposed:
+`rusty_alloc_api::Heap::dealloc` calls `rusty_alloc::alloc::free`, whose body
+is `free_inline`, and LTO is free to inline it into a consumer's drop. Instead,
+the asm goto's cfg in `free_inline` now excludes
+`all(target_env = "msvc", panic = "unwind")`. Those builds take the
+plain-Rust decrement that aarch64 and Miri already use. MSVC builds with
+`panic = "abort"`, which have no funclets, keep the asm. `7037d8d` is not
+merged; its branch is left in place.
+
+**Cost** (§6 step 3). Callgrind cannot run an MSVC build, so the same code was
+forced on Linux and counted there: +3 Ir per local free, exactly opps #6's
+figure. Opscan small 52.26 -> 55.26, big 94 -> 97, mixed 94.26 -> 97.26. On the
+Rust `GlobalAlloc` workloads it cost maps +0.30 %, threads +0.40 %,
+overaligned +0.62 % and trees +3.56 % (free-heavy), with identical checksums.
+Outlining would have cost about +2 (call and return) and covered one caller.
+
+**Regression gate** (§5). The defect depends on LLVM's inlining choices,
+and those depend on more than the code.
+
+- **No small consumer reproduced it.** Four shapes, thin and fat LTO, all
+  built clean on the unfixed code: a `Vec` owned across a panicking call, a
+  `Heap::dealloc` in a `Drop`, a direct `GlobalAlloc::dealloc` in a `Drop`, an
+  unwind-only guard with an `#[inline(always)]` drop. The IR shows why: rustc
+  marks calls in cleanup blocks cold, so the inliner leaves the drop out of
+  line and the `callbr` stays on the normal path.
+- **rusty_sloth repointed to a PATH did not reproduce it either.** Unfixed
+  2.2.4 as a path dependency built green, with and without `--features cuda`.
+  The same source from crates.io failed. The source is byte-identical
+  (`diff -r` of the published crate against the tree). What differs is the
+  package id, which goes into every symbol hash and so into LLVM's inlining
+  order.
+- **What does reproduce it: crates.io identity, source swapped.**
+  `cargo vendor` plus source replacement keeps rusty_alloc a crates.io
+  package, so only its `src/` changes. rusty_sloth `--release --features cuda`:
+
+  | allocator source | result |
+  |---|---|
+  | 2.2.4 as published | **fails**, 4 × "Bogus funclet pad use" |
+  | 2.2.4 + this fix | **builds**; `sloth --help` runs |
+
+  A vendored package is immutable to Cargo: swapping its source does NOT
+  rebuild it. `cargo clean -p rusty_alloc -p rusty_alloc-api` first, or the
+  "fixed" build reuses the broken rlib (it did once here: 4 errors, nothing
+  recompiled).
+- **Deterministic check.** In `rusty_alloc`'s own IR for
+  `x86_64-pc-windows-msvc`, `callbr` count: unfixed 3 (unwind) / 3 (abort);
+  fixed **0** (unwind) / 3 (abort). With no `callbr` there is nothing for any
+  inlining order to misplace.
+
+The gate is the corpus row for rusty_sloth: `build = "release"` and
+`identity = "registry"` (`run.sh`'s `registry_swap`). It runs locally only,
+because CI has neither rusty_sloth nor CUDA.

tools/corpus/README.md

@@ -17,6 +17,23 @@ bash tools/corpus/run.sh --test   # + each consumer's test suite
 Consumers are registered in `corpus.toml`. Add one the moment it takes a
 dependency on this crate.
 
+A consumer with `build = "release"` is BUILT in its own release profile
+instead, in both modes. `check` never runs codegen and `test` never uses LTO,
+and some defects exist only in optimised codegen. 2.2.1–2.2.4 failed LLVM's
+verifier in rusty_sloth, an unwinding MSVC consumer with thin LTO
+(`docs/plans/msvc-funclet-asm-goto.md`). That row is slow: an LTO build of an
+ML stack, twice. A candidate failing LLVM's verifier ("Broken module found")
+is blamed on us even though the error names no crate, because the baseline
+built and only the allocator differs between the two builds.
+
+`identity = "registry"` keeps the candidate's rusty_alloc a crates.io
+package, vendored with only its `src/` swapped for this tree's
+(`registry_swap`), instead of rewriting it to a path. Use it when a defect
+depends on codegen: a path dependency has a different package id, so
+different symbol hashes and different LLVM inlining. The funclet defect
+fails rusty_sloth from crates.io and builds clean as a path dependency of the
+very same source.
+
 ## What it does
 
 Two arms per consumer. **BASELINE** is the consumer as it sits on disk, with its

tools/corpus/corpus.toml

@@ -1,61 +1,85 @@
-# The downstream corpus: crates that ship rusty_alloc to production.
-#
-# A major version is a promise that consumers can keep compiling. 2.0.0 makes
-# two breaking changes -- `default-features = false` now selects `no_std`, and
-# `heap::Heap` gained a field -- and neither is visible from inside this repo.
-# The only way to know what they cost is to build the things that depend on us.
-#
-# `path` is absolute because these are sibling checkouts, not submodules; a
-# consumer whose path is missing is SKIPPED and reported as such, never silently
-# passed. Add a consumer here the moment it takes a dependency on this crate.
-#
-# `features`/`no_default` describe the configuration that actually SHIPS. There
-# is no value in testing a shape nobody deploys.
-
-[[consumer]]
-name = "spacedb-sdk"
-path = "F:/coding/mata-master/packages/spacedb"
-pkg = "spacedb-sdk"
-synth = "yes"
-# `rusty-alloc` is default-ON here on purpose: a SpaceDB replica often runs on a
-# machine its operator does not control, so the unconfigured node is the
-# hardened one. That makes this the shape in production.
-features = ""
-note = "rusty_alloc-api pinned =1.1.6, optional, default-on"
-
-[[consumer]]
-name = "spacedb-sdk (secure)"
-path = "F:/coding/mata-master/packages/spacedb"
-pkg = "spacedb-sdk"
-synth = "yes"
-features = "secure"
-note = "forwards rusty_alloc-api/secure -- guard pages + encrypted free lists"
-
-[[consumer]]
-name = "rusty_alloc_default"
-path = "F:/coding/rusty_alloc_default"
-features = ""
-note = "the #[global_allocator] shim every Remade UI crate installs; rusty_zstd reaches us THROUGH it"
-
-[[consumer]]
-name = "rusty_zstd"
-path = "F:/coding/rusty_zstd"
-features = "rusty-alloc"
-pkg = "rusty_zstd"
-note = "depends via rusty_alloc_default, not directly"
-
-[[consumer]]
-name = "rusty_maplibre"
-path = "F:/coding/rusty_maplibre"
-features = ""
-note = "KNOWN RISK: takes rusty_alloc AND rusty_alloc-api with default-features = false, which is exactly what 2.0.0 redefines"
-
-[[consumer]]
-name = "spacedb (published mirror)"
-path = "F:/coding/spacedb"
-pkg = "spacedb-sdk"
-features = ""
-# The standalone checkout `release/release.sh` generates from mata-master's
-# packages — the tree crates.io receives. Registered separately because it is
-# what ships, and it carries its own workspace root (no synth needed).
-note = "generated mirror of packages/spacedb; rusty_alloc-api =1.1.6, default-on"
+# The downstream corpus: crates that ship rusty_alloc to production.
+#
+# A major version is a promise that consumers can keep compiling. 2.0.0 makes
+# two breaking changes -- `default-features = false` now selects `no_std`, and
+# `heap::Heap` gained a field -- and neither is visible from inside this repo.
+# The only way to know what they cost is to build the things that depend on us.
+#
+# `path` is absolute because these are sibling checkouts, not submodules; a
+# consumer whose path is missing is SKIPPED and reported as such, never silently
+# passed. Add a consumer here the moment it takes a dependency on this crate.
+#
+# `features`/`no_default` describe the configuration that actually SHIPS. There
+# is no value in testing a shape nobody deploys.
+
+[[consumer]]
+name = "spacedb-sdk"
+path = "F:/coding/mata-master/packages/spacedb"
+pkg = "spacedb-sdk"
+synth = "yes"
+# `rusty-alloc` is default-ON here on purpose: a SpaceDB replica often runs on a
+# machine its operator does not control, so the unconfigured node is the
+# hardened one. That makes this the shape in production.
+features = ""
+note = "rusty_alloc-api pinned =1.1.6, optional, default-on"
+
+[[consumer]]
+name = "spacedb-sdk (secure)"
+path = "F:/coding/mata-master/packages/spacedb"
+pkg = "spacedb-sdk"
+synth = "yes"
+features = "secure"
+note = "forwards rusty_alloc-api/secure -- guard pages + encrypted free lists"
+
+[[consumer]]
+name = "rusty_alloc_default"
+path = "F:/coding/rusty_alloc_default"
+features = ""
+note = "the #[global_allocator] shim every Remade UI crate installs; rusty_zstd reaches us THROUGH it"
+
+[[consumer]]
+name = "rusty_zstd"
+path = "F:/coding/rusty_zstd"
+features = "rusty-alloc"
+pkg = "rusty_zstd"
+note = "depends via rusty_alloc_default, not directly"
+
+[[consumer]]
+name = "rusty_maplibre"
+path = "F:/coding/rusty_maplibre"
+features = ""
+note = "KNOWN RISK: takes rusty_alloc AND rusty_alloc-api with default-features = false, which is exactly what 2.0.0 redefines"
+
+[[consumer]]
+name = "spacedb (published mirror)"
+path = "F:/coding/spacedb"
+pkg = "spacedb-sdk"
+features = ""
+# The standalone checkout `release/release.sh` generates from mata-master's
+# packages — the tree crates.io receives. Registered separately because it is
+# what ships, and it carries its own workspace root (no synth needed).
+note = "generated mirror of packages/spacedb; rusty_alloc-api =1.1.6, default-on"
+
+[[consumer]]
+name = "rusty_sloth"
+path = "F:/coding/rusty_sloth"
+pkg = "rusty_sloth-cli"
+# `cuda` is the shipped shape, and the one the defect was reproduced in. Needs
+# the CUDA toolkit; a machine without it reports BASELINE ALREADY RED, not a
+# pass.
+features = "cuda"
+build = "release"
+# AND with rusty_alloc kept a crates.io package (vendored, source swapped).
+# Repointed to a path, unfixed 2.2.4 built GREEN here, with and without cuda,
+# while the same source from crates.io failed: the package id changes the
+# symbol hashes, and with them LLVM's inlining (checked 2026-10-07).
+identity = "registry"
+# The shape no other consumer has: it UNWINDS (default panic strategy) and
+# ships thin LTO with one codegen unit, so the allocator's inlined free lands
+# in EH cleanup funclets on MSVC. 2.2.1-2.2.4 failed LLVM's verifier there
+# ("Bogus funclet pad use"); docs/plans/msvc-funclet-asm-goto.md. Also the
+# consumer that exercised 2.2.4's stranded-span fix: it quantises on worker
+# threads that exit and frees on main. Built with `cargo build --release`,
+# not `check`: the defect exists only in optimised codegen. Slow (an LTO build
+# of an ML stack, twice), and wants ~10 GB free in TMPDIR.
+note = "via crates/rusty_sloth-alloc; --features cuda; unwinding + thin LTO + cgu=1; release BUILD"