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, runtime: New ABI separation breaks //go:linkname #28769

Closed
Helflym opened this issue Nov 13, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@Helflym
Copy link
Contributor

commented Nov 13, 2018

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

go version devel +e092e96d4e Tue Nov 13 13:47:34 2018 -0600 aix/ppc64
after 685aca4 + few AIX commits not yet pushed.

Does this issue reproduce with the latest release?

No

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

aix/ppc64

What did you do?

The new ABI separation seems to break relocations linked with some //go:linkname between syscall functions and their runtime implementation:

root@castor4:/opt/freeware/src/packages/BUILD/go-build/src(cgo)$ ./make.bash 
Building Go cmd/dist using /opt/freeware/lib/golang.
Building Go toolchain1 using /opt/freeware/lib/golang.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
# cmd/link
internal/syscall/unix.syscall6: relocation target syscall.syscall6 not defined for ABI0 (but is defined for ABIInternal)
go tool dist: FAILED: /opt/freeware/src/packages/BUILD/go-build/pkg/tool/aix_ppc64/go_bootstrap install -gcflags=all= -ldflags=all= -i cmd/asm cmd/cgo cmd/compile cmd/link: exit status 2

Here is what triggers the fault:
internal/syscall/unix/asm_aix_ppc64.s:

TEXT ·syscall6(SB),NOSPLIT,$0
	JMP	syscall·syscall6(SB)

syscall/syscall_aix.go:

// Implemented in runtime/syscall_aix.go.
func rawSyscall6(trap, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno)
func syscall6(trap, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno)

runtime/syscall_aix.go:

//go:linkname syscall_syscall6 syscall.syscall6
func syscall_syscall6(fn, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2, err uintptr) { ... }
//go:linkname syscall_rawSyscall6 syscall.rawSyscall6
func syscall_rawSyscall6(fn, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2, err uintptr) { ... }

Note that creating a JMP in syscall/asm_aix_ppc64.s instead of a //go:linkname fixes it:

TEXT ·syscall6(SB),NOSPLIT,$0
	JMP	runtime·syscall_syscall6(SB)

Is it indented to avoid ABI bypass or something like this ? Or just a bug ?

@aclements

@aclements

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

@aclements

This comment has been minimized.

Copy link
Member

commented Nov 13, 2018

@Helflym, could you try this patch? If this works, I'll mail a CL. Thanks!

diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go
index 89ef2da8cb..a360a1f620 100644
--- a/src/cmd/go/internal/work/gc.go
+++ b/src/cmd/go/internal/work/gc.go
@@ -297,7 +297,7 @@ func (gcToolchain) symabis(b *Builder, a *Action, sfiles []string) (string, erro
        if p.ImportPath == "runtime" {
                // Assembly in syscall and runtime/cgo references
                // symbols in runtime.
-               otherPkgs = []string{"syscall", "runtime/cgo"}
+               otherPkgs = []string{"syscall", "internal/syscall", "runtime/cgo"}
        } else if p.ImportPath == "runtime/internal/atomic" {
                // sync/atomic is an assembly wrapper around
                // runtime/internal/atomic.
@Helflym

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

It needs to be "internal/syscall/unix". Otherwise, it doesn't work.

@gopherbot

This comment has been minimized.

Copy link

commented Nov 15, 2018

Change https://golang.org/cl/149817 mentions this issue: cmd/go: more cross-package references from internal/syscall/unix

@gopherbot gopherbot closed this in c6d4939 Nov 23, 2018

@Helflym

This comment has been minimized.

Copy link
Contributor Author

commented Dec 13, 2018

@aclements I've the same problem with x/sys/unix which has a jmp to syscall.rawSyscall6.

# golang.org/x/sys/unix.test
golang.org/x/sys/unix.rawSyscall6: relocation target syscall.rawSyscall6 not defined for ABI0 (but is defined for ABIInternal)
FAIL    golang.org/x/sys/unix [build failed]

Do you want to add it as another cross-package ? Or do I need to remove the linkname which will remove further problems like this one ?

Edit: I've submitted https://go-review.googlesource.com/c/go/+/153997 which should remove definitely this issue, as syscall.Syscall6 might be called from any packages.

@gopherbot

This comment has been minimized.

Copy link

commented Dec 13, 2018

Change https://golang.org/cl/153997 mentions this issue: syscall: remove linknames to runtime symbols for aix/ppc64

gopherbot pushed a commit that referenced this issue Dec 14, 2018

syscall: remove linknames to runtime symbols for aix/ppc64
Replaces //go:linkname by assembly functions for syscall
functions on aix/ppc64.
Since the new runtime internal ABI, this was triggering an error if
syscall.Syscall6 was called by others packages like x/sys/unix.
This commit should remove every future occurences of this problem.

Fixes #28769

Change-Id: I6a4bf77472ee1e974bdb76b27e74275e568f5a76
Reviewed-on: https://go-review.googlesource.com/c/153997
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.