go icon indicating copy to clipboard operation
go copied to clipboard

runtime: clean up system call code

Open prattmic opened this issue 2 years ago • 24 comments

The code used to make system calls from the runtime is a mess and a big pain point when making OS-level changes in the runtime.

Quick summary of some of the pain points:

  • Completely independent implementation from package syscall.
  • System call stubs exist in a variety of OS- and arch-specific sys* files (57, to be precise, mostly assembly), totaling ~23k lines of code [1], which is 100% hand-written. Adding a new syscall stub often requires adding new assembly to 10+ files.
  • There is no generic Syscall function that can be used to make an arbitrary system call. You must add a new stub to make a new call.

Some things we could do to improve the situation:

  • Add generic Syscall functions, used by other stubs. And/or use code generation to generate stubs rather than doing so manually.
  • Move syscall code to a new package (like runtime/internal/syscall or perhaps runtime/internal/linux?) to avoid littering the runtime package itself with so many files.
  • Make package syscall depend on the runtime's Syscall stub so we only have a single implementation.

[1] Some of this code is things other than syscall stubs, like thread entrypoints.

cc @aclements @mknyszek @cherrymui

prattmic avatar Feb 08 '22 22:02 prattmic

And/or use code generation to generate stubs rather than doing so manually.

All or most of the information we would need to generate the stubs is already in the syscall package. The needs of the syscall package are of course a bit different, but the metadata should be effectively the same.

Orthogonal to code generation, given a generic runtime Syscall function (or a few of them for different numbers of arguments), would there even be any overhead to having the stubs be in Go rather than assembly? In addition to significantly reducing the amount of assembly, I think this would encourage us to do better error handling.

aclements avatar Feb 08 '22 22:02 aclements

Just to add another pain point: the stubs handle errors differently. Some returns errno (negated or not?), some just crash hard (writing to an invalid address), which is difficult to debug sometimes.

cherrymui avatar Feb 08 '22 22:02 cherrymui

Change https://go.dev/cl/383999 mentions this issue: runtime/internal/syscall: new package for linux

gopherbot avatar Feb 08 '22 22:02 gopherbot

Seems this introduce build error, i got: gofrontend commit 45fd14ab8baf5e86012a808426f8ef52c1d77943 image

ii64 avatar Feb 23 '22 11:02 ii64

@ii64 this issue is about a plan for the future, not what has happened. Your comment is unrelated to this issue.

Looks like you are using gollvm. What version of gollvm are you using? Try updating to the latest. If that still fail, please file a new issue. Thanks.

cherrymui avatar Feb 23 '22 14:02 cherrymui

Change https://go.dev/cl/388094 mentions this issue: test: rename live_syscall.go to live_uintptrkeepalive.go

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/388477 mentions this issue: syscall: define Syscall in terms of RawSyscall

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/388475 mentions this issue: runtime/internal/syscall, syscall: replace RawSyscall6 with runtime implementation

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/388478 mentions this issue: syscall: define Syscall6 in terms of RawSyscall6

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/388095 mentions this issue: cmd/compile: add //go:uintptrkeepalive

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/388476 mentions this issue: syscall: define RawSyscall in terms of RawSyscall6

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/388479 mentions this issue: syscall: implement rawSyscallNoError in terms of RawSyscall

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/388474 mentions this issue: syscall: move Syscall declarations to OS files

gopherbot avatar Feb 28 '22 19:02 gopherbot

Change https://go.dev/cl/401096 mentions this issue: runtime/internal/syscall: use ABIInternal for Syscall6 on amd64

gopherbot avatar Apr 19 '22 20:04 gopherbot

Change https://go.dev/cl/401337 mentions this issue: DO NOT SUBMIT: WIP: support inlining with //go:uintptrkeepalive

gopherbot avatar Apr 20 '22 18:04 gopherbot

Change https://go.dev/cl/401636 mentions this issue: Revert "syscall: define Syscall in terms of RawSyscall on linux"

gopherbot avatar Apr 21 '22 18:04 gopherbot

Change https://go.dev/cl/401634 mentions this issue: Revert "runtime/internal/syscall: use ABIInternal for Syscall6 on amd64"

gopherbot avatar Apr 21 '22 18:04 gopherbot

Change https://go.dev/cl/401635 mentions this issue: Revert "syscall: define Syscall6 in terms of RawSyscall6 on linux"

gopherbot avatar Apr 21 '22 18:04 gopherbot

Change https://go.dev/cl/401654 mentions this issue: syscall: define Syscall in terms of RawSyscall on linux

gopherbot avatar Apr 21 '22 20:04 gopherbot

Change https://go.dev/cl/401656 mentions this issue: runtime/internal/syscall: use ABIInternal for Syscall6 on amd64

gopherbot avatar Apr 21 '22 20:04 gopherbot

Change https://go.dev/cl/401655 mentions this issue: syscall: define Syscall6 in terms of RawSyscall6 on linux

gopherbot avatar Apr 21 '22 20:04 gopherbot

Everything I expect to get into 1.19 is in. Moving future work to 1.20.

prattmic avatar Apr 22 '22 19:04 prattmic

Change https://go.dev/cl/402914 mentions this issue: syscall: add //go:norace to RawSyscall

gopherbot avatar Apr 28 '22 15:04 gopherbot

Change https://go.dev/cl/421994 mentions this issue: runtime: move epoll syscalls to runtime/internal/syscall

gopherbot avatar Aug 08 '22 13:08 gopherbot

Change https://go.dev/cl/437295 mentions this issue: Revert "runtime: move epoll syscalls to runtime/internal/syscall"

gopherbot avatar Sep 30 '22 18:09 gopherbot

Change https://go.dev/cl/440115 mentions this issue: runtime: move epoll syscalls to runtime/internal/syscall

gopherbot avatar Oct 07 '22 10:10 gopherbot