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

cmd/link: fails to link package having a .syso file when using external linker #33139

Closed
jpap opened this issue Jul 16, 2019 · 7 comments
Closed

cmd/link: fails to link package having a .syso file when using external linker #33139

jpap opened this issue Jul 16, 2019 · 7 comments
Milestone

Comments

@jpap
Copy link
Contributor

@jpap jpap commented Jul 16, 2019

Being able to inclue a .syso file in a package is great. A Go program using that package works well when that program is linked using the internal linker. Link is however broken when the that program is linked using the external linker, for example, because the Go program also uses cgo.

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

$ go version
go version go1.12.7 linux/amd64

Does this issue reproduce with the latest release?

This affects all go versions, including tip (at the time of submission).

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

This affects all GOOS/GOARCH pairs. I have tried it on darwin/amd64 and linux/amd64.

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jpap/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jpap/go"
GOPROXY=""
GORACE=""
GOROOT="/home/jpap/Local/go"
GOTMPDIR=""
GOTOOLDIR="/home/jpap/Local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build378647343=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. Create a package that has a .syso file.
  2. Create a main package that uses the package above.
  3. Link the main package in external link mode to build an executable. (This could be explicit, using -ldflags='-linkmode=external', or by using cgo in the main package, where the external linker is then required.)

Sample project follows...

Directory structure

$GOPATH/src/jpap.org/linktest
├── cmd
│   ├── external
│   │   └── main.go
│   └── internal
│       └── main.go
└── syso
    ├── asm
    │   ├── Makefile
    │   ├── test_darwin.s
    │   └── test_linux.s
    ├── test.go
    ├── test.s
    └── test.syso

To build the test.syso file on Linux using gcc,

$ cd $GOPATH/src/jpap.org/linktest/syso/asm
$ make linux
gcc -c -o ../test.syso test_linux.s
$

syso/asm/Makefile

linux:
	gcc -c -o ../test.syso test_linux.s
darwin:
	gcc -c -o ../test.syso test_darwin.s

syso/asm/test_darwin.s

	.globl _asmTest
_asmTest:
	ret

syso/asm/test_linux.s

	.globl asmTest
asmTest:
	ret

syso/test.go

package syso

func Test()

syso/test.s

#include "textflag.h"

TEXT ·Test(SB), NOSPLIT, $0
	JMP asmTest(SB)

cmd/internal/main.go

package main

import (
	"jpap.org/linktest/syso"
)

func main() {
	syso.Test()
}

cmd/external/main.go


/*
 // Force use of external linker
 #include <stdio.h>
 */
import "C"

import (
	"jpap.org/linktest/syso"
)

func main() {
	syso.Test()
}

What did you expect to see?

  1. Link successful, and executable built for both cmd/internal/main.go (using internal link) and cmd/external/main.go (using external link).

What did you see instead?

  1. Internal link succeeds for cmd/internal/main.go.
  2. External link fails for cmd/external/main.go with error relocation target XXX not defined for each referenced global symbol in the imported package .syso, for example:
$ cd $GOPATH/src/jpap.org/cmd/external
$ go build .
# jpap.org/linktest/cmd/external
jpap.org/linktest/syso.Test: relocation target asmTest not defined
$ 
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jul 16, 2019

Change https://golang.org/cl/186417 mentions this issue: cmd/link: load symbols from .syso in external link mode

@cherrymui

This comment has been minimized.

Copy link
Contributor

@cherrymui cherrymui commented Jul 17, 2019

Emmm, if this fails, I wonder why cmd/go/testdata/script/cgo_syso_issue29253.txt works...

@jpap

This comment has been minimized.

Copy link
Contributor Author

@jpap jpap commented Jul 17, 2019

#29253 is different because of Cgo and the extern declaration of function f(void):

  1. Cgo wraps the call to f in __cgo_2a8cbb76e5a7_Cfunc_f via a relocation (see below) that works because f is declared extern in the import "C" preamble.
  2. The above Cgo wrapper (in a host object file), the pkg/o.syso (host) object file, and the Go object code for the pkg package are combined into a Go object file. The symbol table for the Go object file does not contain the f symbol because Go doesn't directly call f; it is invoked via the Cgo wrapper __cgo_2a8cbb76e5a7_Cfunc_f. Go's cmd/link therefore does not try to verify look up the f symbol while preparing to call the external linker. We therefore don't hit the same path as #33139.
  3. The external linker, when called by cmd/link, is passed the pkg/o.syso object file, as well as the Cgo wrapper object file, which it can use to relocate f by itself.

Relocations for package pkg build archive (filtered):

RELOCATION RECORDS FOR [__text]:
000000000000001d X86_64_RELOC_BRANCH __cgo_topofstack
0000000000000016 X86_64_RELOC_BRANCH _f
000000000000000e X86_64_RELOC_BRANCH __cgo_topofstack

Dissassembly for Cgo wrapper before relocations (from within the Go object file cited above), where you can clearly see the callq targets are missing (zero):

__cgo_2a8cbb76e5a7_Cfunc_f:
       0:       55      pushq   %rbp
       1:       48 89 e5        movq    %rsp, %rbp
       4:       41 57   pushq   %r15
       6:       41 56   pushq   %r14
       8:       53      pushq   %rbx
       9:       50      pushq   %rax
       a:       49 89 fe        movq    %rdi, %r14
       d:       e8 00 00 00 00  callq   0 <__cgo_2a8cbb76e5a7_Cfunc_f+0x12>
      12:       49 89 c7        movq    %rax, %r15
      15:       e8 00 00 00 00  callq   0 <__cgo_2a8cbb76e5a7_Cfunc_f+0x1a>
      1a:       89 c3   movl    %eax, %ebx
      1c:       e8 00 00 00 00  callq   0 <__cgo_2a8cbb76e5a7_Cfunc_f+0x21>
      21:       4c 29 f8        subq    %r15, %rax
      24:       41 89 1c 06     movl    %ebx, (%r14,%rax)
      28:       48 83 c4 08     addq    $8, %rsp
      2c:       5b      popq    %rbx
      2d:       41 5e   popq    %r14
      2f:       41 5f   popq    %r15
      31:       5d      popq    %rbp
      32:       c3      retq

Disassembly of final executable, after relocations (filtered):

__cgo_2a8cbb76e5a7_Cfunc_f:
 4051160:       55      pushq   %rbp
 4051161:       48 89 e5        movq    %rsp, %rbp
 4051164:       41 57   pushq   %r15
 4051166:       41 56   pushq   %r14
 4051168:       53      pushq   %rbx
 4051169:       50      pushq   %rax
 405116a:       49 89 fe        movq    %rdi, %r14
 405116d:       e8 1e 93 ff ff  callq   -27874 <__cgo_topofstack>
 4051172:       49 89 c7        movq    %rax, %r15
 4051175:       e8 26 00 00 00  callq   38 <_f>
 405117a:       89 c3   movl    %eax, %ebx
 405117c:       e8 0f 93 ff ff  callq   -27889 <__cgo_topofstack>
 4051181:       4c 29 f8        subq    %r15, %rax
 4051184:       41 89 1c 06     movl    %ebx, (%r14,%rax)
 4051188:       48 83 c4 08     addq    $8, %rsp
 405118c:       5b      popq    %rbx
 405118d:       41 5e   popq    %r14
 405118f:       41 5f   popq    %r15
 4051191:       5d      popq    %rbp
 4051192:       c3      retq

_f:
 40511a0:       55      pushq   %rbp
 40511a1:       48 89 e5        movq    %rsp, %rbp
 40511a4:       b8 2a 00 00 00  movl    $42, %eax
 40511a9:       5d      popq    %rbp
 40511aa:       c3      retq
@ALTree ALTree added this to the Go1.14 milestone Jul 17, 2019
@cherrymui

This comment has been minimized.

Copy link
Contributor

@cherrymui cherrymui commented Jul 17, 2019

Thanks. The difference is that in cmd/go/testdata/script/cgo_syso_issue29253.txt, the symbol in the syso is only referenced from C code, not Go code (including Go assembly). Here the symbol in the syso is directly called from Go assembly.

In general, calling external functions directly from Go (including Go assembly) is discouraged. But I'm not sure we want to reject it here, given that internal linking works. But it is also possible that this is not a "supported" feature so we don't fix.

As a workaround, you can force internal linking by passing -ldflags=-linkmode=internal.

@jpap

This comment has been minimized.

Copy link
Contributor Author

@jpap jpap commented Jul 17, 2019

In general, calling external functions directly from Go (including Go assembly) is discouraged. But I'm not sure we want to reject it here, given that internal linking works. But it is also possible that this is not a "supported" feature so we don't fix.

This kind of direct access is not only advertised on the Go Wiki since 2014, but is an extraordinarily useful feature for executing performance-critical code that is not written in Go, and where the overhead of using cgo is to be avoided where possible.

Yes -- internal linking works, but so does external linking with a syso file in the main package (not discussed above). It is only when a syso file is included in an imported package that the external link fails (this issue, #33139). I therefore see this issue as a bug worthy of a fix and I am also supporting a PR #33140 to address it.

As a workaround, you can force internal linking by passing -ldflags=-linkmode=internal.

Sadly that is not a valid workaround:

  • When using cgo, external linking is mandatory. That's why I wrote the sample project using cgo as I did above, to illustrate that there is no workaround. If you know another way, other than to just use cgo and accept the overhead, I'd be keen to try it!
  • As a 3rd party package author, I might want to include a syso host object file into my project, calling it via an assembler stub similar to the one above: if a user needs to import the package, and they're using cgo (previous point), their build is irreparably broken.
@gopherbot gopherbot closed this in 5e514b7 Oct 1, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 1, 2019

Change https://golang.org/cl/198177 mentions this issue: Revert "cmd/link: load symbols from .syso in external link mode"

@andybons andybons reopened this Oct 1, 2019
gopherbot pushed a commit that referenced this issue Oct 1, 2019
This reverts CL 186417.

Reason for revert: Broke darwin (10_14), linux (ppc), aix (ppc)

Updates #33139

Change-Id: I8bf3c817a96a0e57e45754a097cea7062b2fcdfd
Reviewed-on: https://go-review.googlesource.com/c/go/+/198177
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Oct 3, 2019

Change https://golang.org/cl/198798 mentions this issue: cmd/link: pass-through undefined call targets in external link mode

@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot gopherbot closed this in 59a6847 Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.