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

Jump to bottom

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
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Go1.15

Comments

@richardwilkes
Copy link
Contributor

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 NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Feb 18, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.15 milestone Feb 18, 2020
@ianlancetaylor
Copy link
Contributor

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

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

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 commented Feb 18, 2020 •
edited

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

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

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

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

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

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.

richardwilkes added a commit to richardwilkes/go that referenced this issue Feb 22, 2020
@richardwilkes
runtime: allow float syscall return values on windows amd64
efd63af 
Fixes golang#37273
@richardwilkes richardwilkes mentioned this issue Feb 22, 2020
Closed
@gopherbot
Copy link

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

@alexbrainman
Copy link
Member

... 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
@richardwilkes
runtime: allow float syscall return values on windows amd64
bbe30ba 
RELNOTE=yes
Fixes golang#37273
@gopherbot
Copy link

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
@aclements
syscall: document float arguments and results on windows/amd64
07ced37 
Updates #6510.
Updates #37273.

Change-Id: Id2732fcff0a0c5e4a324cd33ef995c7e528f5e1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/236562
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@emarj emarj mentioned this issue Mar 26, 2021
Open
@golang golang locked and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
Go1.15
Development

Successfully merging a pull request may close this issue.

runtime: allow float syscall return values on windows amd64 richardwilkes/go
5 participants
@networkimprov @ianlancetaylor @gopherbot @alexbrainman @richardwilkes