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

mmap_unix.go missing BSD in Go build tag #564

Closed
vendion opened this issue Dec 13, 2023 · 4 comments · Fixed by #567
Closed

mmap_unix.go missing BSD in Go build tag #564

vendion opened this issue Dec 13, 2023 · 4 comments · Fixed by #567

Comments

@vendion
Copy link

vendion commented Dec 13, 2023

https://github.com/bytedance/sonic/blob/8c71eb047516a6b70f216887923f353f42d0501b/loader/mmap_unix.go#L1C1-L2C23

Would it be possible to make the loader/mmap_unix.go file target more Unix based OSes than just Darwin and Linux? I'm trying to build a project that indirectly depends on this package and I get the following build error.

# github.com/bytedance/sonic/loader
../../go/pkg/mod/github.com/bytedance/sonic@v1.10.2/loader/funcdata_latest.go:228:13: undefined: mmap
../../go/pkg/mod/github.com/bytedance/sonic@v1.10.2/loader/funcdata_latest.go:233:5: undefined: mprotect
@AsterDY
Copy link
Collaborator

AsterDY commented Dec 14, 2023

didn't try before. Are you interested in giving a PR?

@vendion
Copy link
Author

vendion commented Dec 24, 2023

Well it is a very simple change as the syscalls for the BSDs should match those for Darwin (based on BSD anyways) & Linux so something like this should be more than enough:

diff --git a/loader/mmap_unix.go b/loader/mmap_unix.go
index 3ea944e..1d15e54 100644
--- a/loader/mmap_unix.go
+++ b/loader/mmap_unix.go
@@ -1,5 +1,5 @@
-//go:build darwin || linux
-// +build darwin linux
+//go:build !windows
+// +build !windows

 /**
  * Copyright 2023 ByteDance Inc.

Unless you have reasons for being explict in this case.

@vendion
Copy link
Author

vendion commented Dec 24, 2023

I didn't submit a PR, as it's a one line change and that the formatting of the file doesn't follow the standard gofmt standard so the whole flie would end up being needlessly modified.

diff --git a/loader/mmap_unix.go b/loader/mmap_unix.go
index 3ea944e..1d15e54 100644
--- a/loader/mmap_unix.go
+++ b/loader/mmap_unix.go
@@ -1,5 +1,5 @@
-//go:build darwin || linux
-// +build darwin linux
+//go:build !windows
+// +build !windows

 /**
  * Copyright 2023 ByteDance Inc.
@@ -20,26 +20,25 @@
 package loader

 import (
-    `syscall`
+	"syscall"
 )

 const (
-    _AP = syscall.MAP_ANON  | syscall.MAP_PRIVATE
-    _RX = syscall.PROT_READ | syscall.PROT_EXEC
-    _RW = syscall.PROT_READ | syscall.PROT_WRITE
+	_AP = syscall.MAP_ANON | syscall.MAP_PRIVATE
+	_RX = syscall.PROT_READ | syscall.PROT_EXEC
+	_RW = syscall.PROT_READ | syscall.PROT_WRITE
 )

-
 func mmap(nb int) uintptr {
-    if m, _, e := syscall.RawSyscall6(syscall.SYS_MMAP, 0, uintptr(nb), _RW, _AP, 0, 0); e != 0 {
-        panic(e)
-    } else {
-        return m
-    }
+	if m, _, e := syscall.RawSyscall6(syscall.SYS_MMAP, 0, uintptr(nb), _RW, _AP, 0, 0); e != 0 {
+		panic(e)
+	} else {
+		return m
+	}
 }

 func mprotect(p uintptr, nb int) {
-    if _, _, err := syscall.RawSyscall(syscall.SYS_MPROTECT, p, uintptr(nb), _RX); err != 0 {
-        panic(err)
-    }
+	if _, _, err := syscall.RawSyscall(syscall.SYS_MPROTECT, p, uintptr(nb), _RX); err != 0 {
+		panic(err)
+	}
 }

@AsterDY
Copy link
Collaborator

AsterDY commented Dec 25, 2023

ok,I will submit it

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 a pull request may close this issue.

2 participants