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
Closed

syscall, runtime: New ABI separation breaks //go:linkname #28769

Helflym opened this issue Nov 13, 2018 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker

Comments

@Helflym
Copy link
Contributor

Helflym 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
Copy link
Member

aclements commented Nov 13, 2018 via email

@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Nov 13, 2018
@aclements
Copy link
Member

@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
Copy link
Contributor Author

Helflym commented Nov 14, 2018

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

@gopherbot
Copy link

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

@Helflym
Copy link
Contributor Author

Helflym 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
Copy link

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
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>
@golang golang locked and limited conversation to collaborators Dec 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

4 participants