New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syscall: audit missing syscalls on linux/arm64 #10235

Closed
minux opened this Issue Mar 24, 2015 · 18 comments

Comments

Projects
None yet
10 participants
@minux
Member

minux commented Mar 24, 2015

Despite our efforts, it seems there are still syscalls in the syscall
package that are missing on linux/arm64.

Getpgrp is one.

@minux minux added the repo-main label Mar 24, 2015

@minux minux added this to the Go1.5 milestone Mar 24, 2015

@rsc rsc removed the repo-main label Apr 14, 2015

@rsc rsc modified the milestones: Go1.6Early, Go1.5 Jul 21, 2015

@rsc rsc modified the milestones: Unplanned, Go1.6Early Nov 4, 2015

@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Nov 30, 2015

At least the following deprecated syscalls still need wrappers for arm64:
epoll_create -> epoll_create1
epoll_wait -> epoll_pwait
futimesat -> utimensat
getpgrp -> getpgid
pause -> ?
time -> gettimeofday?
ustat -> statfs
utime -> utimes

There are also some syscalls in the unimplemented table that need to be taken care when implemented:

evenfd -> eventfd2
signalfd -> signalfd4
poll -> ppoll

suihkulokki commented Nov 30, 2015

At least the following deprecated syscalls still need wrappers for arm64:
epoll_create -> epoll_create1
epoll_wait -> epoll_pwait
futimesat -> utimensat
getpgrp -> getpgid
pause -> ?
time -> gettimeofday?
ustat -> statfs
utime -> utimes

There are also some syscalls in the unimplemented table that need to be taken care when implemented:

evenfd -> eventfd2
signalfd -> signalfd4
poll -> ppoll

@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Nov 30, 2015

looking at glibc, pause is implemented as
sigset_t set;
rt_sigprocmask(SIG_BLOCK, NULL, &set, 8) = 0
rt_sigsuspend(&set, 8

and time is implemented as gettimeofday from the vdso. - which is how go seems to do Time on amd64 as well.

suihkulokki commented Nov 30, 2015

looking at glibc, pause is implemented as
sigset_t set;
rt_sigprocmask(SIG_BLOCK, NULL, &set, 8) = 0
rt_sigsuspend(&set, 8

and time is implemented as gettimeofday from the vdso. - which is how go seems to do Time on amd64 as well.

@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Dec 9, 2015

And dup2 -> dup3 wrapper needed (see #11981)

suihkulokki commented Dec 9, 2015

And dup2 -> dup3 wrapper needed (see #11981)

@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Jan 5, 2016

Any way to make this less unplanned? I keep bumping into go packages that naively call syscalls not being aware they have been deprecated and are not available on arm64..

cc @davecheney and @4ad

suihkulokki commented Jan 5, 2016

Any way to make this less unplanned? I keep bumping into go packages that naively call syscalls not being aware they have been deprecated and are not available on arm64..

cc @davecheney and @4ad

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Jan 5, 2016

Member

@suihkulokki, which? Those packagess are buggy, then. Using syscall is like using unsafe... you have to really know what you're doing. They probably don't run well on Windows or Plan 9 or Solaris either.

Member

bradfitz commented Jan 5, 2016

@suihkulokki, which? Those packagess are buggy, then. Using syscall is like using unsafe... you have to really know what you're doing. They probably don't run well on Windows or Plan 9 or Solaris either.

@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Jan 8, 2016

@bradfitz This is typically linux-specific code, such as some of the libraries used by docker (example: libcontainer) or code where linux implementation is in a different file then windows one (fsnotify). Both examples have now fixes commited, but I expect that more are being created as we talk. Developers are not aware that bunch of syscalls go provides are actually deprecated ones, because they work just fine on amd64. My faith that people who use the syscall package know what they are doing is not very strong...

suihkulokki commented Jan 8, 2016

@bradfitz This is typically linux-specific code, such as some of the libraries used by docker (example: libcontainer) or code where linux implementation is in a different file then windows one (fsnotify). Both examples have now fixes commited, but I expect that more are being created as we talk. Developers are not aware that bunch of syscalls go provides are actually deprecated ones, because they work just fine on amd64. My faith that people who use the syscall package know what they are doing is not very strong...

@4ad

This comment has been minimized.

Show comment
Hide comment
@4ad

4ad Jan 8, 2016

Member

My faith that people who use the syscall package know what they are doing is not very strong

People that use the syscall package (and unsafe, as already mentioned) should know what they are doing.

It's not clear to me why we should cater to poorly-written programs, thereby decreasing their incentive to move to the go.sys package even more.

Member

4ad commented Jan 8, 2016

My faith that people who use the syscall package know what they are doing is not very strong

People that use the syscall package (and unsafe, as already mentioned) should know what they are doing.

It's not clear to me why we should cater to poorly-written programs, thereby decreasing their incentive to move to the go.sys package even more.

@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Jan 8, 2016

It's not clear to me why we should cater to poorly-written programs, thereby decreasing their incentive to move to the go.sys package even more.

Ok, I didn't know about go.sys. However, the same problems are there too. People who need Epoll will use EpollCreate() instead of EpollCreate1() because nothing hints that the former is deprecated. We can:

  1. whenever a arm64 user compiles the affected code, send a patch upstream to change it
  • this is the current somewhat inconvenient state
    2) add a EpollCreate -> EpollCreate1 wrapper to go.sys
  • this is the "cater poorly written programs choice"
    3) remove legacy syscalls like EpollCreate from go.sys
  • this breaks backward compatibility

I would assume 2, is preferred choice, since the current arm64 has a bunch already, example:

https://github.com/golang/sys/blob/master/unix/syscall_linux_arm64.go#L132

suihkulokki commented Jan 8, 2016

It's not clear to me why we should cater to poorly-written programs, thereby decreasing their incentive to move to the go.sys package even more.

Ok, I didn't know about go.sys. However, the same problems are there too. People who need Epoll will use EpollCreate() instead of EpollCreate1() because nothing hints that the former is deprecated. We can:

  1. whenever a arm64 user compiles the affected code, send a patch upstream to change it
  • this is the current somewhat inconvenient state
    2) add a EpollCreate -> EpollCreate1 wrapper to go.sys
  • this is the "cater poorly written programs choice"
    3) remove legacy syscalls like EpollCreate from go.sys
  • this breaks backward compatibility

I would assume 2, is preferred choice, since the current arm64 has a bunch already, example:

https://github.com/golang/sys/blob/master/unix/syscall_linux_arm64.go#L132

@anthonyfok

This comment has been minimized.

Show comment
Hide comment
@anthonyfok

anthonyfok Jan 13, 2016

But what about the unimplemented syscall.EpollPwait() when syscall.EpollWait() is supposedly deprecated? Unless I am mistaken, there is currently no way out for ARM64: syscall.EpollWait() gives ENOSYS: function not implemented error (from Linux kernel), but EpollPwait is not yet implemented in Go.

This currently affects fsnotify, which in turn affects Hugo and others, see go-fsnotify/fsnotify#112 and gohugoio/hugo#1772

@bradfitz, I see that you dealt with a similar issue for the Perl Sys::Syscall package at bradfitz/sys-syscall#6, so you can't simply dismiss that that Linux-specific code requiring syscall.EpollPwait() as buggy, right? 😉

Should this problem be addressed here, or over at golang.org/x/sys? Many thanks!

anthonyfok commented Jan 13, 2016

But what about the unimplemented syscall.EpollPwait() when syscall.EpollWait() is supposedly deprecated? Unless I am mistaken, there is currently no way out for ARM64: syscall.EpollWait() gives ENOSYS: function not implemented error (from Linux kernel), but EpollPwait is not yet implemented in Go.

This currently affects fsnotify, which in turn affects Hugo and others, see go-fsnotify/fsnotify#112 and gohugoio/hugo#1772

@bradfitz, I see that you dealt with a similar issue for the Perl Sys::Syscall package at bradfitz/sys-syscall#6, so you can't simply dismiss that that Linux-specific code requiring syscall.EpollPwait() as buggy, right? 😉

Should this problem be addressed here, or over at golang.org/x/sys? Many thanks!

@minux

This comment has been minimized.

Show comment
Hide comment
@minux

minux Jan 13, 2016

Member
Member

minux commented Jan 13, 2016

@anthonyfok

This comment has been minimized.

Show comment
Hide comment
@anthonyfok

anthonyfok Jan 14, 2016

Thank you @minux!

(对不起,昨天有眼不识泰山,一下子没认出来!)感谢 Minux 大神! 🙏

anthonyfok commented Jan 14, 2016

Thank you @minux!

(对不起,昨天有眼不识泰山,一下子没认出来!)感谢 Minux 大神! 🙏

@luxas

This comment has been minimized.

Show comment
Hide comment
@luxas

luxas Jan 16, 2016

Kubernetes is affected by this also as I'm adding support for arm64.
Quick question: Is syscall.Dup2 equalient to syscall.Dup3 on arm64?
It would be great if you added some more wrappers for arm64...

luxas commented Jan 16, 2016

Kubernetes is affected by this also as I'm adding support for arm64.
Quick question: Is syscall.Dup2 equalient to syscall.Dup3 on arm64?
It would be great if you added some more wrappers for arm64...

@4ad

This comment has been minimized.

Show comment
Hide comment
@4ad

4ad Jan 16, 2016

Member

Quick question: Is syscall.Dup2 equalient to syscall.Dup3 on arm64?

This has nothing to do with Go, it has to do with Linux: http://linux.die.net/man/2/dup3

Member

4ad commented Jan 16, 2016

Quick question: Is syscall.Dup2 equalient to syscall.Dup3 on arm64?

This has nothing to do with Go, it has to do with Linux: http://linux.die.net/man/2/dup3

@mildred

This comment has been minimized.

Show comment
Hide comment
@mildred

mildred Jan 27, 2016

I would say https://golang.org/src/syscall/syscall_linux.go#L933 contains a good list of unimplemented syscalls. In particular I'm missing the syscalls about extended attributes on symlinks:

  • lgetxattr
  • llistxattr
  • lremovexattr
  • lsetxattr

mildred commented Jan 27, 2016

I would say https://golang.org/src/syscall/syscall_linux.go#L933 contains a good list of unimplemented syscalls. In particular I'm missing the syscalls about extended attributes on symlinks:

  • lgetxattr
  • llistxattr
  • lremovexattr
  • lsetxattr
@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot commented Mar 4, 2016

CL https://golang.org/cl/20178 mentions this issue.

gopherbot pushed a commit to golang/sys that referenced this issue Mar 15, 2016

x/sys/unix: add Dup2 wrapper for arm64/ppc64
arm64 doesn't have a Dup2 syscall, instead Dup3 is supposed
to be used. Since Dup3 is linux-specific, provide a wrapper
to make writing portable code easier.

    Updates golang/go#10235

To verify it, added a testcase for Dup and Dup2.

Change-Id: I066bb60d62b2bd64d7ba0fdfbb334ce2213c78e9
Reviewed-on: https://go-review.googlesource.com/20178
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Riku Voipio <riku.voipio@linaro.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Apr 18, 2016

I've hopefully implemented missing ones for arm64 in x/sys/unix . What was left is Ustat, which I think is too obscure to bother wrapping. Use Statfs instead.

suihkulokki commented Apr 18, 2016

I've hopefully implemented missing ones for arm64 in x/sys/unix . What was left is Ustat, which I think is too obscure to bother wrapping. Use Statfs instead.

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Apr 18, 2016

Contributor

@suihkulokki Thanks! Closing this issue on the assumption that the work is done.

Contributor

ianlancetaylor commented Apr 18, 2016

@suihkulokki Thanks! Closing this issue on the assumption that the work is done.

@suihkulokki

This comment has been minimized.

Show comment
Hide comment
@suihkulokki

suihkulokki Apr 19, 2016

Thanks, feel free to @ me if new issues with arm/arm64 syscalls appear.

suihkulokki commented Apr 19, 2016

Thanks, feel free to @ me if new issues with arm/arm64 syscalls appear.

@golang golang locked and limited conversation to collaborators Apr 19, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.