Repository navigation
compiler: scan typed stack fields on explicit GC root targets - #3022
Conversation
There was a problem hiding this comment.
Review summary
This PR adds a whole-program frontend escape-analysis pass (internal/goescape/) plus SSA stack-allocation lowering and typed-stack-field GC roots so collectors scan pointer fields of explicitly-rooted stack objects. The design is careful and well-documented, and it is backed by an extensive test suite (escape precision, stack-root, transient, keepalive, J32/J64/W32 execution).
I ran code-quality, performance, security/memory-safety, and documentation-accuracy passes over the full diff.
Memory safety / GC soundness: I traced the stack-promotion decisions, the field-address root init ordering (buildGCRoots + initializeStackRoots publish the frame head only after zero-init and SetGCRoot stores complete; managed allocas are hoisted so field addresses dominate safepoints), the RootValue width decode, the read-only string/byte borrow revocation, and the budget arithmetic. I did not find a substantiated dangling-pointer, missed-root, overread, or overflow. The outlineGCRoots path is correctly disabled whenever stack objects are present, and both budget subtractions are guarded against unsigned underflow.
The findings below are a documentation mismatch plus a few non-blocking quality/performance notes. None are blocking.
- One doc row contradicts the implemented map behavior.
- Two compile-time cost notes (redundant recomputation) with easy memoization fixes.
- One misleading comment.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
ea93037 to
f38064d
Compare
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
f38064d to
8b11bfa
Compare
LLGo baseline benchmarks
String equality benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
Use main's bootstrap safepoint handling and repeat fresh worker processes to cover GC during initialization without changing worker startup order.
8b11bfa to
7d600ca
Compare
7d600ca to
a26aeeb
Compare
Explicit-root targets currently retain pointer-bearing temporaries on the heap because rooting their address does not scan fields changed by a borrowing callee:
Register physical stack-field addresses and read their current pointers during collection. One finalized frame preserves entry allocas, initialization, replay/head guards and shared stack budgets. Enable eligible locals, interface boxes, closure/slice/map storage, synchronous receiver words and invocation-owned defer records. KeepAlive now uses the general mechanism. Loop/asynchronous/oversized storage retains heap ownership.
Depends on #3021; #2997 is merged. Incremental diff: cpunion/llgo@7eb338c...feat/escape-typed-stack-roots-3012
Validation: full compiler/SSA/escape/C ABI/compile-command suites; LLVM layout/optimization checks for J32/J64/W32; Go and native LLGo execution; actual J32, J64 and W32 execution, including two-worker J32, callee mutation, panic/defer and Goexit. Physical pointer-width tests cover Wasm32 C fields without overreading a Go word. Panic payloads use scanned explicit roots across longjmp; the macOS coverage regression passes six repetitions.
Part 2 of #3012 (sections 4, 7–8).