x/sys/windows: mkwinsyscall: support syscalls with more than 15 params

[dagood]

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 x/sys/windows: mkwinsyscall: Support - and . in DLL name #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.

[ianlancetaylor]

Thanks. I don't think this has to be a proposal. It's just a bug fix.

[qmuntal]

For the record: as of today (go1.20), syscall.SyscallN is internally limited to 42 arguments, even though it accepts a variadic input.

go/src/runtime/syscall_windows.go

Lines 528 to 533 in 9894ded

	// maxArgs should be divisible by 2, as Windows stack
	// must be kept 16-byte aligned on syscall entry.
	//
	// Although it only permits maximum 42 parameters, it
	// is arguably large enough.
	const maxArgs = 42

[alexbrainman]

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

[sladyn98]

I could give this a shot @alexbrainman, would this fix just involve changing the number of params ?

[alexbrainman]

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

[dagood]

I'm not currently working on this. Thanks @sladyn98 for giving it a try. 🙂

[gopherbot]

Change https://go.dev/cl/518995 mentions this issue: windows: use SyscallN in mkwinsyscall

[gopherbot]

Change https://go.dev/cl/518857 mentions this issue: syscall: update windows zsyscalls to use SyscallN