From: Keith Randall Date: Wed, 8 May 2024 15:51:39 +0000 (-0700) Subject: cmd/compile: avoid past-the-end pointer when zeroing X-Git-Tag: go1.23rc1~430 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=93e3696b5dac778cf638a67616a4a4d521d6fce9;p=gostls13.git cmd/compile: avoid past-the-end pointer when zeroing When we optimize append(s, make([]T, n)...), we have to be careful not to pass &s[0] + len(s)*sizeof(T) as the argument to memclr, as that pointer might be past-the-end. This can only happen if n is zero, so just special-case n==0 in the generated code. Fixes #67255 Change-Id: Ic680711bb8c38440eba5e759363ef65f5945658b Reviewed-on: https://go-review.googlesource.com/c/go/+/584116 Reviewed-by: Austin Clements LUCI-TryBot-Result: Go LUCI Reviewed-by: Cuong Manh Le Reviewed-by: Keith Randall --- diff --git a/src/cmd/compile/internal/walk/assign.go b/src/cmd/compile/internal/walk/assign.go index fc3b858a80..63b6a1d2c1 100644 --- a/src/cmd/compile/internal/walk/assign.go +++ b/src/cmd/compile/internal/walk/assign.go @@ -623,21 +623,23 @@ func isAppendOfMake(n ir.Node) bool { // panicmakeslicelen() // } // s := l1 -// n := len(s) + l2 -// // Compare n and s as uint so growslice can panic on overflow of len(s) + l2. -// // cap is a positive int and n can become negative when len(s) + l2 -// // overflows int. Interpreting n when negative as uint makes it larger -// // than cap(s). growslice will check the int n arg and panic if n is -// // negative. This prevents the overflow from being undetected. -// if uint(n) <= uint(cap(s)) { -// s = s[:n] -// } else { -// s = growslice(T, s.ptr, n, s.cap, l2, T) +// if l2 != 0 { +// n := len(s) + l2 +// // Compare n and s as uint so growslice can panic on overflow of len(s) + l2. +// // cap is a positive int and n can become negative when len(s) + l2 +// // overflows int. Interpreting n when negative as uint makes it larger +// // than cap(s). growslice will check the int n arg and panic if n is +// // negative. This prevents the overflow from being undetected. +// if uint(n) <= uint(cap(s)) { +// s = s[:n] +// } else { +// s = growslice(T, s.ptr, n, s.cap, l2, T) +// } +// // clear the new portion of the underlying array. +// hp := &s[len(s)-l2] +// hn := l2 * sizeof(T) +// memclr(hp, hn) // } -// // clear the new portion of the underlying array. -// hp := &s[len(s)-l2] -// hn := l2 * sizeof(T) -// memclr(hp, hn) // } // s // @@ -671,11 +673,18 @@ func extendSlice(n *ir.CallExpr, init *ir.Nodes) ir.Node { s := typecheck.TempAt(base.Pos, ir.CurFunc, l1.Type()) nodes = append(nodes, ir.NewAssignStmt(base.Pos, s, l1)) + // if l2 != 0 { + // Avoid work if we're not appending anything. But more importantly, + // avoid allowing hp to be a past-the-end pointer when clearing. See issue 67255. + nifnz := ir.NewIfStmt(base.Pos, ir.NewBinaryExpr(base.Pos, ir.ONE, l2, ir.NewInt(base.Pos, 0)), nil, nil) + nifnz.Likely = true + nodes = append(nodes, nifnz) + elemtype := s.Type().Elem() // n := s.len + l2 nn := typecheck.TempAt(base.Pos, ir.CurFunc, types.Types[types.TINT]) - nodes = append(nodes, ir.NewAssignStmt(base.Pos, nn, ir.NewBinaryExpr(base.Pos, ir.OADD, ir.NewUnaryExpr(base.Pos, ir.OLEN, s), l2))) + nifnz.Body = append(nifnz.Body, ir.NewAssignStmt(base.Pos, nn, ir.NewBinaryExpr(base.Pos, ir.OADD, ir.NewUnaryExpr(base.Pos, ir.OLEN, s), l2))) // if uint(n) <= uint(s.cap) nuint := typecheck.Conv(nn, types.Types[types.TUINT]) @@ -697,7 +706,7 @@ func extendSlice(n *ir.CallExpr, init *ir.Nodes) ir.Node { l2)), } - nodes = append(nodes, nif) + nifnz.Body = append(nifnz.Body, nif) // hp := &s[s.len - l2] // TODO: &s[s.len] - hn? @@ -723,7 +732,7 @@ func extendSlice(n *ir.CallExpr, init *ir.Nodes) ir.Node { // if growslice isn't called do we need to do the zeroing ourselves. nif.Body = append(nif.Body, clr...) } else { - nodes = append(nodes, clr...) + nifnz.Body = append(nifnz.Body, clr...) } typecheck.Stmts(nodes) diff --git a/test/fixedbugs/issue67255.go b/test/fixedbugs/issue67255.go new file mode 100644 index 0000000000..7ca7a239dd --- /dev/null +++ b/test/fixedbugs/issue67255.go @@ -0,0 +1,33 @@ +// run + +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +var zero int + +var sink any + +func main() { + var objs [][]*byte + for i := 10; i < 200; i++ { + // The objects we're allocating here are pointer-ful. Some will + // max out their size class, which are the ones we want. + // We also allocate from small to large, so that the object which + // maxes out its size class is the last one allocated in that class. + // This allocation pattern leaves the next object in the class + // unallocated, which we need to reproduce the bug. + objs = append(objs, make([]*byte, i)) + } + sink = objs // force heap allocation + + // Bug will happen as soon as the write barrier turns on. + for range 10000 { + sink = make([]*byte, 1024) + for _, s := range objs { + s = append(s, make([]*byte, zero)...) + } + } +}