Skip to content

Conversation

@qmuntal
Copy link
Contributor

@qmuntal qmuntal commented Oct 1, 2025

What issue is this addressing?

#343

What type of issue is this addressing?

bug

What this PR does | solves

Implement and linkname the following functions:

//go:linkname _cgo_libc_setegid syscall.cgo_libc_setegid
//go:linkname _cgo_libc_seteuid syscall.cgo_libc_seteuid
//go:linkname _cgo_libc_setgid syscall.cgo_libc_setgid
//go:linkname _cgo_libc_setregid syscall.cgo_libc_setregid
//go:linkname _cgo_libc_setresgid syscall.cgo_libc_setresgid
//go:linkname _cgo_libc_setresuid syscall.cgo_libc_setresuid
//go:linkname _cgo_libc_setreuid syscall.cgo_libc_setreuid
//go:linkname _cgo_libc_setuid syscall.cgo_libc_setuid
//go:linkname _cgo_libc_setgroups syscall.cgo_libc_setgroups

Note that almost all new code is autogenerated. The generator has been extended to support os-specific functions, which has added a bit of complexity. A simpler alternative would be to implement the new functions for all supported OSes, and only linkname them on Linux.

@hajimehoshi hajimehoshi requested a review from Copilot October 1, 2025 16:40
Copy link
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 implements setgid and related system call wrappers on Linux to fix issue #343. It provides purego implementations for 9 Linux-specific syscalls that mirror glibc functions with POSIX semantics when CGO is disabled.

  • Implements 9 new Linux-specific functions: setegid, seteuid, setgid, setregid, setresgid, setresuid, setreuid, setuid, and setgroups
  • Extends the code generator to support OS-specific functions with proper linkname integration
  • Adds comprehensive tests to verify the syscall wrappers work correctly

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
syscall_linux_test.go Test file verifying syscall wrappers return expected EPERM errors for non-root users
internal/fakecgo/symbols_linux.go Core implementation with syscall wrappers, trampolines, and linkname declarations
internal/fakecgo/linux.go Linux-specific linkname bindings to syscall package and errno handling
internal/fakecgo/fakecgo.go Common helper functions for argument handling moved from symbols.go
internal/fakecgo/gen.go Enhanced code generator supporting OS-specific functions and trampoline generation
internal/fakecgo/ztrampolines_*.s Generated assembly trampolines for amd64, arm64, and loong64 architectures
internal/fakecgo/symbols.go Removed duplicate function declarations now in fakecgo.go
README.md Updated documentation to reflect new linux.go file

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@hajimehoshi
Copy link
Member

TestSetuidEtc/Setregid seems to get stuck.

@hajimehoshi hajimehoshi marked this pull request as draft October 14, 2025 16:30
@qmuntal
Copy link
Contributor Author

qmuntal commented Oct 14, 2025

TestSetuidEtc/Setregid seems to get stuck.

It does. Unfortunately I can't reproduce the failure locally. Will fix it (sorry for the noise).

@hajimehoshi
Copy link
Member

It does. Unfortunately I can't reproduce the failure locally. Will fix it (sorry for the noise).

I think you can run the tests in your GitHub repository instead of here.

hajimehoshi pushed a commit that referenced this pull request Oct 16, 2025
…poline (#347)

`threadentry_trampoline` is called from glibc (that is, C code) when
creating a new thread. When the thread exists, `threadentry_trampoline`
return back to glibc, which can perform some additional cleanups.

We currently aren't honoring the Linux C ABI. It specify some registers
as calle-saved which are clobbered by the Go code. This can cause
unexpected behaviors when returning to glibc when the thread exists,
as the C compiler assumes some registers will have the same value as
before the `threadentry_trampoline` function was called, but they don't.

This is a precondition to merge #344.
Also, might fix #304.
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.

2 participants