go
go copied to clipboard
x/sys/windows: mkwinsyscall: support syscalls with more than 15 params
Currently mkwinsyscall
includes a 15 arg limit with a panic
: https://github.com/golang/sys/blob/6e4d1c53cf3b2c1c6d104466749f76cc7a5c9ed8/windows/mkwinsyscall/mkwinsyscall.go#L554-L558
The limit was committed Oct 3, 2019, and since then, Go 1.18 deprecated the individual Syscall
, Syscall6
, etc. methods in favor of SyscallN
.
Changing mkwinsyscall
to use SyscallN
simplifies the code as well as widening support, unless I'm not aware of some caveat. 😄
Here's an example API with 17 params: https://learn.microsoft.com/en-us/windows/win32/api/fontsub/nf-fontsub-createfontpackage
Background
I'm trying to use mkwinsyscall
for x/sys/windows: use win32metadata? #43838 by having a generator create //sys
comments. When I generated //sys
lines for all the API methods for Windows.Win32.winmd
, I found a handful of methods with a few more params than the current limit:
CreateFontPackage
ICDrawBegin
DRMGetUsagePolicy
ScriptPlaceOpenType
D2D1GetGradientMeshInteriorPointsFromCoonsPatch
AccessCheckByTypeAndAuditAlarmW
AccessCheckByTypeResultListAndAuditAlarmW
AccessCheckByTypeResultListAndAuditAlarmByHandleW
AccessCheckByTypeAndAuditAlarmA
AccessCheckByTypeResultListAndAuditAlarmA
AccessCheckByTypeResultListAndAuditAlarmByHandleA
- Like I mentioned in https://github.com/golang/go/issues/57913, I'm not sure if it makes sense to use
mkwinsyscall
to generate a broad list of APIs like these, but I figure it's worth making room for the possibility by proposing this change.
Thanks. I don't think this has to be a proposal. It's just a bug fix.
For the record: as of today (go1.20), syscall.SyscallN is internally limited to 42 arguments, even though it accepts a variadic input.
https://github.com/golang/go/blob/9894ded19417fbc40420dc813d3c6606348ad31b/src/runtime/syscall_windows.go#L528-L533
Thank you @dagood for creating this issue.
I agree with Ian. It is just a bug in mkwinsyscall
program. We should fix it.
Perhaps we could even change mkwinsyscall
to stop using Syscall3 and friends and use SyscallN instead. If we do that, we should regenerate all existing code in the standard library and goloang.org/x/sys/windows/...
packages at the same time.
Alex
I could give this a shot @alexbrainman, would this fix just involve changing the number of params ?
I could give this a shot @alexbrainman, ...
Sure. Go ahead. Unless @dagood or others already started working on this change.
... would this fix just involve changing the number of params ?
Current version uses Syscall/Syscall6/Syscall9/Syscall12/Syscall15 syscalls.
You can add Syscall18 support, that will take care of https://learn.microsoft.com/en-us/windows/win32/api/fontsub/nf-fontsub-createfontpackage with 17 parameters that is listed above.
Or you could replace all Syscall/Syscall6/Syscall9/Syscall12/Syscall15 syscalls with new SyscallN. That will cover every call that Go syscall package supports. If you do that, you would have to regenerate all code that uses mkwinsyscall program in golang.org/x/sys/windows/... and in the standard library, so people who use mkwinsyscall next time are not surprised by large diffs that new version of mkwinsyscall will generate.
Alex
I'm not currently working on this. Thanks @sladyn98 for giving it a try. 🙂
Change https://go.dev/cl/518995 mentions this issue: windows: use SyscallN in mkwinsyscall
Change https://go.dev/cl/518857 mentions this issue: syscall: update windows zsyscalls to use SyscallN