Skip to content

Commit 4bb0847

Browse files
committed
cmd/compile,runtime: change unsafe.Slice((*T)(nil), 0) to return []T(nil)
This CL removes the unconditional OCHECKNIL check added in walkUnsafeSlice by instead passing it as a pointer to runtime.unsafeslice, and hiding the check behind a `len == 0` check. While here, this CL also implements checkptr functionality for unsafe.Slice and disallows use of unsafe.Slice with //go:notinheap types. Updates #46742. Change-Id: I743a445ac124304a4d7322a7fe089c4a21b9a655 Reviewed-on: https://go-review.googlesource.com/c/go/+/331070 Run-TryBot: Matthew Dempsky <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Matthew Dempsky <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 1519271 commit 4bb0847

File tree

9 files changed

+83
-27
lines changed

9 files changed

+83
-27
lines changed

src/cmd/compile/internal/typecheck/builtin.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmd/compile/internal/typecheck/builtin/runtime.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,9 @@ func makeslice(typ *byte, len int, cap int) unsafe.Pointer
183183
func makeslice64(typ *byte, len int64, cap int64) unsafe.Pointer
184184
func makeslicecopy(typ *byte, tolen int, fromlen int, from unsafe.Pointer) unsafe.Pointer
185185
func growslice(typ *byte, old []any, cap int) (ary []any)
186-
func unsafeslice(typ *byte, len int)
187-
func unsafeslice64(typ *byte, len int64)
186+
func unsafeslice(typ *byte, ptr unsafe.Pointer, len int)
187+
func unsafeslice64(typ *byte, ptr unsafe.Pointer, len int64)
188+
func unsafeslicecheckptr(typ *byte, ptr unsafe.Pointer, len int64)
188189

189190
func memmove(to *any, frm *any, length uintptr)
190191
func memclrNoHeapPointers(ptr unsafe.Pointer, n uintptr)

src/cmd/compile/internal/typecheck/func.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,7 +1018,14 @@ func tcUnsafeSlice(n *ir.BinaryExpr) *ir.BinaryExpr {
10181018
t := n.X.Type()
10191019
if !t.IsPtr() {
10201020
base.Errorf("first argument to unsafe.Slice must be pointer; have %L", t)
1021+
} else if t.Elem().NotInHeap() {
1022+
// TODO(mdempsky): This can be relaxed, but should only affect the
1023+
// Go runtime itself. End users should only see //go:notinheap
1024+
// types due to incomplete C structs in cgo, and those types don't
1025+
// have a meaningful size anyway.
1026+
base.Errorf("unsafe.Slice of incomplete (or unallocatable) type not allowed")
10211027
}
1028+
10221029
if !checkunsafeslice(&n.Y) {
10231030
n.SetType(nil)
10241031
return n

src/cmd/compile/internal/walk/builtin.go

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -654,36 +654,28 @@ func walkRecover(nn *ir.CallExpr, init *ir.Nodes) ir.Node {
654654
}
655655

656656
func walkUnsafeSlice(n *ir.BinaryExpr, init *ir.Nodes) ir.Node {
657+
ptr := safeExpr(n.X, init)
657658
len := safeExpr(n.Y, init)
658659

659660
fnname := "unsafeslice64"
660-
argtype := types.Types[types.TINT64]
661+
lenType := types.Types[types.TINT64]
661662

662663
// Type checking guarantees that TIDEAL len/cap are positive and fit in an int.
663664
// The case of len or cap overflow when converting TUINT or TUINTPTR to TINT
664665
// will be handled by the negative range checks in unsafeslice during runtime.
665-
if len.Type().IsKind(types.TIDEAL) || len.Type().Size() <= types.Types[types.TUINT].Size() {
666+
if ir.ShouldCheckPtr(ir.CurFunc, 1) {
667+
fnname = "unsafeslicecheckptr"
668+
// for simplicity, unsafeslicecheckptr always uses int64
669+
} else if len.Type().IsKind(types.TIDEAL) || len.Type().Size() <= types.Types[types.TUINT].Size() {
666670
fnname = "unsafeslice"
667-
argtype = types.Types[types.TINT]
671+
lenType = types.Types[types.TINT]
668672
}
669673

670674
t := n.Type()
671675

672-
// Call runtime.unsafeslice[64] to check that the length argument is
673-
// non-negative and smaller than the max length allowed for the
674-
// element type.
676+
// Call runtime.unsafeslice{,64,checkptr} to check ptr and len.
675677
fn := typecheck.LookupRuntime(fnname)
676-
init.Append(mkcall1(fn, nil, init, reflectdata.TypePtr(t.Elem()), typecheck.Conv(len, argtype)))
677-
678-
ptr := walkExpr(n.X, init)
679-
680-
c := ir.NewUnaryExpr(n.Pos(), ir.OCHECKNIL, ptr)
681-
c.SetTypecheck(1)
682-
init.Append(c)
683-
684-
// TODO(mdempsky): checkptr instrumentation. Maybe merge into length
685-
// check above, along with nil check? Need to be careful about
686-
// notinheap pointers though: can't pass them as unsafe.Pointer.
678+
init.Append(mkcall1(fn, nil, init, reflectdata.TypePtr(t.Elem()), typecheck.Conv(ptr, types.Types[types.TUNSAFEPTR]), typecheck.Conv(len, lenType)))
687679

688680
h := ir.NewSliceHeaderExpr(n.Pos(), t,
689681
typecheck.Conv(ptr, types.Types[types.TUNSAFEPTR]),

src/runtime/checkptr.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,30 @@ func checkptrAlignment(p unsafe.Pointer, elem *_type, n uintptr) {
1616
}
1717

1818
// Check that (*[n]elem)(p) doesn't straddle multiple heap objects.
19-
if size := n * elem.size; size > 1 && checkptrBase(p) != checkptrBase(add(p, size-1)) {
19+
// TODO(mdempsky): Fix #46938 so we don't need to worry about overflow here.
20+
if checkptrStraddles(p, n*elem.size) {
2021
throw("checkptr: converted pointer straddles multiple allocations")
2122
}
2223
}
2324

25+
// checkptrStraddles reports whether the first size-bytes of memory
26+
// addressed by ptr is known to straddle more than one Go allocation.
27+
func checkptrStraddles(ptr unsafe.Pointer, size uintptr) bool {
28+
if size <= 1 {
29+
return false
30+
}
31+
32+
end := add(ptr, size-1)
33+
if uintptr(end) < uintptr(ptr) {
34+
return true
35+
}
36+
37+
// TODO(mdempsky): Detect when [ptr, end] contains Go allocations,
38+
// but neither ptr nor end point into one themselves.
39+
40+
return checkptrBase(ptr) != checkptrBase(end)
41+
}
42+
2443
func checkptrArithmetic(p unsafe.Pointer, originals []unsafe.Pointer) {
2544
if 0 < uintptr(p) && uintptr(p) < minLegalPointer {
2645
throw("checkptr: pointer arithmetic computed bad pointer value")

src/runtime/checkptr_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ func TestCheckPtr(t *testing.T) {
3030
{"CheckPtrArithmetic2", "fatal error: checkptr: pointer arithmetic result points to invalid allocation\n"},
3131
{"CheckPtrSize", "fatal error: checkptr: converted pointer straddles multiple allocations\n"},
3232
{"CheckPtrSmall", "fatal error: checkptr: pointer arithmetic computed bad pointer value\n"},
33+
{"CheckPtrSliceOK", ""},
34+
{"CheckPtrSliceFail", "fatal error: checkptr: unsafe.Slice result straddles multiple allocations\n"},
3335
}
3436

3537
for _, tc := range testCases {

src/runtime/slice.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,19 +112,37 @@ func makeslice64(et *_type, len64, cap64 int64) unsafe.Pointer {
112112
return makeslice(et, len, cap)
113113
}
114114

115-
func unsafeslice(et *_type, len int) {
115+
func unsafeslice(et *_type, ptr unsafe.Pointer, len int) {
116+
if len == 0 {
117+
return
118+
}
119+
120+
if ptr == nil {
121+
panic(errorString("unsafe.Slice: ptr is nil and len is not zero"))
122+
}
123+
116124
mem, overflow := math.MulUintptr(et.size, uintptr(len))
117125
if overflow || mem > maxAlloc || len < 0 {
118126
panicunsafeslicelen()
119127
}
120128
}
121129

122-
func unsafeslice64(et *_type, len64 int64) {
130+
func unsafeslice64(et *_type, ptr unsafe.Pointer, len64 int64) {
123131
len := int(len64)
124132
if int64(len) != len64 {
125133
panicunsafeslicelen()
126134
}
127-
unsafeslice(et, len)
135+
unsafeslice(et, ptr, len)
136+
}
137+
138+
func unsafeslicecheckptr(et *_type, ptr unsafe.Pointer, len64 int64) {
139+
unsafeslice64(et, ptr, len64)
140+
141+
// Check that underlying array doesn't straddle multiple heap objects.
142+
// unsafeslice64 has already checked for overflow.
143+
if checkptrStraddles(ptr, uintptr(len64)*et.size) {
144+
throw("checkptr: unsafe.Slice result straddles multiple allocations")
145+
}
128146
}
129147

130148
func panicunsafeslicelen() {

src/runtime/testdata/testprog/checkptr.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ func init() {
1313
register("CheckPtrArithmetic2", CheckPtrArithmetic2)
1414
register("CheckPtrSize", CheckPtrSize)
1515
register("CheckPtrSmall", CheckPtrSmall)
16+
register("CheckPtrSliceOK", CheckPtrSliceOK)
17+
register("CheckPtrSliceFail", CheckPtrSliceFail)
1618
}
1719

1820
func CheckPtrAlignmentNoPtr() {
@@ -49,3 +51,14 @@ func CheckPtrSize() {
4951
func CheckPtrSmall() {
5052
sink2 = unsafe.Pointer(uintptr(1))
5153
}
54+
55+
func CheckPtrSliceOK() {
56+
p := new([4]int64)
57+
sink2 = unsafe.Slice(&p[1], 3)
58+
}
59+
60+
func CheckPtrSliceFail() {
61+
p := new(int64)
62+
sink2 = p
63+
sink2 = unsafe.Slice(p, 100)
64+
}

test/unsafebuiltins.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,11 @@ func main() {
3030
assert(len(s) == len(p))
3131
assert(cap(s) == len(p))
3232

33-
// nil pointer
34-
mustPanic(func() { _ = unsafe.Slice((*int)(nil), 0) })
33+
// nil pointer with zero length returns nil
34+
assert(unsafe.Slice((*int)(nil), 0) == nil)
35+
36+
// nil pointer with positive length panics
37+
mustPanic(func() { _ = unsafe.Slice((*int)(nil), 1) })
3538

3639
// negative length
3740
var neg int = -1

0 commit comments

Comments
 (0)