Repository navigation
transform: track escapes through nested aggregate results - #5776
Merged
deadprogram merged 2 commits intoSep 29, 2026
Merged
Conversation
Contributor
Author
|
@deadprogram i'm still validating this, but please keep this on the list of 0.43; this is a possible escape analysis issue. |
davecheney
force-pushed
the
fix-allocs-aggregate-escape
branch
from
September 28, 2026 20:07
ce2058f to
582389a
Compare
davecheney
force-pushed
the
fix-allocs-aggregate-escape
branch
4 times, most recently
from
September 28, 2026 21:11
f9d7666 to
a1a717c
Compare
Escape analysis follows pointers returned by a callee, but stops at extractvalue when the extracted value is an aggregate. This loses pointers inside the aggregate and can place escaping allocations on the stack, leaving callers with pointers to dead stack storage. This was possibly introduced in 27f8ec1. The effect is not limited to multiple return values and single array returns. A slice returned alongside another value, an escape to a package-level variable, a capture by a closure, a value reaching the return through a phi, a non-error interface and multi-level forwarding chains are all affected, at every optimisation level that runs OptimizeAllocs. Follow extracted values whose types contain pointers. Share the recursive type check used by GC root discovery so pointer-free aggregates and zero-length arrays do not cause unnecessary heap allocations. extractvalue was the only unsound arm of the opcode switch: getelementptr, bitcast and insertvalue already recurse, load and icmp are safe, store escapes when the value is the stored operand, ret is gated on allowReturn, and everything else falls through to the conservative default. Binary size is unchanged. The -size=short output is byte-identical for testdata/json.go, testdata/stdlib.go, testdata/map.go, microbit with examples/serial, and pico with examples/machinetest. Expand IR and Go coverage for nested aggregates, forwarding calls, escape destinations and nonescaping uses. Add runtime regressions with volatile stack writes so optimization cannot remove the stack clobber.
Merge the extracted interface with nil before returning it. Load the branch condition through volatile memory and exercise both outcomes so the compiler cannot fold the regression into a direct return. Check that the runtime fixture returns a phi with an extracted aggregate input before escape analysis, and that its allocation remains on the heap.
davecheney
force-pushed
the
fix-allocs-aggregate-escape
branch
2 times, most recently
from
September 29, 2026 08:51
580dee6 to
dea9331
Compare
Contributor
Author
|
@deadprogram @jakebailey @dgryski i'm marking this ready for review. I've thrown every tool I have at this. The bulk of this PR are test fixtures which all fail without the fix. I'm confident they demonstrate an escape analysis failure. I'm also reasonably confident they fix the bug they demonstrate. |
davecheney
marked this pull request as ready for review
September 29, 2026 09:15
deadprogram
approved these changes
Sep 29, 2026
deadprogram
left a comment
Member
There was a problem hiding this comment.
Thank you for working on this. I tested locally and it does what it promises. Now merging!
Member
|
I'm amazed nothing in the test corpus hit this (at least not obviously..) |
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
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.
Escape analysis follows pointers returned by a callee, but stops at
extractvalue when the extracted value is an aggregate. This loses
pointers inside the aggregate and can place escaping allocations on the
stack, leaving callers with pointers to dead stack storage. This was
possibly introduced in 27f8ec1.
The effect is not limited to multiple return values and single array
returns. A slice returned alongside another value, an escape to a
package-level variable, a capture by a closure, a value reaching the
return through a phi, a non-error interface and multi-level forwarding
chains are all affected, at every optimisation level that runs
OptimizeAllocs.
Follow extracted values whose types contain pointers. Share the
recursive type check used by GC root discovery so pointer-free
aggregates and zero-length arrays do not cause unnecessary heap
allocations. extractvalue was the only unsound arm of the opcode
switch: getelementptr, bitcast and insertvalue already recurse, load
and icmp are safe, store escapes when the value is the stored operand,
ret is gated on allowReturn, and everything else falls through to the
conservative default.
Binary size is unchanged. The -size=short output is byte-identical for
testdata/json.go, testdata/stdlib.go, testdata/map.go, microbit with
examples/serial, and pico with examples/machinetest.
Expand IR and Go coverage for nested aggregates, forwarding calls,
escape destinations and nonescaping uses. Add runtime regressions with
volatile stack writes so optimization cannot remove the stack clobber.