Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/sys/windows/mkwinsyscall: generates invalid calls for GOARCH=386 #42373

Open
adriansr opened this issue Nov 4, 2020 · 5 comments
Open

x/sys/windows/mkwinsyscall: generates invalid calls for GOARCH=386 #42373

adriansr opened this issue Nov 4, 2020 · 5 comments

Comments

@adriansr
Copy link

@adriansr adriansr commented Nov 4, 2020

What version of Go are you using (go version)?

go version go1.15.3 windows/386

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

set GO111MODULE=
set GOARCH=386
set GOBIN=
set GOCACHE=C:\Users\vagrant\AppData\Local\go-build
set GOENV=C:\Users\vagrant\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=386
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\vagrant\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\vagrant\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\go\pkg\tool\windows_386
set GCCGO=gccgo
set GO386=sse2
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\vagrant\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m32 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\vagrant\AppData\Local\Temp\go-build184742666=/tmp/go-build -gno-record-gcc-switches

What did you do?

package foo

//sys _Foo(param1 uint64, param2 int) (success bool, err error) = mylib._Foo

What did you expect to see?

For arches where an uintptr is 32-bit, the first parameter should take two arguments in Syscall:

func _Foo(param1 uint64, param2 int) (success bool, err error) {
        var (
                r0 uintptr
                e1 syscall.Errno
        )
        if unsafe.Sizeof(uintptr(0)) == unsafe.Sizeof(uint64(0)) {
                r0, _, e1 = syscall.Syscall(proc_Foo.Addr(), 2, uintptr(param1), uintptr(param2), 0)
        } else {
                var _p0 [2]uintptr = *(*[2]uintptr)(unsafe.Pointer(&param1))
                r0, _, e1 = syscall.Syscall(proc_Foo.Addr(), 3, _p0[0], _p0[1], uintptr(param2))
        }

        success = r0 != 0
        if success == 0 {
                err = errnoErr(e1)
        }
        return
}

What did you see instead?

func _Foo(param1 uint64, param2 int) (success bool, err error) {
        r0, _, e1 := syscall.Syscall(proc_Foo.Addr(), 2, uintptr(param1), uintptr(param2), 0)
        success = r0 != 0
        if success == 0 {
                err = errnoErr(e1)
        }
        return
}

This call won't be valid when GOARCH=386 as param1 (64bit) is being cast to an uintptr (32bit). The receiver would be expecting 64-bits on the stack for the first argument.

@gopherbot gopherbot added this to the Unreleased milestone Nov 4, 2020
@adriansr
Copy link
Author

@adriansr adriansr commented Nov 4, 2020

I have a fix in my fork which I will submit for review:

adriansr/sys@2cd7216

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 4, 2020

Change https://golang.org/cl/267617 mentions this issue: windows/mkwinsyscall: Pass 64-bit arguments in 32-bit arch correctly

Loading

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Nov 4, 2020

```go
//sys _Foo(param1 uint64, param2 int) (success bool, err error) = mylib._Foo

Which Windows API do you have trouble with?

Alex

Loading

@bcmills bcmills changed the title x/sys: mkwinsyscall generates invalid calls for GOARCH=386 x/sys/windows/mkwinsyscall: generates invalid calls for GOARCH=386 Nov 4, 2020
@adriansr
Copy link
Author

@adriansr adriansr commented Nov 4, 2020

Loading

@nanokatze
Copy link

@nanokatze nanokatze commented Nov 11, 2020

Currently it appears that in syscall package, calls with 64-bit arguments are handled on individual basis.

// setFilePointerEx calls SetFilePointerEx.
// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa365542(v=vs.85).aspx
func setFilePointerEx(handle Handle, distToMove int64, newFilePointer *int64, whence uint32) error {
var e1 Errno
switch runtime.GOARCH {
default:
panic("unsupported architecture")
case "amd64":
_, _, e1 = Syscall6(procSetFilePointerEx.Addr(), 4, uintptr(handle), uintptr(distToMove), uintptr(unsafe.Pointer(newFilePointer)), uintptr(whence), 0, 0)
case "386":
// distToMove is a LARGE_INTEGER:
// https://msdn.microsoft.com/en-us/library/windows/desktop/aa383713(v=vs.85).aspx
_, _, e1 = Syscall6(procSetFilePointerEx.Addr(), 5, uintptr(handle), uintptr(distToMove), uintptr(distToMove>>32), uintptr(unsafe.Pointer(newFilePointer)), uintptr(whence), 0)
case "arm":
// distToMove must be 8-byte aligned per ARM calling convention
// https://msdn.microsoft.com/en-us/library/dn736986.aspx#Anchor_7
_, _, e1 = Syscall6(procSetFilePointerEx.Addr(), 6, uintptr(handle), 0, uintptr(distToMove), uintptr(distToMove>>32), uintptr(unsafe.Pointer(newFilePointer)), uintptr(whence))
}
if e1 != 0 {
return errnoErr(e1)
}
return nil
}
It appears that your fix would be incomplete as it does not address certain arm-specific peculiarities.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants