Skip to content

syscall: SyscallN always escapes the variadic argument [1.22 backport] #70201

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.

Sign up for GitHub

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

Jump to bottom
Closed
gopherbot opened this issue Nov 5, 2024 · 6 comments
Closed

syscall: SyscallN always escapes the variadic argument [1.22 backport] #70201

gopherbot opened this issue Nov 5, 2024 · 6 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone
Go1.22.10

Comments

@gopherbot
Copy link
Contributor

@qmuntal requested issue #70197 to be considered for backport to the next 1.22 minor release.

@gopherbot please open the backport tracking issues. syscall.SyscallN alternatives are deprecated, and there is the risk of users switching back to them if syscall.SyscallN is less performant.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Nov 5, 2024
@gopherbot gopherbot mentioned this issue Nov 5, 2024
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 5, 2024
@gopherbot gopherbot added this to the Go1.22.9 milestone Nov 5, 2024
@randall77
Copy link
Contributor

I'm not sure this warrants a backport. Risk of someone switching to a deprecated API doesn't constitute "serious problem with no workaround".

@qmuntal
Copy link
Member

qmuntal commented Nov 6, 2024

The issue with the deprecated APIs (for example syscall.Syscall is that it is easy to set the wrong nargs parameter, in which case the behavior is completely undefined. If that happens, the syscall might succeed, it might do something unexpected or might throw an exception.

@dr2chase
Copy link
Contributor

dr2chase commented Nov 6, 2024

We decided not to backport because it isn't a regression, and the problem will be solved with the 1.24 release.

@dr2chase dr2chase closed this as completed Nov 6, 2024
@dr2chase
Copy link
Contributor

dr2chase commented Nov 6, 2024

@ianlancetaylor points out that this is a regression-in-waiting and blocks progress on x/sys/windows, so, reopen for consideration next week.

@dr2chase dr2chase reopened this Nov 6, 2024
@gopherbot gopherbot modified the milestones: Go1.22.9, Go1.22.10 Nov 6, 2024
@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Nov 20, 2024
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/630215 mentions this issue: [release-branch.go1.22]syscall: mark SyscallN as noescape

gopherbot pushed a commit that referenced this issue Nov 20, 2024
@qmuntal @gopherbot
[release-branch.go1.22] syscall: mark SyscallN as noescape
6f05fa7 
syscall.SyscallN is implemented by runtime.syscall_syscalln, which makes
sure that the variadic argument doesn't escape.

There is no need to worry about the lifetime of the elements of the
variadic argument, as the compiler will keep them live until the
function returns.

For #70197
Fixes #70201

Change-Id: I12991f0be12062eea68f2b103fa0a794c1b527eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/625297
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 7fff741)
Reviewed-on: https://go-review.googlesource.com/c/go/+/630215
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@dmitshur
Copy link
Contributor

Closed by merging CL 630215 (commit 6f05fa7) to release-branch.go1.22.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Milestone
Go1.22.10
Development

No branches or pull requests
5 participants
@dmitshur @dr2chase @qmuntal @randall77 @gopherbot