Skip to content

Commit

Permalink
Create a correctly-sized slice to proxy *uint16
Browse files Browse the repository at this point in the history
Fixes the below issue seen in the containerd test suite.
```
fatal error: checkptr: converted pointer straddles multiple allocations
```

Also adds `-gcflags=all=-d=checkptr` to all the test runs on CI, to
avoid this regressing in future. This requires testing with Go 1.14 or
newer, so CI now runs on Go 1.15, as Go 1.14 did not recommend using
checkptr on Windows.

And _that_ requires the Visual Studio 2019 build image on AppVeyor.

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
  • Loading branch information
TBBle committed Jan 16, 2021
1 parent 3d95010 commit fe51c0e
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 25 deletions.
18 changes: 9 additions & 9 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
version: 0.1.{build}

image: Visual Studio 2017
image: Visual Studio 2019

clone_folder: c:\gopath\src\github.com\Microsoft\hcsshim

environment:
GOPATH: c:\gopath
PATH: "%GOPATH%\\bin;C:\\gometalinter-2.0.12-windows-amd64;%PATH%"

stack: go 1.13.4
stack: go 1.15

build_script:
- appveyor DownloadFile https://github.com/alecthomas/gometalinter/releases/download/v2.0.12/gometalinter-2.0.12-windows-amd64.zip
Expand All @@ -19,16 +19,16 @@ build_script:
- go build ./cmd/tar2ext4
- go build ./cmd/wclayer
- go build ./cmd/device-util
- go build ./internal/tools/grantvmgroupaccess
- go build ./internal/tools/grantvmgroupaccess
- go build ./internal/tools/uvmboot
- go build ./internal/tools/zapdir
- go test -v ./... -tags admin
- go test -gcflags=all=-d=checkptr -v ./... -tags admin
- cd test
- go test -v ./internal -tags admin
- go test -c ./containerd-shim-runhcs-v1/ -tags functional
- go test -c ./cri-containerd/ -tags functional
- go test -c ./functional/ -tags functional
- go test -c ./runhcs/ -tags functional
- go test -gcflags=all=-d=checkptr -v ./internal -tags admin
- go test -gcflags=all=-d=checkptr -c ./containerd-shim-runhcs-v1/ -tags functional
- go test -gcflags=all=-d=checkptr -c ./cri-containerd/ -tags functional
- go test -gcflags=all=-d=checkptr -c ./functional/ -tags functional
- go test -gcflags=all=-d=checkptr -c ./runhcs/ -tags functional
- go build -o sample-logging-driver.exe ./cri-containerd/helpers/log.go

artifacts:
Expand Down
2 changes: 1 addition & 1 deletion internal/safefile/safeopen.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func LinkRelative(oldname string, oldroot *os.File, newname string, newroot *os.
linkinfo := (*winapi.FileLinkInformation)(unsafe.Pointer(linkinfoBuffer))
linkinfo.RootDirectory = parent.Fd()
linkinfo.FileNameLength = uint32(len(newbase16) * 2)
copy((*[32768]uint16)(unsafe.Pointer(&linkinfo.FileName[0]))[:], newbase16)
copy(winapi.Uint16BufferToSlice(&linkinfo.FileName[0], len(newbase16)), newbase16)

var iosb winapi.IOStatusBlock
status := winapi.NtSetInformationFile(
Expand Down
19 changes: 14 additions & 5 deletions internal/winapi/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,23 @@ package winapi

import (
"errors"
"reflect"
"syscall"
"unicode/utf16"
"unsafe"
)

// Uint16BufferToSlice wraps a uint16 pointer-and-length into a slice
// for easier interop with Go APIs
func Uint16BufferToSlice(buffer *uint16, bufferLength int) (result []uint16) {
hdr := (*reflect.SliceHeader)(unsafe.Pointer(&result))
hdr.Data = uintptr(unsafe.Pointer(buffer))
hdr.Cap = bufferLength
hdr.Len = bufferLength

return
}

type UnicodeString struct {
Length uint16
MaximumLength uint16
Expand All @@ -15,12 +27,9 @@ type UnicodeString struct {

//String converts a UnicodeString to a golang string
func (uni UnicodeString) String() string {
p := (*[0xffff]uint16)(unsafe.Pointer(uni.Buffer))

// UnicodeString is not guaranteed to be null terminated, therefore
// use the UnicodeString's Length field
lengthInChars := uni.Length / 2
return syscall.UTF16ToString(p[:lengthInChars])
return syscall.UTF16ToString(Uint16BufferToSlice(uni.Buffer, int(uni.Length/2)))
}

// NewUnicodeString allocates a new UnicodeString and copies `s` into
Expand All @@ -36,7 +45,7 @@ func NewUnicodeString(s string) (*UnicodeString, error) {
MaximumLength: uint16(len(ws) * 2),
Buffer: &make([]uint16, len(ws))[0],
}
copy((*[32768]uint16)(unsafe.Pointer(uni.Buffer))[:], ws)
copy(Uint16BufferToSlice(uni.Buffer, len(ws)), ws)
return uni, nil
}

Expand Down
15 changes: 5 additions & 10 deletions internal/winapi/winapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ package winapi
import (
"testing"
"unicode/utf16"
"unsafe"
)

func wideStringsEqual(target, actual []uint16, actualLengthInBytes int) bool {
actualLength := actualLengthInBytes / 2
if len(target) != actualLength {
func wideStringsEqual(target, actual []uint16) bool {
if len(target) != len(actual) {
return false
}

Expand Down Expand Up @@ -38,13 +36,10 @@ func TestNewUnicodeString(t *testing.T) {
t.Fatalf("Expected new Unicode String maximum length to be %d for target string %s, got %d instead", targetLength, target, uni.MaximumLength)
}

uniBufferStringAsSlice := (*[32768]uint16)(unsafe.Pointer(uni.Buffer))[:]
uniBufferStringAsSlice := Uint16BufferToSlice(uni.Buffer, len(target))

// since we have to do casting to convert the unicode string's buffer into a uint16 slice
// the length of the actual slice will not be the true length of the contents in the unicode buffer
// therefore we need to use the unicode string's length field when comparing
if !wideStringsEqual(targetWideString, uniBufferStringAsSlice, int(uni.Length)) {
t.Fatalf("Expected wide string %v, got %v instead", targetWideString, uniBufferStringAsSlice[:uni.Length])
if !wideStringsEqual(targetWideString, uniBufferStringAsSlice) {
t.Fatalf("Expected wide string %v, got %v instead", targetWideString, uniBufferStringAsSlice)
}
}
}
Expand Down

0 comments on commit fe51c0e

Please sign in to comment.