Skip to content

purego: Support 32 arguments on 64bit#431

Merged
hajimehoshi merged 21 commits intoebitengine:mainfrom
tmc:support-32args-on-64bit
Apr 30, 2026
Merged

purego: Support 32 arguments on 64bit#431
hajimehoshi merged 21 commits intoebitengine:mainfrom
tmc:support-32args-on-64bit

Conversation

@tmc
Copy link
Copy Markdown
Contributor

@tmc tmc commented Mar 5, 2026

What issue is this addressing?

Closes #425

What type of issue is this addressing?

feature

What this PR does | solves

This increases the # of supported arguments to 32.

@tmc tmc changed the title Support 32args on 64bit purego: Support 32 arguments on 64bit Mar 5, 2026
@hajimehoshi hajimehoshi requested a review from Copilot March 5, 2026 07:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR expands PureGo’s supported argument count for 64-bit targets to 32 (aligning with existing 32-bit limits) by widening the syscall argument container, updating per-arch syscall assembly to spill additional stack arguments, and adding ABI tests to exercise higher-arity calls.

Changes:

  • Increase SyscallN max supported arguments to 32 on supported 64-bit Unix-like targets (Windows remains capped at 15).
  • Extend per-architecture syscall assembly stubs to pass a16a32 (and adjust stack frame sizing accordingly).
  • Add new ABI test functions/tests for 20/32 uintptr args and a 32-arg mixed int/float case.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
syscall.go Raises maxArgs to 32 for selected 64-bit targets and routes non-Windows calls through runtime_cgocall using a pooled args struct.
syscall_32bit.go Changes 32-bit SyscallN to use pooled args + runtime_cgocall.
syscall_64bit_fallback.go Adds a fallback implementation (still maxArgs=15) for 64-bit Unix-like targets outside the explicitly supported arch list.
syscall_sysv.go Adjusts callback argument decoding to account for ppc64le stack slot progression with float register args.
sys_amd64.s Expands stack spill area and spills a16a32 for amd64 SysV.
sys_arm64.s Expands stack spill area and spills a16a32 for arm64.
sys_loong64.s Expands stack spill area and spills a16a32 for loong64.
sys_riscv64.s Expands stack spill area and spills a16a32 for riscv64.
sys_s390x.s Expands stack spill area and spills a16a32 for s390x.
sys_ppc64le.s Expands stack spill area and now loads additional float regs + spills a16a32 for ppc64le.
func.go Updates RegisterFunc marshaling/validation to use cached float-reg counts and adds a ppc64le-specific stack index adjustment.
testdata/abitest/abi_test.c Adds C helpers to validate 20/32-argument passing and a 32-arg mixed int/float case.
func_test.go Adds/updates ABI tests for 20/32 argument cases and improves panic assertions.
Comments suppressed due to low confidence (1)

sys_amd64.s:45

  • In syscall15X on amd64 the args pointer is stored and later loaded via PTR_ADDRESS(BP), but the stack frame is created by subtracting STACK_SIZE from SP while leaving BP pointing above the allocated frame. This causes the pointer to be written outside the allocated STACK_SIZE region into the caller’s stack frame, corrupting the caller’s stack and potentially enabling crashes or code execution if an attacker can trigger calls through this path. Ensure the args pointer is saved within the allocated frame (e.g., based off SP or a negative offset from BP) so all accesses stay inside the STACK_SIZE region.
#define STACK_SIZE 224
#define PTR_ADDRESS (STACK_SIZE - 8)

// syscall15X calls a function in libc on behalf of the syscall package.
// syscall15X takes a pointer to a struct like:
// struct {
//	fn    uintptr
//	a1    uintptr
//	a2    uintptr
//	a3    uintptr
//	a4    uintptr
//	a5    uintptr
//	a6    uintptr
//	a7    uintptr
//	a8    uintptr
//	a9    uintptr
//	a10    uintptr
//	a11    uintptr
//	a12    uintptr
//	a13    uintptr
//	a14    uintptr
//	a15    uintptr
//	r1    uintptr
//	r2    uintptr
//	err   uintptr
// }
// syscall15X must be called on the g0 stack with the
// C calling convention (use libcCall).
GLOBL ·syscall15XABI0(SB), NOPTR|RODATA, $8
DATA ·syscall15XABI0(SB)/8, $syscall15X(SB)
TEXT syscall15X(SB), NOSPLIT|NOFRAME, $0
	PUSHQ BP
	MOVQ  SP, BP
	SUBQ  $STACK_SIZE, SP
	MOVQ  DI, PTR_ADDRESS(BP) // save the pointer

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread syscall_32bit.go Outdated
Comment thread sys_ppc64le.s Outdated
Comment thread func.go
Comment thread func.go Outdated
Comment thread syscall_64bit_fallback.go Outdated
Comment thread cdecl.go Outdated
Comment thread syscall_64bit_fallback.go Outdated
Comment thread syscall.go Outdated
Comment thread syscall_32bit.go
@tmc
Copy link
Copy Markdown
Contributor Author

tmc commented Mar 5, 2026

Addressing copilot's comment.

@hajimehoshi
Copy link
Copy Markdown
Member

This PR requires rebasing

@tmc tmc force-pushed the support-32args-on-64bit branch from 38df5f1 to 5884add Compare March 12, 2026 22:50
@tmc
Copy link
Copy Markdown
Contributor Author

tmc commented Mar 12, 2026

This PR requires rebasing

rebased.

Comment thread syscall_64bit_fallback.go
@@ -0,0 +1,76 @@
// SPDX-License-Identifier: Apache-2.0
Copy link
Copy Markdown
Member

@hajimehoshi hajimehoshi Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this file is for an OS and an architecture PureGo does not support. Is this correct?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not answered yet

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why you have not answered this...

Copy link
Copy Markdown
Member

@hajimehoshi hajimehoshi Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this file no longer exists and instead you put syscall_64bit.go, but could you explain the purposes of the files?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry missed this comment -- it was originally basically unused but compilable code. since then the syscalln was put into -> syscall_64bit.go and syscall_32bit.go but i'm guessing you want to keep the 64bit code in syscall.go, which i'll do now.

@tmc tmc force-pushed the support-32args-on-64bit branch from 5884add to 7a4926d Compare March 15, 2026 00:56
@hajimehoshi
Copy link
Copy Markdown
Member

The test is failing now

Comment thread syscall_stackargs_ppc64le.go Outdated
Comment thread syscall_stackargs_s390x.go Outdated
Comment thread syscall_64bit.go Outdated
Comment thread func.go Outdated
Comment thread syscall_ppc64le.go Outdated
Comment thread syscall_64bit.go Outdated
@hajimehoshi
Copy link
Copy Markdown
Member

I'm waiting for an answer at #431 (comment)

@tmc
Copy link
Copy Markdown
Contributor Author

tmc commented Mar 19, 2026

IIUC, this file is for an OS and an architecture PureGo does not support. Is this correct?

Missed that, addressing now and will resolve that conflict.

@tmc tmc force-pushed the support-32args-on-64bit branch 2 times, most recently from 16b470a to dcc75eb Compare March 19, 2026 02:53
@tmc
Copy link
Copy Markdown
Contributor Author

tmc commented Mar 19, 2026

Moved syscall_64bit.go to (back to) syscall.go and rebase to main.

Comment thread syscall.go Outdated
@tmc tmc force-pushed the support-32args-on-64bit branch from b9722f3 to dcc75eb Compare March 19, 2026 17:56
@hajimehoshi
Copy link
Copy Markdown
Member

#431 (comment)

@tmc
Copy link
Copy Markdown
Contributor Author

tmc commented Mar 20, 2026

#431 (comment)

responded above

@tmc
Copy link
Copy Markdown
Contributor Author

tmc commented Apr 20, 2026

opened #444

@hajimehoshi
Copy link
Copy Markdown
Member

You might need to rebase or merge the main branch.

tmc added 18 commits April 20, 2026 19:18
The build constraints excluded every supported architecture, so this
file only compiled for unsupported platforms like mips64 where purego
does not work anyway. Removing it addresses the review comment about
consolidating syscall15Args definitions.
- Rename syscall15Args to syscallArgs, syscall15X to syscallX,
  syscall15XABI0 to syscallXABI0 throughout Go and assembly files
- Remove unused syscall_syscall15X functions from syscall_unix.go,
  syscall_windows.go, and syscall_cgo_linux.go
- Remove unnecessary float fields f9-f13 from 64-bit syscallArgs
  struct (only f1-f8 are used by 64-bit assembly)
- Replace Set method with syscall_SyscallN function that handles
  pool acquisition, struct population, and cgocall in one step
- Simplify func.go call site to use syscall_SyscallN directly
reformats code to avoid failure
@tmc tmc force-pushed the support-32args-on-64bit branch from 6122b32 to 5185f3f Compare April 21, 2026 02:19
@tmc
Copy link
Copy Markdown
Contributor Author

tmc commented Apr 21, 2026

rebased to upstream/main

Comment thread internal/cgo/syscall_cgo_unix.go
tmc added 2 commits April 26, 2026 12:56
Match the Go-side syscallArgs layout so runtime_cgocall passes a
struct the C side can read correctly. Errno is written to a3 to
match the asm convention; the Go caller reads s.a1, s.a2, s.a3.
The cgo path is invoked via runtime_cgocall(syscallXABI0, ...)
directly against C.syscall15, so the SyscallX Go wrapper has had
no callers since b9c5f0a. The syscall_syscall15X helpers in
syscall_unix.go and syscall_windows.go were deleted in that commit
per review feedback; this completes the same cleanup for
internal/cgo.
Comment thread func_test.go
@TotallyGamerJet
Copy link
Copy Markdown
Collaborator

LGTM! @hajimehoshi

@hajimehoshi hajimehoshi merged commit fa49e74 into ebitengine:main Apr 30, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support 32 arguments on 64bit

4 participants