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

syscall: floating point parameters are not supported on windows/arm64 #62583

Open
agiacomolli opened this issue Sep 12, 2023 · 8 comments
Open
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@agiacomolli
Copy link

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

$ go version
go version go1.21.1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/agiacomolli/.cache/go-build'
GOENV='/home/agiacomolli/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/agiacomolli/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/agiacomolli/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/tmp/go1.21.1'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/tmp/go1.21.1/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/tmp/windows-crash/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build63189677=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Cross build for Windows from a Linux machine. amd64 build works as expected running on both amd64 or arm64 machine.

arm64 version crashes with a runtime exception.

package main

import (
        "log"
        "math"
        "syscall"
        "time"
        "unsafe"

        "golang.org/x/sys/windows"
)

var (
        modoleaut32 = windows.NewLazySystemDLL("oleaut32.dll")

        procVariantTimeToSystemTime = modoleaut32.NewProc("VariantTimeToSystemTime")
)

func main() {
        t, err := GetVariantDate(45000.0)
        if err != nil {
                log.Fatal(err)
        }

        log.Printf("time = %v", t)
}

func GetVariantDate(value float64) (time.Time, error) {
        var st syscall.Systemtime

        r, _, err := procVariantTimeToSystemTime.Call(
                uintptr(math.Float64bits(value)),
                uintptr(unsafe.Pointer(&st)),
        )
        if r == 0 {
                return time.Time{}, err
        }

        return time.Date(
                int(st.Year),
                time.Month(st.Month),
                int(st.Day),
                int(st.Hour),
                int(st.Minute),
                int(st.Second),
                int(st.Milliseconds/1000),
                time.UTC,
        ), nil
}

What did you expect to see?

Building on Linux:

GOOS=windows GOARCH=arm64 go build -o crash.exe main.go

Running on Windows ARM64:

PS C:\tmp> .\crash.exe
2023/09/11 19:30:33 time = 2023-03-15 00:00:00 +0000 UTC

What did you see instead?

PS C:\tmp> .\crash.exe
Exception 0xc0000005 0x1 0x40e5f9000000000e 0x7ffb0b780d30
PC=0x7ffb0b780d30

runtime.cgocall(0x7ff784c919c0, 0x7ff784da1348)
        /tmp/go1.21.1/src/runtime/cgocall.go:157 +0x38 fp=0x40000b7c80 sp=0x40000b7c40 pc=0x7ff784c33b98
syscall.SyscallN(0x7ffb0b780cf0?, {0x40000b7d30?, 0x3?, 0x7ff784ccb3a8?})
        /tmp/go1.21.1/src/runtime/syscall_windows.go:544 +0xe8 fp=0x40000b7d00 sp=0x40000b7c80 pc=0x7ff784c8d9f8
syscall.Syscall(0x40000b7d88?, 0x7ff784ccb3d4?, 0x40000b7d88?, 0x7ff784ccb3f4?, 0x7ff784d92134?)
        /tmp/go1.21.1/src/runtime/syscall_windows.go:482 +0x30 fp=0x40000b7d50 sp=0x40000b7d00 pc=0x7ff784c8d750
golang.org/x/sys/windows.(*Proc).Call(0x7ff784d92160?, {0x400009a0f0?, 0x4000047e01?, 0x7ff784c476e0?})
        /home/agiacomolli/go/pkg/mod/golang.org/x/sys@v0.12.0/windows/dll_windows.go:172 +0xe0 fp=0x40000b7e00 sp=0x40000b7d50 pc=0x7ff784ccaa80
golang.org/x/sys/windows.(*LazyProc).Call(0x7ff784d92160, {0x400009a0f0, 0x2, 0x2})
        /home/agiacomolli/go/pkg/mod/golang.org/x/sys@v0.12.0/windows/dll_windows.go:348 +0x4c fp=0x40000b7e30 sp=0x40000b7e00 pc=0x7ff784ccb53c
main.GetVariantDate(0x40e5f90000000000)
        /tmp/windows-crash/main.go:31 +0x6c fp=0x40000b7e90 sp=0x40000b7e30 pc=0x7ff784cd187c
main.main()
        /tmp/windows-crash/main.go:20 +0x28 fp=0x40000b7f30 sp=0x40000b7e90 pc=0x7ff784cd16d8
runtime.main()
        /tmp/go1.21.1/src/runtime/proc.go:267 +0x2a4 fp=0x40000b7fd0 sp=0x40000b7f30 pc=0x7ff784c66d74
runtime.goexit()
        /tmp/go1.21.1/src/runtime/asm_arm64.s:1197 +0x4 fp=0x40000b7fd0 sp=0x40000b7fd0 pc=0x7ff784c90d14

goroutine 2 [force gc (idle)]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        /tmp/go1.21.1/src/runtime/proc.go:398 +0xc8 fp=0x4000043f90 sp=0x4000043f70 pc=0x7ff784c67188
runtime.goparkunlock(...)
        /tmp/go1.21.1/src/runtime/proc.go:404
runtime.forcegchelper()
        /tmp/go1.21.1/src/runtime/proc.go:322 +0xb8 fp=0x4000043fd0 sp=0x4000043f90 pc=0x7ff784c67018
runtime.goexit()
        /tmp/go1.21.1/src/runtime/asm_arm64.s:1197 +0x4 fp=0x4000043fd0 sp=0x4000043fd0 pc=0x7ff784c90d14
created by runtime.init.6 in goroutine 1
        /tmp/go1.21.1/src/runtime/proc.go:310 +0x24

goroutine 3 [GC sweep wait]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        /tmp/go1.21.1/src/runtime/proc.go:398 +0xc8 fp=0x4000045f60 sp=0x4000045f40 pc=0x7ff784c67188
runtime.goparkunlock(...)
        /tmp/go1.21.1/src/runtime/proc.go:404
runtime.bgsweep(0x0?)
        /tmp/go1.21.1/src/runtime/mgcsweep.go:280 +0xa0 fp=0x4000045fb0 sp=0x4000045f60 pc=0x7ff784c527a0
runtime.gcenable.func1()
        /tmp/go1.21.1/src/runtime/mgc.go:200 +0x28 fp=0x4000045fd0 sp=0x4000045fb0 pc=0x7ff784c474e8
runtime.goexit()
        /tmp/go1.21.1/src/runtime/asm_arm64.s:1197 +0x4 fp=0x4000045fd0 sp=0x4000045fd0 pc=0x7ff784c90d14
created by runtime.gcenable in goroutine 1
        /tmp/go1.21.1/src/runtime/mgc.go:200 +0x6c

goroutine 4 [GC scavenge wait]:
runtime.gopark(0x4000016070?, 0x7ff784d17d18?, 0x1?, 0x0?, 0x4000040b60?)
        /tmp/go1.21.1/src/runtime/proc.go:398 +0xc8 fp=0x4000055f50 sp=0x4000055f30 pc=0x7ff784c67188
runtime.goparkunlock(...)
        /tmp/go1.21.1/src/runtime/proc.go:404
runtime.(*scavengerState).park(0x7ff784da0b80)
        /tmp/go1.21.1/src/runtime/mgcscavenge.go:425 +0x5c fp=0x4000055f80 sp=0x4000055f50 pc=0x7ff784c5003c
runtime.bgscavenge(0x0?)
        /tmp/go1.21.1/src/runtime/mgcscavenge.go:653 +0x44 fp=0x4000055fb0 sp=0x4000055f80 pc=0x7ff784c50584
runtime.gcenable.func2()
        /tmp/go1.21.1/src/runtime/mgc.go:201 +0x28 fp=0x4000055fd0 sp=0x4000055fb0 pc=0x7ff784c47488
runtime.goexit()
        /tmp/go1.21.1/src/runtime/asm_arm64.s:1197 +0x4 fp=0x4000055fd0 sp=0x4000055fd0 pc=0x7ff784c90d14
created by runtime.gcenable in goroutine 1
        /tmp/go1.21.1/src/runtime/mgc.go:201 +0xac

goroutine 18 [finalizer wait]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        /tmp/go1.21.1/src/runtime/proc.go:398 +0xc8 fp=0x4000047d80 sp=0x4000047d60 pc=0x7ff784c67188
runtime.runfinq()
        /tmp/go1.21.1/src/runtime/mfinal.go:193 +0x108 fp=0x4000047fd0 sp=0x4000047d80 pc=0x7ff784c46618
runtime.goexit()
        /tmp/go1.21.1/src/runtime/asm_arm64.s:1197 +0x4 fp=0x4000047fd0 sp=0x4000047fd0 pc=0x7ff784c90d14
created by runtime.createfing in goroutine 1
        /tmp/go1.21.1/src/runtime/mfinal.go:163 +0x80
r0   0x0
r1   0x2
r2   0x4
r3   0x0
r4   0x0
r5   0x1
r6   0xf1f09ff972
r7   0x1e
r8   0x0
r9   0xc
r10  0x1e
r11  0x7ffb0b874308
r12  0x16b
r13  0xf1f09ff9d0
r14  0x20
r15  0x16c
r16  0x40000b13a0
r17  0x40000b7dd0
r18  0x0
r19  0x40e5f90000000000
r20  0xf1f09ff9b0
r21  0xf1f09ff818
r22  0x0
r23  0x0
r24  0x0
r25  0x0
r26  0x40000b7de0
r27  0x1488
r28  0x7ff784da0c20
r29  0xf1f09ff960
lr   0x7ffb0b780d20
sp   0xf1f09ff960
pc   0x7ffb0b780d30
cpsr 0x80000000
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 12, 2023
@qmuntal qmuntal added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 12, 2023
@qmuntal
Copy link
Contributor

qmuntal commented Sep 12, 2023

It looks like windows/arm64 doesn't set float parameters into the dedicated floating point registers, which produces an exception down the line.

For example, here is where the first cgo parameter is set:

MOVD (0*8)(R12), R0

But it should be something like this:

MOVD	(0*8)(R12), R0
FMOVD    R0, F0

Unfortunately, the fix is not that easy. AArch64 calling convention allocates registers for integers and floating points independently. I.e., in VariantTimeToSystemTime(arg0 float64, arg1 uintptr), arg0 will be stored in V0 and arg1 will be stored in R0, not in R1. This means that one needs to know the value class for each parameter in order to convert from Go calling convention to AArch64.

I wonder how is this implemented in other arm64 OSes. @golang/compiler

@qmuntal qmuntal changed the title runtime: crash calling proc on Windows ARM64 syscall: floating point parameters are not supported on windows/arm64 Sep 12, 2023
@qmuntal
Copy link
Contributor

qmuntal commented Sep 12, 2023

Also, worth noting that TestFloatArgs is skipped on windows/arm64:

func TestFloatArgs(t *testing.T) {
if _, err := exec.LookPath("gcc"); err != nil {
t.Skip("skipping test: gcc is missing")
}
if runtime.GOARCH != "amd64" {
t.Skipf("skipping test: GOARCH=%s", runtime.GOARCH)
}

Related to #6510.

@cherrymui
Copy link
Member

@qmuntal on other platforms we generally assume syscalls don't take floating point arguments. syscall.Syscall certainly assumes so. In the case that we do need floating point argument, it has a special code path, e.g. https://cs.opensource.google/go/go/+/master:src/runtime/sys_darwin.go;l=105 , which we call a macOS system function with a floating point arg. It has a different signature, and different assembly implementations.

@cherrymui
Copy link
Member

If floating point args are a general case for Windows syscalls, we probably need some other mechanism beyond syscall.Syscall. Maybe syscall.SyscallFP1, syscall.SyscallFP2, etc. for syscalls with 1, 2, ... FP args...

@mauri870
Copy link
Member

@cherrymui Since we have syscall.SyscallN can't we have syscall.SyscallFPN instead so it is future proof?

@cherrymui
Copy link
Member

I don't think we can have two variadic args, one for integer and one for floats. If there is a way to write such a variadic function and implement it, I'd be totally fine.

@qmuntal
Copy link
Contributor

qmuntal commented Sep 12, 2023

Hmm, what about this API which accept a mix of integer and floating point arguments:

// SyscallFPN is like SyscallN but for functions that accept floating point arguments.
// To flag an argument as floating point, set the corresponding bit in the floats bitmask.
// If floats is 1, all arguments are assumed to be floating point.
//
// For example, if the second and third argument are floating points, use
//
//	syscall.SyscallFPN(trap, 1<<2|1<<3, a0, uintptr(math.Float64bits(a1)), uintptr(math.Float64bits(a2)), a3)
func SyscallFPN(trap uintptr, floats byte, args ...uintptr) (r1, r2 uintptr, err Errno)

@aclements
Copy link
Member

Related issue, but for Windows C APIs calling back to Go: #45300

I'd be much more comfortable with a typed API, perhaps something along the lines of:

// Get assigns this Proc as a Go function to *f. f must be a pointer to a func-typed variable.
// f's arguments may have type uintptr or float64 and it may have up to two results,
// each of type uintptr or float64. (I made up these constraints, but something like that.)
func (p *Proc) Get(f any) error

This is essentially the inverse of how syscall.NewCallback works. I haven't thought through how this would be implemented. It would probably be tricky, and look something like a cross between NewCallback and reflect.makeMethodValue.

In general, I quite dislike the Windows syscall.Syscall* functions because they're just not a good match for how things work on Windows, especially with the way they overload the "trap" argument as a function PC. I'd rather not go even deeper down the rabbit hole of these functions if we can avoid it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
Development

No branches or pull requests

7 participants