Skip to content
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: add Mmap for Solaris and Illumos? #52875

Closed
bcmills opened this issue May 12, 2022 · 5 comments
Closed

syscall: add Mmap for Solaris and Illumos? #52875

bcmills opened this issue May 12, 2022 · 5 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. OS-illumos OS-Solaris
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented May 12, 2022

Solaris supports an mmap syscall.

x/sys/unix.Mmap is defined for GOOS=solaris, and runtime.mmap appears to use the system call as well.

And yet, even though Mmap exists on Solaris and syscall.Mmap exists on most Go Unix ports, for some reason syscall.Mmap does not appear to be defined for GOOS=solaris or GOOS=illumos.

We're looking at possibly using syscall.Mmap in cmd/go in https://go.dev/cl/403975 (CC @matloob). Would it make sense to add syscall.Mmap, syscall.Munmap, and related constants for Solaris and Illumos for consistency with other platforms?

(CC @ianlancetaylor @tklauser @golang/solaris @golang/illumos)

@bcmills bcmills added OS-Solaris NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 12, 2022
@bcmills bcmills added this to the Backlog milestone May 12, 2022
@ianlancetaylor
Copy link
Contributor

Although the syscall package is frozen, I think that this kind of addition is OK since it matches other targets.

@tklauser
Copy link
Member

tklauser commented May 13, 2022

I too think this is OK.

There have been similar additions in the past when a particular function was present in syscall for some, but not all platforms. See e.g. https://go.dev/cl/403394, https://go.dev/cl/391434, https://go.dev/cl/390714 for some recent examples where this was the case for solaris and illumos.

@gopherbot
Copy link

Change https://go.dev/cl/413374 mentions this issue: syscall: add Mmap and Munmap on solaris

@gopherbot
Copy link

Change https://go.dev/cl/413375 mentions this issue: cmd/go/internal/mmap: use syscall.Mmap on solaris

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@tklauser tklauser removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 28, 2022
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
They exist on all other Unix ports, define them on GOOS=solaris as well.

Fixes golang#52875

Change-Id: I7285156b3b48ce12fbcc6d1d88865540a5c51a21
Reviewed-on: https://go-review.googlesource.com/c/go/+/413374
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Aug 16, 2022
Now that syscall.Mmap is defined on solaris (see CL 413374), use it in
mmapFile like on other Unix ports.

For #52875

Change-Id: Ic5c5a84da8613f0c6dc947a52b7fcca50af43d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/413375
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/430496 mentions this issue: cmd, syscall: use syscall.Mmap on solaris for Go ≥ 1.20

gopherbot pushed a commit that referenced this issue Sep 15, 2022
CL 413374 added syscall.Mmap on solaris. Use it in cmd/compile and
cmd/link if the bootstrap toolchain is Go ≥ 1.20.

For #52875
For #54265

Change-Id: I9a0534bf97926eecf0c6f1f9218e855344ba158f
Reviewed-on: https://go-review.googlesource.com/c/go/+/430496
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Jun 23, 2023
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. OS-illumos OS-Solaris
Projects
None yet
Development

No branches or pull requests

5 participants