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: can't call Windows function that returns float #37273

Closed
richardwilkes opened this issue Feb 17, 2020 · 12 comments
Closed

syscall: can't call Windows function that returns float #37273

richardwilkes opened this issue Feb 17, 2020 · 12 comments

Comments

@richardwilkes
Copy link
Contributor

@richardwilkes richardwilkes commented Feb 17, 2020

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

$ go version
go version go1.13.8 darwin/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="/Users/rich/Library/Caches/go-build"
GOENV="/Users/rich/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY="jaxf-github.fanatics.corp/"
GONOSUMDB="jaxf-github.fanatics.corp/"
GOOS="darwin"
GOPATH="/Users/rich/go"
GOPRIVATE="jaxf-github.fanatics.corp/"
GOPROXY="direct"
GOROOT="/usr/local/Cellar/go/1.13.8/libexec"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.8/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/s9/9s3fk2mx5f936dkcp66ltwsc0000gn/T/go-build181499683=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

This is essentially a request for #3588 to be solved rather than just closed out and ignored.

Currently, it is not possible to retrieve a floating point value returned by a syscall. As noted in the original report, that's not very common in the old Windows APIs, but it does occur in many of the newer APIs, especially around graphics. My particular case is the Direct2D and DirectWrite APIs, although I've seen the need elsewhere as well.

I've looked, and it seems to be fairly straight-forward to add this support via a new set of functions (SyscallFloat, perhaps) -- however, it currently seems impossible to do so without doing it inside the runtime and syscall packages, which would require a fork, since there is various internal state that an outside package cannot access.

If we can't have the additional call(s) created for some reason, can we instead at least expose enough of the internals that an external module could provide the functionality?

I'd be happy to create the necessary PR to make either scenario work.

One thing to note: I realize I could get around this by using cgo. However, as most would likely agree, using cgo on the Windows platform is a nightmare. It's generally quite tolerable on platforms like macOS and linux, where the compiler toolchain is easily installed, but Windows makes that difficult and I've found getting downstream developers to be able to get their Windows machine setup properly for cgo to require a lot of hand-holding, unlike with other platforms, essentially making it a non-starter.

@ianlancetaylor ianlancetaylor changed the title syscall incapable of returning a float value syscall: can't call Windows function that returns float Feb 18, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.15 milestone Feb 18, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 18, 2020

It would help if somebody suggested specific new API that would work for all known cases. It seems to me that just adding syscall.SyscallFloat may not be enough. What about syscall.(*Proc).Call and syscall.(*LazyProc).Call?

@richardwilkes
Copy link
Contributor Author

@richardwilkes richardwilkes commented Feb 18, 2020

Good point. I had not yet encountered a case where I needed this in my own projects, but that should be considered as well. I’ll try to come up with a list and post it here.

@networkimprov
Copy link

@networkimprov networkimprov commented Feb 18, 2020

Are the APIs you reference unimplemented in x/sys/windows?

See also discussion in #6510.

cc @alexbrainman @zx2c4 @mattn

@richardwilkes
Copy link
Contributor Author

@richardwilkes richardwilkes commented Feb 18, 2020

Looking through both package syscall and golang.org/x/sys/windows, it appears these are the APIs that need a "Float" variant for the return:

syscall.Syscall -> syscall.SyscallFloat
syscall.Syscall6 -> syscall.SyscallFloat6
syscall.Syscall9 -> syscall.SyscallFloat9
syscall.Syscall12 -> syscall.SyscallFloat12
syscall.Syscall15 -> syscall.SyscallFloat15
syscall.Syscall18 -> syscall.SyscallFloat18
syscall.(*Proc).Call -> syscall.(*Proc).CallFloat
syscall.(*LazyProc).Call -> syscall.(*LazyProc).CallFloat

windows.(*Proc).Call -> windows.(*Proc).CallFloat
windows.(*LazyProc).Call -> windows.(*LazyProc).CallFloat

runtime·asmstdcall -> runtime·asmstdcallfloat <-- this is where a one-line change is needed to return the float register (X0, I believe)

Regarding @networkimprov 's question: golang.org/x/sys/windows doesn't implement the syscall code itself -- it just calls through to the implementation in syscall.

@richardwilkes
Copy link
Contributor Author

@richardwilkes richardwilkes commented Feb 18, 2020

Another possibility, which would require only a single code change in function runtime·asmstdcall in runtime/sys_windows_amd64.s and no visible API changes: make use of the currently-unused r2 result by always copying the X0 register into it on return, much like is currently done for the first 4 potential floating point arguments. This value is used on 32-bit systems, but I don't think the float problem exists there and no change is needed in runtime/sys_windows_386.s.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 19, 2020

Do Windows function that accept floating point arguments work correctly?

Is there a need to fix this in the syscall package, which is more or less frozen, or can we just fix it in golang.org/x/sys/windows?

@richardwilkes
Copy link
Contributor Author

@richardwilkes richardwilkes commented Feb 19, 2020

In my usage of them, yes, floating point args do work since the fix to fill the first four registers was put in. I found I do have to pass them by calling the conversion function in the math package, but that’s workable.

As stated earlier, the fix has to be in the runtime package, as that’s where the info is and the plumbing is private to that package so can’t be fixed externally.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 20, 2020

My particular case is the Direct2D and DirectWrite APIs, although I've seen the need elsewhere as well.

Which API are you talking about?

Another possibility, which would require only a single code change in function runtime·asmstdcall in runtime/sys_windows_amd64.s and no visible API changes: make use of the currently-unused r2 result by always copying the X0 register into it on return, much like is currently done for the first 4 potential floating point arguments.

That sounds reasonable. Is X0 register documented by Microsoft to receive returned value on amd64?

This value is used on 32-bit systems, but I don't think the float problem exists there and no change is needed in runtime/sys_windows_386.s.

Fair enough.

Alex

@richardwilkes
Copy link
Contributor Author

@richardwilkes richardwilkes commented Feb 21, 2020

My particular case is the Direct2D and DirectWrite APIs, although I've seen the need elsewhere as well.

Which API are you talking about?

As stated above, both the Direct2D and DirectWrite APIs are the ones I'm currently using. Did you want a specific function call? If so, here's one off the top of my head: https://docs.microsoft.com/en-us/windows/win32/api/d2d1/nf-d2d1-id2d1brush-getopacity
I'm not sure it matters, though -- this is basic functionality that should be possible, even if official Microsoft APIs didn't have FLOAT return args.

Another possibility, which would require only a single code change in function runtime·asmstdcall in runtime/sys_windows_amd64.s and no visible API changes: make use of the currently-unused r2 result by always copying the X0 register into it on return, much like is currently done for the first 4 potential floating point arguments.

That sounds reasonable. Is X0 register documented by Microsoft to receive returned value on amd64?

Yes. From here: https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2019:
A scalar return value that can fit into 64 bits is returned through RAX; this includes __m64 types. Non-scalar types including floats, doubles, and vector types such as __m128, __m128i, __m128d are returned in XMM0. The state of unused bits in the value returned in RAX or XMM0 is undefined.

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 22, 2020

Change https://golang.org/cl/220578 mentions this issue: runtime: allow float syscall return values on windows amd64

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 24, 2020

... Did you want a specific function call?

I did. I was not aware of its existence.

If so, here's one off the top of my head: https://docs.microsoft.com/en-us/windows/win32/api/d2d1/nf-d2d1-id2d1brush-getopacity

Sounds good. Thank you.

I'm not sure it matters, though -- this is basic functionality that should be possible, even if official Microsoft APIs didn't have FLOAT return args.

No argument.

... From here: https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2019:
A scalar return value that can fit into 64 bits is returned through RAX; this includes __m64 types. Non-scalar types including floats, doubles, and vector types such as __m128, __m128i, __m128d are returned in XMM0. The state of unused bits in the value returned in RAX or XMM0 is undefined.

Looks good. Thank you.

Alex

richardwilkes added a commit to richardwilkes/go that referenced this issue Feb 24, 2020
@gopherbot gopherbot closed this in 450d0b2 Feb 25, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 4, 2020

Change https://golang.org/cl/236562 mentions this issue: syscall: document float arguments and results on windows/amd64

gopherbot pushed a commit that referenced this issue Jun 4, 2020
Updates #6510.
Updates #37273.

Change-Id: Id2732fcff0a0c5e4a324cd33ef995c7e528f5e1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/236562
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.

5 participants
You can’t perform that action at this time.