From 90f0ae3b59ed56d2064e619bfc1c7f513201d208 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Thu, 26 Jan 2023 13:27:13 -0800 Subject: [PATCH] Replace gohacks.SliceHeader with unsafe.Slice All remaining uses of gohacks.SliceHeader are to create a slice pointing to some other backing array. Go 1.20 introduces unsafe.Slice to do exactly this, so switch to this interface. For now we want code to continue to build with Go 1.19, so we still use a gohacks.Slice wrapper, which uses unsafe.Slice on 1.20 and SliceHeader on <1.20. Once 1.19 support is dropped, uses can drop gohacks altogether. gohacks.Slice is inlined into callers, and unsafe.Slice is a compiler intrinsic, so these wrappers have minimal overhead. The primary difference is the addition of a nil check on the pointer and an overflow check on ptr+length*size. I think these are minimal enough to not cause problems, but if they are (e.g., in safemem), we could consider adding a SliceUnchecked function that continues to use SliceHeader. But I'd like to try to avoid that, as it adds process to verify it is still compatible with new Go releases. For #8422. PiperOrigin-RevId: 504926819 --- pkg/gohacks/BUILD | 2 ++ pkg/gohacks/gohacks_unsafe.go | 17 ++-------- pkg/gohacks/slice_go113_unsafe.go | 45 +++++++++++++++++++++++++++ pkg/gohacks/slice_go120_unsafe.go | 30 ++++++++++++++++++ pkg/hostarch/addr_range_seq_unsafe.go | 9 ++---- pkg/safemem/block_unsafe.go | 6 +--- pkg/safemem/seq_unsafe.go | 9 ++---- 7 files changed, 86 insertions(+), 32 deletions(-) create mode 100644 pkg/gohacks/slice_go113_unsafe.go create mode 100644 pkg/gohacks/slice_go120_unsafe.go diff --git a/pkg/gohacks/BUILD b/pkg/gohacks/BUILD index b4e05f922..38b4db9a6 100644 --- a/pkg/gohacks/BUILD +++ b/pkg/gohacks/BUILD @@ -6,6 +6,8 @@ go_library( name = "gohacks", srcs = [ "gohacks_unsafe.go", + "slice_go113_unsafe.go", + "slice_go120_unsafe.go", ], stateify = False, visibility = ["//:sandbox"], diff --git a/pkg/gohacks/gohacks_unsafe.go b/pkg/gohacks/gohacks_unsafe.go index 0711de9d2..6452163a4 100644 --- a/pkg/gohacks/gohacks_unsafe.go +++ b/pkg/gohacks/gohacks_unsafe.go @@ -30,15 +30,6 @@ import ( "unsafe" ) -// SliceHeader is equivalent to reflect.SliceHeader, but represents the pointer -// to the underlying array as unsafe.Pointer rather than uintptr, allowing -// SliceHeaders to be directly converted to slice objects. -type SliceHeader struct { - Data unsafe.Pointer - Len int - Cap int -} - // StringHeader is equivalent to reflect.StringHeader, but represents the // pointer to the underlying array as unsafe.Pointer rather than uintptr, // allowing StringHeaders to be directly converted to strings. @@ -63,13 +54,9 @@ func Noescape(p unsafe.Pointer) unsafe.Pointer { // ImmutableBytesFromString is equivalent to []byte(s), except that it uses the // same memory backing s instead of making a heap-allocated copy. This is only // valid if the returned slice is never mutated. -func ImmutableBytesFromString(s string) (bs []byte) { +func ImmutableBytesFromString(s string) []byte { shdr := (*StringHeader)(unsafe.Pointer(&s)) - bshdr := (*SliceHeader)(unsafe.Pointer(&bs)) - bshdr.Data = shdr.Data - bshdr.Len = shdr.Len - bshdr.Cap = shdr.Len - return + return Slice((*byte)(shdr.Data), shdr.Len) } // StringFromImmutableBytes is equivalent to string(bs), except that it uses diff --git a/pkg/gohacks/slice_go113_unsafe.go b/pkg/gohacks/slice_go113_unsafe.go new file mode 100644 index 000000000..8ee39f560 --- /dev/null +++ b/pkg/gohacks/slice_go113_unsafe.go @@ -0,0 +1,45 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.13 && !go1.20 +// +build go1.13,!go1.20 + +// TODO(go.dev/issue/8422): Remove this once Go 1.19 is no longer supported, +// and update callers to use unsafe.Slice directly. + +package gohacks + +import ( + "unsafe" +) + +// sliceHeader is equivalent to reflect.SliceHeader, but represents the pointer +// to the underlying array as unsafe.Pointer rather than uintptr, allowing +// sliceHeaders to be directly converted to slice objects. +type sliceHeader struct { + Data unsafe.Pointer + Len int + Cap int +} + +// Slice returns a slice whose underlying array starts at ptr an which length +// and capacity are len. +func Slice[T any](ptr *T, length int) []T { + var s []T + hdr := (*sliceHeader)(unsafe.Pointer(&s)) + hdr.Data = unsafe.Pointer(ptr) + hdr.Len = length + hdr.Cap = length + return s +} diff --git a/pkg/gohacks/slice_go120_unsafe.go b/pkg/gohacks/slice_go120_unsafe.go new file mode 100644 index 000000000..9778db863 --- /dev/null +++ b/pkg/gohacks/slice_go120_unsafe.go @@ -0,0 +1,30 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.20 + +package gohacks + +import ( + "unsafe" +) + +// Slice returns a slice whose underlying array starts at ptr an which length +// and capacity are len. +// +// Slice is a wrapper around unsafe.Slice. Prefer to use unsafe.Slice directly +// if possible. +func Slice[T any](ptr *T, length int) []T { + return unsafe.Slice(ptr, length) +} diff --git a/pkg/hostarch/addr_range_seq_unsafe.go b/pkg/hostarch/addr_range_seq_unsafe.go index 31b0452c1..c2f72f965 100644 --- a/pkg/hostarch/addr_range_seq_unsafe.go +++ b/pkg/hostarch/addr_range_seq_unsafe.go @@ -158,16 +158,13 @@ func (ars AddrRangeSeq) Tail() AddrRangeSeq { // Preconditions: ars.length >= 2. func (ars AddrRangeSeq) externalTail() AddrRangeSeq { - headLen := (*AddrRange)(ars.data).Length() - ars.offset + data := (*AddrRange)(ars.data) + headLen := data.Length() - ars.offset var tailLimit int64 if ars.limit > headLen { tailLimit = int64(ars.limit - headLen) } - var extSlice []AddrRange - extSliceHdr := (*gohacks.SliceHeader)(unsafe.Pointer(&extSlice)) - extSliceHdr.Data = ars.data - extSliceHdr.Len = ars.length - extSliceHdr.Cap = ars.length + extSlice := gohacks.Slice(data, ars.length) return addrRangeSeqFromSliceLimited(extSlice[1:], tailLimit) } diff --git a/pkg/safemem/block_unsafe.go b/pkg/safemem/block_unsafe.go index 7d4c53f0d..6ac131e59 100644 --- a/pkg/safemem/block_unsafe.go +++ b/pkg/safemem/block_unsafe.go @@ -149,11 +149,7 @@ func (b Block) TakeFirst64(n uint64) Block { // ToSlice returns a []byte equivalent to b. func (b Block) ToSlice() []byte { - return *(*[]byte)(unsafe.Pointer(&gohacks.SliceHeader{ - Data: b.start, - Len: b.length, - Cap: b.length, - })) + return gohacks.Slice((*byte)(b.start), b.length) } // Addr returns b's start address as a uintptr. It returns uintptr instead of diff --git a/pkg/safemem/seq_unsafe.go b/pkg/safemem/seq_unsafe.go index 027f6c65d..b28e5e2b9 100644 --- a/pkg/safemem/seq_unsafe.go +++ b/pkg/safemem/seq_unsafe.go @@ -178,17 +178,14 @@ func (bs BlockSeq) Tail() BlockSeq { if bs.length < 0 { return BlockSeq{} } - head := (*Block)(bs.data).DropFirst(bs.offset) + data := (*Block)(bs.data) + head := data.DropFirst(bs.offset) headLen := uint64(head.Len()) if headLen >= bs.limit { // The head Block exhausts the limit, so the tail is empty. return BlockSeq{} } - var extSlice []Block - extSliceHdr := (*gohacks.SliceHeader)(unsafe.Pointer(&extSlice)) - extSliceHdr.Data = bs.data - extSliceHdr.Len = bs.length - extSliceHdr.Cap = bs.length + extSlice := gohacks.Slice(data, bs.length) tailSlice := skipEmpty(extSlice[1:]) tailLimit := bs.limit - headLen return blockSeqFromSliceLimited(tailSlice, tailLimit)