From 8477562ce54868958f0f84f30815a220128aacf7 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 11 Nov 2022 15:30:10 -0800 Subject: [PATCH] cmd/compile: be more careful about pointer incrementing in range loops For range loops, we use a pointer to the backing store that gets incremented on each iteration of the loop. The problem with this scheme is that at the end of the last iteration, we may briefly have a pointer that points past the end of the backing store of the slice that is being iterated over. We cannot let the garbage collector see that pointer. To fix this problem, have the incremented pointer live briefly as a uintptr instead of a normal pointer, so it doesn't keep anything alive. Convert back to a normal pointer just after the loop condition is checked, but before anything that requires a real pointer representation (in practice, any call, which is what could cause a GC scan or stack copy). Fixes #56699 Change-Id: Ia928d23f85a211565357603668bea4e5c534f989 Reviewed-on: https://go-review.googlesource.com/c/go/+/449995 Reviewed-by: David Chase TryBot-Result: Gopher Robot Reviewed-by: Keith Randall Run-TryBot: Keith Randall Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/walk/range.go | 78 ++++++++++++++++++++++---- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/src/cmd/compile/internal/walk/range.go b/src/cmd/compile/internal/walk/range.go index f2591c362af..64af26bf29c 100644 --- a/src/cmd/compile/internal/walk/range.go +++ b/src/cmd/compile/internal/walk/range.go @@ -142,20 +142,78 @@ func walkRange(nrange *ir.RangeStmt) ir.Node { hs.SetTypecheck(1) } - // Pointer to current iteration position - hp := typecheck.Temp(types.NewPtr(elem)) - init = append(init, ir.NewAssignStmt(base.Pos, hp, ir.NewUnaryExpr(base.Pos, ir.OSPTR, hs))) + // We use a "pointer" to keep track of where we are in the backing array + // of the slice hs. This pointer starts at hs.ptr and gets incremented + // by the element size each time through the loop. + // + // It's tricky, though, as on the last iteration this pointer gets + // incremented to point past the end of the backing array. We can't + // let the garbage collector see that final out-of-bounds pointer. + // + // To avoid this, we keep the "pointer" alternately in 2 variables, one + // pointer typed and one uintptr typed. Most of the time it lives in the + // regular pointer variable, but when it might be out of bounds (after it + // has been incremented, but before the loop condition has been checked) + // it lives briefly in the uintptr variable. + // + // hp contains the pointer version (of type *T, where T is the element type). + // It is guaranteed to always be in range, keeps the backing store alive, + // and is updated on stack copies. If a GC occurs when this function is + // suspended at any safepoint, this variable ensures correct operation. + // + // hu contains the equivalent uintptr version. It may point past the + // end, but doesn't keep the backing store alive and doesn't get updated + // on a stack copy. If a GC occurs while this function is on the top of + // the stack, then the last frame is scanned conservatively and hu will + // act as a reference to the backing array to ensure it is not collected. + // + // The "pointer" we're moving across the backing array lives in one + // or the other of hp and hu as the loop proceeds. + // + // hp is live during most of the body of the loop. But it isn't live + // at the very top of the loop, when we haven't checked i