unix: use a type equivalent to reflect.SliceHeader to manipulate slices
The regression test included in this change verifies that the type is,
in fact, equivalent, while allowing the actual header definitions to
avoid importing the (relatively heavy) "reflect" package itself.
This change is loosely based on Keyan Pishdadian's draft in CL 230557.
For golang/go#37805
Change-Id: I998c69cdeb852154cd66ab5fdaa542a6f19666a2
Reviewed-on: https://go-review.googlesource.com/c/sys/+/231177
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
diff --git a/internal/unsafeheader/unsafeheader.go b/internal/unsafeheader/unsafeheader.go
new file mode 100644
index 0000000..bfc5d3c
--- /dev/null
+++ b/internal/unsafeheader/unsafeheader.go
@@ -0,0 +1,30 @@
+// Copyright 2020 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 unsafeheader contains header declarations for the Go runtime's
+// slice and struct implementations.
+//
+// This package allows x/sys to use types equivalent to
+// reflect.SliceHeader and reflect.StructHeader without introducing
+// a dependency on the (relatively heavy) "reflect" package.
+package unsafeheader
+
+import (
+ "unsafe"
+)
+
+// Slice is the runtime representation of a slice.
+// It cannot be used safely or portably and its representation may change in a later release.
+type Slice struct {
+ Data unsafe.Pointer
+ Len int
+ Cap int
+}
+
+// StringHeader is the runtime representation of a string.
+// It cannot be used safely or portably and its representation may change in a later release.
+type String struct {
+ Data unsafe.Pointer
+ Len int
+}
diff --git a/internal/unsafeheader/unsafeheader_test.go b/internal/unsafeheader/unsafeheader_test.go
new file mode 100644
index 0000000..2793c1f
--- /dev/null
+++ b/internal/unsafeheader/unsafeheader_test.go
@@ -0,0 +1,101 @@
+// Copyright 2020 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 unsafeheader_test
+
+import (
+ "bytes"
+ "reflect"
+ "testing"
+ "unsafe"
+
+ "golang.org/x/sys/internal/unsafeheader"
+)
+
+// TestTypeMatchesReflectType ensures that the name and layout of the
+// unsafeheader types matches the corresponding Header types in the reflect
+// package.
+func TestTypeMatchesReflectType(t *testing.T) {
+ t.Run("Slice", func(t *testing.T) {
+ testHeaderMatchesReflect(t, unsafeheader.Slice{}, reflect.SliceHeader{})
+ })
+
+ t.Run("String", func(t *testing.T) {
+ testHeaderMatchesReflect(t, unsafeheader.String{}, reflect.StringHeader{})
+ })
+}
+
+func testHeaderMatchesReflect(t *testing.T, header, reflectHeader interface{}) {
+ h := reflect.TypeOf(header)
+ rh := reflect.TypeOf(reflectHeader)
+
+ for i := 0; i < h.NumField(); i++ {
+ f := h.Field(i)
+ rf, ok := rh.FieldByName(f.Name)
+ if !ok {
+ t.Errorf("Field %d of %v is named %s, but no such field exists in %v", i, h, f.Name, rh)
+ continue
+ }
+ if !typeCompatible(f.Type, rf.Type) {
+ t.Errorf("%v.%s has type %v, but %v.%s has type %v", h, f.Name, f.Type, rh, rf.Name, rf.Type)
+ }
+ if f.Offset != rf.Offset {
+ t.Errorf("%v.%s has offset %d, but %v.%s has offset %d", h, f.Name, f.Offset, rh, rf.Name, rf.Offset)
+ }
+ }
+
+ if h.NumField() != rh.NumField() {
+ t.Errorf("%v has %d fields, but %v has %d", h, h.NumField(), rh, rh.NumField())
+ }
+ if h.Align() != rh.Align() {
+ t.Errorf("%v has alignment %d, but %v has alignment %d", h, h.Align(), rh, rh.Align())
+ }
+}
+
+var (
+ unsafePointerType = reflect.TypeOf(unsafe.Pointer(nil))
+ uintptrType = reflect.TypeOf(uintptr(0))
+)
+
+func typeCompatible(t, rt reflect.Type) bool {
+ return t == rt || (t == unsafePointerType && rt == uintptrType)
+}
+
+// TestWriteThroughHeader ensures that the headers in the unsafeheader package
+// can successfully mutate variables of the corresponding built-in types.
+//
+// This test is expected to fail under -race (which implicitly enables
+// -d=checkptr) if the runtime views the header types as incompatible with the
+// underlying built-in types.
+func TestWriteThroughHeader(t *testing.T) {
+ t.Run("Slice", func(t *testing.T) {
+ s := []byte("Hello, checkptr!")[:5]
+
+ var alias []byte
+ hdr := (*unsafeheader.Slice)(unsafe.Pointer(&alias))
+ hdr.Data = unsafe.Pointer(&s[0])
+ hdr.Cap = cap(s)
+ hdr.Len = len(s)
+
+ if !bytes.Equal(alias, s) {
+ t.Errorf("alias of %T(%q) constructed via Slice = %T(%q)", s, s, alias, alias)
+ }
+ if cap(alias) != cap(s) {
+ t.Errorf("alias of %T with cap %d has cap %d", s, cap(s), cap(alias))
+ }
+ })
+
+ t.Run("String", func(t *testing.T) {
+ s := "Hello, checkptr!"
+
+ var alias string
+ hdr := (*unsafeheader.String)(unsafe.Pointer(&alias))
+ hdr.Data = (*unsafeheader.String)(unsafe.Pointer(&s)).Data
+ hdr.Len = len(s)
+
+ if alias != s {
+ t.Errorf("alias of %q constructed via String = %q", s, alias)
+ }
+ })
+}
diff --git a/unix/syscall_darwin.1_13.go b/unix/syscall_darwin.1_13.go
index f911617..dc0befe 100644
--- a/unix/syscall_darwin.1_13.go
+++ b/unix/syscall_darwin.1_13.go
@@ -6,7 +6,11 @@
package unix
-import "unsafe"
+import (
+ "unsafe"
+
+ "golang.org/x/sys/internal/unsafeheader"
+)
//sys closedir(dir uintptr) (err error)
//sys readdir_r(dir uintptr, entry *Dirent, result **Dirent) (res Errno)
@@ -71,6 +75,7 @@
cnt++
continue
}
+
reclen := int(entry.Reclen)
if reclen > len(buf) {
// Not enough room. Return for now.
@@ -79,13 +84,15 @@
// restarting is O(n^2) in the length of the directory. Oh well.
break
}
+
// Copy entry into return buffer.
- s := struct {
- ptr unsafe.Pointer
- siz int
- cap int
- }{ptr: unsafe.Pointer(&entry), siz: reclen, cap: reclen}
- copy(buf, *(*[]byte)(unsafe.Pointer(&s)))
+ var s []byte
+ hdr := (*unsafeheader.Slice)(unsafe.Pointer(&s))
+ hdr.Data = unsafe.Pointer(&entry)
+ hdr.Cap = reclen
+ hdr.Len = reclen
+ copy(buf, s)
+
buf = buf[reclen:]
n += reclen
cnt++
diff --git a/unix/syscall_unix.go b/unix/syscall_unix.go
index 8f710d0..400ba9f 100644
--- a/unix/syscall_unix.go
+++ b/unix/syscall_unix.go
@@ -12,6 +12,8 @@
"sync"
"syscall"
"unsafe"
+
+ "golang.org/x/sys/internal/unsafeheader"
)
var (
@@ -113,15 +115,12 @@
return nil, errno
}
- // Slice memory layout
- var sl = struct {
- addr uintptr
- len int
- cap int
- }{addr, length, length}
-
- // Use unsafe to turn sl into a []byte.
- b := *(*[]byte)(unsafe.Pointer(&sl))
+ // Use unsafe to convert addr into a []byte.
+ var b []byte
+ hdr := (*unsafeheader.Slice)(unsafe.Pointer(&b))
+ hdr.Data = unsafe.Pointer(addr)
+ hdr.Cap = length
+ hdr.Len = length
// Register mapping in m and return it.
p := &b[cap(b)-1]
