From 4848eb04791849112096f6e86131c18d41b54820 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 7 Sep 2023 18:02:29 +0200 Subject: [PATCH] windows,windows\svc,windows\svc\mgr: use unsafe.Slice instead of unsafeheader.Slice unsafe.Slice is available since Go 1.17, which is already the minimum version supported by this package. This change removes the dependency on the internal unsafeheader package, which can be removed from the module. Change-Id: I6c34cb152f2336ea04c5f9c7e88797ed8914f9cc Reviewed-on: https://go-review.googlesource.com/c/sys/+/526635 Run-TryBot: Quim Muntal Reviewed-by: Bryan Mills LUCI-TryBot-Result: Go LUCI TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui --- internal/unsafeheader/unsafeheader.go | 30 ------ internal/unsafeheader/unsafeheader_test.go | 101 --------------------- windows/security_windows.go | 21 ++--- windows/svc/mgr/mgr.go | 8 +- windows/svc/mgr/recovery.go | 8 +- windows/svc/service.go | 7 +- windows/syscall_windows.go | 23 +---- windows/syscall_windows_test.go | 6 +- 8 files changed, 15 insertions(+), 189 deletions(-) delete mode 100644 internal/unsafeheader/unsafeheader.go delete mode 100644 internal/unsafeheader/unsafeheader_test.go diff --git a/internal/unsafeheader/unsafeheader.go b/internal/unsafeheader/unsafeheader.go deleted file mode 100644 index e07899b90..000000000 --- a/internal/unsafeheader/unsafeheader.go +++ /dev/null @@ -1,30 +0,0 @@ -// 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 string implementations. -// -// This package allows x/sys to use types equivalent to -// reflect.SliceHeader and reflect.StringHeader 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 -} - -// String 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 deleted file mode 100644 index 2793c1fa8..000000000 --- a/internal/unsafeheader/unsafeheader_test.go +++ /dev/null @@ -1,101 +0,0 @@ -// 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/windows/security_windows.go b/windows/security_windows.go index d414ef13b..26be94a8a 100644 --- a/windows/security_windows.go +++ b/windows/security_windows.go @@ -7,8 +7,6 @@ package windows import ( "syscall" "unsafe" - - "golang.org/x/sys/internal/unsafeheader" ) const ( @@ -1341,21 +1339,14 @@ func (selfRelativeSD *SECURITY_DESCRIPTOR) copySelfRelativeSecurityDescriptor() sdLen = min } - var src []byte - h := (*unsafeheader.Slice)(unsafe.Pointer(&src)) - h.Data = unsafe.Pointer(selfRelativeSD) - h.Len = sdLen - h.Cap = sdLen - + src := unsafe.Slice((*byte)(unsafe.Pointer(selfRelativeSD)), sdLen) + // SECURITY_DESCRIPTOR has pointers in it, which means checkptr expects for it to + // be aligned properly. When we're copying a Windows-allocated struct to a + // Go-allocated one, make sure that the Go allocation is aligned to the + // pointer size. const psize = int(unsafe.Sizeof(uintptr(0))) - - var dst []byte - h = (*unsafeheader.Slice)(unsafe.Pointer(&dst)) alloc := make([]uintptr, (sdLen+psize-1)/psize) - h.Data = (*unsafeheader.Slice)(unsafe.Pointer(&alloc)).Data - h.Len = sdLen - h.Cap = sdLen - + dst := unsafe.Slice((*byte)(unsafe.Pointer(&alloc[0])), sdLen) copy(dst, src) return (*SECURITY_DESCRIPTOR)(unsafe.Pointer(&dst[0])) } diff --git a/windows/svc/mgr/mgr.go b/windows/svc/mgr/mgr.go index c2dc8701d..4449161a7 100644 --- a/windows/svc/mgr/mgr.go +++ b/windows/svc/mgr/mgr.go @@ -17,7 +17,6 @@ import ( "unicode/utf16" "unsafe" - "golang.org/x/sys/internal/unsafeheader" "golang.org/x/sys/windows" ) @@ -199,12 +198,7 @@ func (m *Mgr) ListServices() ([]string, error) { if servicesReturned == 0 { return nil, nil } - - var services []windows.ENUM_SERVICE_STATUS_PROCESS - hdr := (*unsafeheader.Slice)(unsafe.Pointer(&services)) - hdr.Data = unsafe.Pointer(&buf[0]) - hdr.Len = int(servicesReturned) - hdr.Cap = int(servicesReturned) + services := unsafe.Slice((*windows.ENUM_SERVICE_STATUS_PROCESS)(unsafe.Pointer(&buf[0])), int(servicesReturned)) var names []string for _, s := range services { diff --git a/windows/svc/mgr/recovery.go b/windows/svc/mgr/recovery.go index 321451990..f7d13bbdf 100644 --- a/windows/svc/mgr/recovery.go +++ b/windows/svc/mgr/recovery.go @@ -13,7 +13,6 @@ import ( "time" "unsafe" - "golang.org/x/sys/internal/unsafeheader" "golang.org/x/sys/windows" ) @@ -70,12 +69,7 @@ func (s *Service) RecoveryActions() ([]RecoveryAction, error) { return nil, err } - var actions []windows.SC_ACTION - hdr := (*unsafeheader.Slice)(unsafe.Pointer(&actions)) - hdr.Data = unsafe.Pointer(p.Actions) - hdr.Len = int(p.ActionsCount) - hdr.Cap = int(p.ActionsCount) - + actions := unsafe.Slice(p.Actions, int(p.ActionsCount)) var recoveryActions []RecoveryAction for _, action := range actions { recoveryActions = append(recoveryActions, RecoveryAction{Type: int(action.Type), Delay: time.Duration(action.Delay) * time.Millisecond}) diff --git a/windows/svc/service.go b/windows/svc/service.go index 2b4a7bc6c..e9e47f0b4 100644 --- a/windows/svc/service.go +++ b/windows/svc/service.go @@ -13,7 +13,6 @@ import ( "sync" "unsafe" - "golang.org/x/sys/internal/unsafeheader" "golang.org/x/sys/windows" ) @@ -222,11 +221,7 @@ func serviceMain(argc uint32, argv **uint16) uintptr { defer func() { theService.h = 0 }() - var args16 []*uint16 - hdr := (*unsafeheader.Slice)(unsafe.Pointer(&args16)) - hdr.Data = unsafe.Pointer(argv) - hdr.Len = int(argc) - hdr.Cap = int(argc) + args16 := unsafe.Slice(argv, int(argc)) args := make([]string, len(args16)) for i, a := range args16 { diff --git a/windows/syscall_windows.go b/windows/syscall_windows.go index 67bad0926..cb8c75a42 100644 --- a/windows/syscall_windows.go +++ b/windows/syscall_windows.go @@ -15,8 +15,6 @@ import ( "time" "unicode/utf16" "unsafe" - - "golang.org/x/sys/internal/unsafeheader" ) type Handle uintptr @@ -1667,12 +1665,8 @@ func NewNTUnicodeString(s string) (*NTUnicodeString, error) { // Slice returns a uint16 slice that aliases the data in the NTUnicodeString. func (s *NTUnicodeString) Slice() []uint16 { - var slice []uint16 - hdr := (*unsafeheader.Slice)(unsafe.Pointer(&slice)) - hdr.Data = unsafe.Pointer(s.Buffer) - hdr.Len = int(s.Length) - hdr.Cap = int(s.MaximumLength) - return slice + slice := unsafe.Slice(s.Buffer, s.MaximumLength) + return slice[:s.Length] } func (s *NTUnicodeString) String() string { @@ -1695,12 +1689,8 @@ func NewNTString(s string) (*NTString, error) { // Slice returns a byte slice that aliases the data in the NTString. func (s *NTString) Slice() []byte { - var slice []byte - hdr := (*unsafeheader.Slice)(unsafe.Pointer(&slice)) - hdr.Data = unsafe.Pointer(s.Buffer) - hdr.Len = int(s.Length) - hdr.Cap = int(s.MaximumLength) - return slice + slice := unsafe.Slice(s.Buffer, s.MaximumLength) + return slice[:s.Length] } func (s *NTString) String() string { @@ -1752,10 +1742,7 @@ func LoadResourceData(module, resInfo Handle) (data []byte, err error) { if err != nil { return } - h := (*unsafeheader.Slice)(unsafe.Pointer(&data)) - h.Data = unsafe.Pointer(ptr) - h.Len = int(size) - h.Cap = int(size) + data = unsafe.Slice((*byte)(unsafe.Pointer(ptr)), size) return } diff --git a/windows/syscall_windows_test.go b/windows/syscall_windows_test.go index 5b6bfb080..40b4d54ef 100644 --- a/windows/syscall_windows_test.go +++ b/windows/syscall_windows_test.go @@ -22,7 +22,6 @@ import ( "time" "unsafe" - "golang.org/x/sys/internal/unsafeheader" "golang.org/x/sys/windows" ) @@ -865,10 +864,7 @@ func TestSystemModuleVersions(t *testing.T) { return } mods := (*windows.RTL_PROCESS_MODULES)(unsafe.Pointer(&moduleBuffer[0])) - hdr := (*unsafeheader.Slice)(unsafe.Pointer(&modules)) - hdr.Data = unsafe.Pointer(&mods.Modules[0]) - hdr.Len = int(mods.NumberOfModules) - hdr.Cap = int(mods.NumberOfModules) + modules = unsafe.Slice(&mods.Modules[0], int(mods.NumberOfModules)) break } for i := range modules {