golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.37k stars 17.71k forks source link

syscall: Syscall6 not implemented on Solaris #24357

Open gen2brain opened 6 years ago

gen2brain commented 6 years ago

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64" GOBIN="" GOCACHE="/home/milann/.cache/go-build" GOEXE="" GOHOSTARCH="amd64" GOHOSTOS="linux" GOOS="linux" GOPATH="/home/milann/golang" GORACE="" GOROOT="/home/milann/go" GOTMPDIR="" GOTOOLDIR="/home/milann/go/pkg/tool/linux_amd64" GCCGO="gccgo" CC="gcc" CXX="g++" CGO_ENABLED="1" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build031994401=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I am trying to call shmget, shmat etc. via syscall. On OpenSolaris there is one code for SHM (52) and then shmget etc. functions are sub calls with codes 0,1,2 etc.

If possible, provide a recipe for reproducing the error. A complete runnable program is good. A link on play.golang.org is best.

package main

import (
    "runtime"
    "syscall"
)

const (
    IPC_PRIVATE = 0
    IPC_CREAT   = 01000
)

func main() {
    if runtime.GOOS == "solaris" {
        id, _, errno := syscall.Syscall6(52, 3, uintptr(int32(IPC_PRIVATE)), uintptr(int32(65536)), uintptr(int32(IPC_CREAT|0777)), 0, 0)
        if int(id) == -1 {
            println(errno)
        }
    } else if runtime.GOOS == "linux" {
                // 29 is syscall.SYS_SHMGET on amd64, missing on solaris
        id, _, errno := syscall.Syscall(29, uintptr(int32(IPC_PRIVATE)), uintptr(int32(65536)), uintptr(int32(IPC_CREAT|0777)))
        if int(id) == -1 {
            println(errno)
        }
    }
}

What did you expect to see?

Program compiles ok with GOOS=solaris.

What did you see instead?

GOOS=solaris go build test.go 
# command-line-arguments
syscall.Syscall6: call to external function
main.main: relocation target syscall.Syscall6 not defined
main.main: undefined: "syscall.Syscall6"

Is this something you are aware of and is there a workaround maybe? For now I also implemented cgo version but I hope just temporary. You can also test with library I am working on: GOOS=solaris go get -tags syscall github.com/gen2brain/shm

andybons commented 6 years ago

@ianlancetaylor

ianlancetaylor commented 6 years ago

Someone just needs to implement syscall.Syscall6 for Solaris. syscall.Syscall already exists, but for some reason Syscall6 is missing. RawSyscall6 is also missing. And they are also missing from the x/sys/unix package.

tklauser commented 6 years ago

/cc @binarycrusader @4ad

I think syscalls are a bit different on Solaris and shouldn't be used directly. The comment above runtime.syscall_syscall (which is called by syscall.Syscall) says:

https://github.com/golang/go/blob/d32018a50017b075cd46be6b1f5cfb5050337140/src/runtime/syscall_solaris.go#L238-L243

Also, syscall.RawSyscall calls runtime.syscall_rawsyscall on Solaris which just panics (see #20833), so I guess we shouldn't add RawSyscall6 (or just an implementation that panics as well).

So I think the proper solution here would be to implement Shmget in x/sys/unix and then use that.

4ad commented 6 years ago

syscall.Syscall is safe to use, that comment is obsolete. It predates the use of libc by the Solaris port. It's illegal to issue system calls directly, but we're just calling libc. syscall.RawSyscall however is not supported.

However, if you want Shmget, you need to implement it in x/sys/unix and use that, though SYS_semsys should work too.

ianlancetaylor commented 6 years ago

I think we should an implementation of RawSyscall6 that panics. The function is supposed to exist even if it is not called. It ought to be possible to write code as in the original issue that links even if RawSyscall6 is never called, by run time checks of GOOS (even if it would probably be better to use build tags).

4ad commented 6 years ago

Why do we need syscall.RawSyscall6 for Solaris? The standard library doesn't need it. Why needlessly add it?

I agree however that the "6" variants need to be added to x/sys/unix. The original code should use x/sys/unix instead of syscall. I don't see why it's desired to normalize the symbols exported by syscall across operating systems, even though they don't work on every operating system.

As posted, the code would fail to compile on Plan 9, nacl, and possibly Windows too (unsure). That's ok, the author should be using build tags. Why is Solaris different than Plan 9, nacl or Windows?

ianlancetaylor commented 6 years ago

The problem now is that the syscall package currently declares RawSyscall6 but does not define it. So it's already there. Code that calls it will compile, but will not link. In fact, worse, it will compile but whether it links or not depends on what compiler optimizations are applied.

Had RawSyscall6 never been declared for Solaris, then I think it would be fine to continue to leave it out. But the current situation seems bad, and I think that defining that function so that links succeed would be the better fix.

It's not a big deal, though.

4ad commented 6 years ago

You're right, it's declared by mistake. This is all my fault.

When I wrote the Solaris port I intended the syscall package to only support the standard library, unlike the other Unix ports which export all kinds of functionality. Evidently, I failed.

In this light I think defining all the declared, but undefined functions and making them panic is a good idea.

ianlancetaylor commented 6 years ago

Don't beat yourself up, it's a very easy mistake to make the way the code is written currently.

gopherbot commented 6 years ago

Change https://golang.org/cl/100655 mentions this issue: runtime, syscall: add RawSyscall6 on Solaris and make it panic

tklauser commented 6 years ago

Thanks @4ad and @ianlancetaylor for the clarification. I sent https://golang.org/cl/100655 which adds a panicing RawSyscall6 and removes the outdated comment above runtime.syscall_syscall.

gen2brain commented 6 years ago

Thanks. So I should use for now cgo on solaris, if implemented, functions will be in x/sys/unix right?

And what about freebsd, it doesn't have syscall.SYS_SHMGET and other related consts defined, I defined them in my lib but I somehow expected to find this defined in all unixes. Should that be in standard library?

4ad commented 6 years ago

Until the XSI shared memory functions are added to x/sys/unix you can use cgo, yes. But you should consider adding these functions to x/sys/unix. It's relatively straightforward.

As for FreeBSD, and any other Unix system, these functions should also go in x/sys/unix. All low-level syscall-like interfaces should go there. It's unfortunate we expose so much in syscall, but there's too late to change anything now. The syscall package predates internal Go packages by many years.

gen2brain commented 6 years ago

Ok, for functions I understand, but I meant syscall.SYS_SHMGET is defined for example on dragonfly, netbsd and openbsd but is missing on freebsd, is just that one allowed to be added to standard library. It does work when defined, solaris is the only one that works totally differently, others just have different structs.

tklauser commented 6 years ago

The syscall package is frozen and no new public API should be added (except for some special casses where it is required by Go itself) as per:

https://github.com/golang/go/blob/f0939ba5b18aea6649bf405dd6571030915afd56/src/syscall/syscall.go#L21-L28

Instead, the golang.org/x/sys package should be used instead of syscall by all code outside the standard Go library. New functions, constants (such as SYS_SHMGET on FreeBSD) etc. will be added and kept updated there.

4ad commented 6 years ago

@gen2brain It's totally fine to define some constant that exists only on some platform, though in this case note that you could just define generic shmget(2)-like functions that just use the underlying mechanism (shmget(2) on Solaris, SYS_SHMGET on some other platforms, etc), instead of exposing the lower-level mechanism itself. That way the calling code could stay portable and not care about the differences between platforms.

tklauser commented 6 years ago

Sorry, somehow I misread Ian's comment at https://github.com/golang/go/issues/24357#issuecomment-372833790 and didn't add syscall.Syscall6 in https://golang.org/cl/100655. I'll send a follow-up CL, adding it as well and fixing the actual bug that was reported.

This will then also allow to add Syscall and Syscall6 to x/sys/unix, calling their respective counterparts in syscall.

gopherbot commented 6 years ago

Change https://golang.org/cl/101135 mentions this issue: runtime, syscall: add Syscall6 on Solaris