Skip to content

Commit 53e046f

Browse files
author
Ralph Küpper
committed
docs(changelog): fold the #7984 fix into the changeset
1 parent b33110a commit 53e046f

1 file changed

Lines changed: 86 additions & 33 deletions

File tree

Lines changed: 86 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,90 @@
1+
### aarch64: the fast GC stack-map walker misread every SVE-shaped prologue (#7984)
2+
3+
`PERRY_STACKMAP_WALKER=verify` caught the frame-pointer-chain walker and the
4+
Itanium unwinder resolving the same GC root 96 bytes apart on
5+
`ubuntu-24.04-arm`. The fp-chain walker is the wrong one — it is also the one
6+
that runs when `verify` is off — and it was blind twice in the same prologue.
7+
8+
Reproduced end to end on aarch64 Linux. The trigger is the tuning, not the
9+
distro: Perry builds a host binary with `-mcpu=native`, and on a Neoverse-class
10+
core that turns SVE on, which changes the shape LLVM emits. The same probe built
11+
`-mcpu=neoverse-n1` passes; no Apple arm can see it at all. Measured on `main`
12+
— the module body, 100 stack-map records — built `-mcpu=neoverse-n2`:
13+
14+
```
15+
12479c: add x29, sp, #0x20 <- fp established; the decoder read 32
16+
1247a0: stp x28, x27, [sp, #48] <- not a `sub sp`, so the run ended HERE
17+
... four more callee-save pairs
18+
1247b4: sub sp, sp, #0x50 <- 80 bytes, dropped
19+
1247b8: addvl sp, sp, #-2 <- and two vector lengths more, dropped
20+
```
21+
22+
1. **A callee-save store ended the accumulation run.** It does not move sp, so
23+
it says nothing about whether the prologue's stack adjustments are finished,
24+
but the rule was "the first instruction that is not a `sub sp` ends the run".
25+
Stores through sp with no writeback are now transparent to it, enumerated by
26+
opcode so an unrecognised instruction still ends the run — the safe
27+
direction.
28+
2. **`addvl`/`addpl` writing sp was not decoded at all.** Its multiplier is in
29+
the instruction; its unit is the runtime SVE vector length, which is not.
30+
That is now read once via `prctl(PR_SVE_GET_VL)` — a syscall rather than
31+
`rdvl`, which faults on a core without SVE — and cached. Where it cannot be
32+
read the whole decode fails and the frame goes to the platform unwinder,
33+
which reads DWARF CFI and needs no VG for an fp-based frame.
34+
35+
**96 was never a constant.** It is that frame's missed tail and it scales with
36+
the vector length: 96 on the runner (`0x50 + 1 x 16`), 208 under `qemu -cpu max`
37+
at VL = 64 (`0x50 + 2 x 64`). A fix keyed on 96 would have been wrong on every
38+
other vector length.
39+
40+
Validated at two vector lengths so the scaling is pinned rather than one
41+
machine's answer: 28 `verify` runs (14 probes x VL 16 B and 64 B) under
42+
`qemu-aarch64 -cpu max`, all byte-exact against the pinned Node oracle. The fast
43+
walker is live there — `11_collect_at_depth` reports `fp_walks 12`,
44+
`fallback_walks 0`, `records_matched 1338`, `locations_visited 2678` with
45+
`verify` green, so 2678 slots were cross-checked against the unwinder and
46+
agreed.
47+
148
### `PERRY_STACKMAP_WALKER=verify` now names the disagreement it finds
249

3-
The first end-to-end `verify` run on aarch64 ELF caught the fp-chain walker and
4-
the Itanium unwinder resolving the same GC root 96 bytes apart (#7984). The
5-
whole of what the gate could report was `fast walk visited 1 unique slots,
6-
unwinder visited 1` and the two addresses in decimal — not the frame, not the
7-
base register, not the function whose prologue was decoded, and therefore not
8-
*which walker is wrong*. Every candidate explanation predicts exactly that
9-
output: a `sub sp` the prologue decoder's contiguous-run rule missed, a frame
10-
the x29 chain skipped because an intermediate frame carries no frame record
11-
(legal on Linux, not on Darwin), or a CFA one frame out on libgcc.
50+
All the gate could report was `fast walk visited 1 unique slots, unwinder
51+
visited 1` and two addresses in decimal — not the frame, not the base register,
52+
not the function whose prologue was decoded, and therefore not which walker was
53+
wrong. Every candidate explanation predicts exactly that output.
1254

1355
Both walkers now hand back a `ResolvedRoot` rather than a bare
14-
`MutableRootSlot`: the same address, plus the frame return address it was
15-
matched on, the record's function, the map's base register and frame offset,
16-
and the base that walker resolved that register to.
17-
`visit_stack_map_root_slots` projects it straight back to a `MutableRootSlot`,
18-
so the collector's view is unchanged. On a mismatch `verify` prints every root
19-
from both walks, states that an equal slot count means a *base* disagreement
20-
rather than a missed frame (with the per-slot byte delta), and on aarch64 dumps
21-
`fp_to_sp_offset`'s decode together with the prologue words it read — the
22-
ground truth for the frame layout the fast walker derives an SP base from.
23-
24-
The prologue dump is gated on the parsed map vouching for the function address
25-
(`function_starts`, the same set `match_records` consults). The first draft was
26-
not gated, and a unit test with a synthetic address turned the diagnostic into
27-
a SIGSEGV with no output — which is what would happen in the field for the one
28-
failure mode where a report matters most, a map whose addresses are wrong.
29-
`an_unvouched_function_address_is_never_dereferenced` pins it.
30-
31-
`gc-native-roots.yml`'s crash path tailed 20 lines of the failing run's stderr,
32-
which truncates the report's head; it now tails 120. `verify` and the decoder
33-
tests move into `stack_maps_verify.rs` and `stack_maps_decode_tests.rs` because
34-
`stack_maps.rs` was eight lines under the 2000-line cap.
35-
36-
This does not fix #7984 — the `ubuntu-24.04-arm` arm stays red. It makes that
37-
arm's next red run diagnostic instead of a riddle.
56+
`MutableRootSlot`: the address, plus the frame return address it was matched on,
57+
the record's function, the map's base register and frame offset, and the base
58+
that walker resolved that register to. `visit_stack_map_root_slots` projects it
59+
straight back, so the collector's view is unchanged. On a mismatch `verify`
60+
prints every root from both walks, states that an equal slot count means a
61+
*base* disagreement rather than a missed frame (with the per-slot delta), prints
62+
each walk's frame and record counts so an early-terminating walk is
63+
distinguishable, and on aarch64 dumps `fp_to_sp_offset`'s decode together with
64+
the prologue words it read. That report is what identified #7984 in one run.
65+
66+
The prologue dump is gated on the parsed map vouching for the function address.
67+
The first draft was not, and a unit test with a synthetic address turned the
68+
diagnostic into a SIGSEGV with no output — which is what would happen in the
69+
field for the one failure mode where a report matters most, a map whose
70+
addresses are wrong.
71+
72+
### The two aarch64 walkers now have unit coverage at all
73+
74+
Nothing `cargo test` runs had ever called `fp_chain::visit` or `unwind::visit`;
75+
the decoder and the matcher were covered, the step that turns a
76+
`(register, offset)` pair into a stack address was not.
77+
`stack_maps_walker_agreement.rs` drives both over `global_asm!` probe frames in
78+
the two real layouts — the aarch64-ELF one with the frame record in the middle
79+
of the frame, and the Mach-O one with it at the top — and requires each walker
80+
to land on a word **holding a sentinel** the probe wrote into the slot its
81+
record names. Set equality is satisfied by two empty sets and by two identically
82+
wrong ones; the slot's contents are the discriminating quantity.
83+
`a_wrong_frame_offset_is_caught` is the sabotage arm, and a third test pins that
84+
an undecodable prologue declines the fast walk while the unwinder still
85+
resolves the root — the fallback the SVE fix rests on.
86+
87+
`gc-native-roots.yml` runs them on both aarch64 arms, before the probe matrix,
88+
requiring each test by name (`--lib <filter>` is a substring match, so a rename
89+
would select nothing and cargo would still exit 0). Its crash path tailed 20
90+
lines of the failing run's stderr, which truncates the report; now 120.

0 commit comments

Comments
 (0)