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/compile: -buildmode=c-archive produces code not suitable for use in a shared object on arm64 #62556

Closed
nmeum opened this issue Sep 10, 2023 · 15 comments
Assignees
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@nmeum
Copy link

nmeum commented Sep 10, 2023

tl;dr On aarch64, Go 1.21 seems to emit code that is not suitable for use in a C/C++ shared library with -buildmode=c-archive on aarch64. I believe this to be a Go 1.21 regression as this was explicitly mentioned as a supported feature in commit d92a360. The root cause seems to be a R_AARCH64_ADR_PREL_PG_HI21 relocation for the crosscall2 symbol.

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

$ go version
go version go1.21.1 linux/arm64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/home/soeren/.cache/go-build'
GOENV='/home/soeren/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS='-buildmode=pie -modcacherw -trimpath -buildvcs=false'
GOHOSTARCH='arm64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/soeren/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/soeren/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_arm64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2174769471=/tmp/go-build -gno-record-gcc-switches

What did you do?

I maintain the Go package for Alpine Linux (which uses musl libc) with the release of Go 1.21.1 we rebuild all of our packaged Go software for the CVE fixes in the standard library. While doing so, we noticed that the package fcitx5-bamboo failed to build from source on aarch64 (on all other architecture it build fine). The error message was:

/usr/lib/gcc/aarch64-alpine-linux-musl/13.1.1/../../../../aarch64-alpine-linux-musl/bin/ld: bamboo/bamboo-core.a(go.o): relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `crosscall2' which may bind externally can not be used when making a shared object; recompile with -fPIC
bamboo/bamboo-core.a(go.o): in function `runtime/cgo.set_crosscall2.abi0':
go.go:(.text+0x8f228): dangerous relocation: unsupported relocation
collect2: error: ld returned 1 exit status
ninja: subcommand failed

The package previously compiled fine with Go 1.20.6 on aarch64.

Closer investigation revealed that the fcitx5-bamboo build system does the following:

  1. It compiles a few Go source files to a C archive (bamboo-core.a) using go -buildmode=c-archive.
  2. It compiles some C++ code and links it together with the c-archive, emitted by the Go compiler in the previous step, to a shared library using g++ -shared.

Naturally, the error message above is emitted by g++ not by the Go compiler. However, as per a prior discussion on golang-dev and commit d92a360, I believe that files produced by go build -buildmode=c-archive are supposed to be suitable for use in a shared library which the file emitted here (bamboo-core.a) is not. If my assumption is correct, then I believe this to be a regression introduced in Go 1.21, i.e. a bug in the Go compiler. The issue in this regard seems to be a R_AARCH64_ADR_PREL_PG_HI21 relocation emitted for crosscall2:

$ readelf -W -a bamboo/bamboo-core.a | grep ' crosscall2' | grep R_AARCH64_ADR_PREL_PG_HI21
000000000008f228  00000c0000000113 R_AARCH64_ADR_PREL_PG_HI21 000000000008f240 crosscall2 + 0

I am not familiar enough with aarch64 internals and relocations in general in order to determine why this code requires a R_AARCH64_ADR_PREL_PG_HI21 relocation and why this relocation is not suitable for use in a shared library.

To reproduce this run the following commands on aarch64:

$ git clone https://github.com/fcitx/fcitx5-bamboo
$ cd fcitx5-bamboo
$ git checkout 56cd63fac51a1da334903b06875d71a6ce0971de
$ git submodule update --init
$ cd bamboo/
$ go build -o bamboo-core.a -buildmode=c-archive *.go
$ readelf -W -a bamboo-core.a | grep ' crosscall2' | grep R_AARCH64_ADR_PREL_PG_HI21
000000000008f228  00000c0000000113 R_AARCH64_ADR_PREL_PG_HI21 000000000008f240 crosscall2 + 0

This should print the R_AARCH64_ADR_PREL_PG_HI21 relocation, or at least it does on Alpine Linux Edge aarch64. Again, I am not sure why this relocation is not suitable for use in a shared library. If you want to re-produce the g++ error message follow the upstream build instructions. Unfortunately, I did not manage to come up with a minimal example yet.

Full build log for our failing aarch64 Alpine Linux Edge build: fcitx5-bamboo-go-1.21.1-alpine-linux-edge-aarch64.txt

What did you expect to see?

Successful linking of the c-archive emitted by go build -buildmode=c-archive to a shared library with g++ -shared.

What did you see instead?

The g++ error message regarding a R_AARCH64_ADR_PREL_PG_HI21 relocation.

algitbot pushed a commit to alpinelinux/aports that referenced this issue Sep 10, 2023
@seankhliao seankhliao changed the title cmd/go: -buildmode=c-archive produces code not suitable for use in a shared object on arm64 cmd/compile: -buildmode=c-archive produces code not suitable for use in a shared object on arm64 Sep 10, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 10, 2023
@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. arch-arm64 labels Sep 10, 2023
@mknyszek mknyszek added this to the Backlog milestone Sep 20, 2023
@thanm
Copy link
Contributor

thanm commented Sep 20, 2023

Possibly related: https://sourceware.org/bugzilla/show_bug.cgi?id=28370 ?

@cherrymui
Copy link
Member

I think this may be because we take the address of a function without going through the GOT, and crosscall2 happens to be an exported function (exported in the sense of DSO) (https://cs.opensource.google/go/go/+/master:src/runtime/cgo/callbacks.go;l=26)

We probably could bypass the issue by using a local wrapper function.

(This comment is interesting

// We need to export the symbol crosscall2 in order to support
// callbacks from shared libraries. This applies regardless of
// linking mode.

Why it needs to be exported if we're not using -buildmode=shared or -linkshared? In exe, c-archive, or c-shared build modes I think the crosscall2 call are made from cgo-generated code, which is in the same DSO. Maybe SWIG calls can be in another DSO?)

@cherrymui
Copy link
Member

@nmeum what version of the C linker do you use? Could you show the output of ld -v?

On my machine with GNU ld (GNU Binutils for Debian) 2.40.50.20230611 it seems to work okay with that particular relocation.

@ianlancetaylor
Copy link
Contributor

At the time the "We need to export the symbol crosscall2" comment was written, SWIG (and cgo) worked by writing a shared library. Now that that is no longer can true we can retire the comment.

@nmeum
Copy link
Author

nmeum commented Sep 21, 2023

@nmeum what version of the C linker do you use? Could you show the output of ld -v?

On my machine with GNU ld (GNU Binutils for Debian) 2.40.50.20230611 it seems to work okay with that particular relocation.

I am using GNU ld (GNU Binutils) 2.41. However, even with Binutils 2.41 it works when using Go 1.20.7 and breaks with Go 1.21.1.

@nmeum
Copy link
Author

nmeum commented Sep 21, 2023

So did some further tests it seems that the regression was introduced in c426c87 (this commit also introduced the set_crosscall2 function mentioned in the linker error) prior to this commit (i.e. 2693ade) fcitx5-bamboo builds fine and there is no relocation error.

@cherrymui
Copy link
Member

Thanks. Yeah, that supports my assumption about taking the address of crosscall2 without going through the GOT. I'll work on a fix.

@cherrymui
Copy link
Member

Thanks @ianlancetaylor . I'll send a CL to remove that comment and un-export crosscall2.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/533535 mentions this issue: runtime/cgo: avoid taking the address of crosscall2 in code

@cherrymui
Copy link
Member

@nmeum could you try if CL https://go.dev/cl/533535 fixes the issue? With the CL I can see the R_AARCH64_ADR_PREL_PG_HI21 relocation is gone. But I don't have a C toolchain that actually fails without the CL.

One way to try it is to use the gotip command

$ go install golang.org/dl/gotip@latest
$ gotip download 533535

Thanks.

@nmeum
Copy link
Author

nmeum commented Oct 7, 2023

@nmeum could you try if CL https://go.dev/cl/533535 fixes the issue?

Yes, with that patch applied fcitx5-bamboo builds again on Alpine Linux aarch64 with binutils 2.41. Thanks!

@cherrymui
Copy link
Member

@nmeum thanks for confirming!

@cherrymui
Copy link
Member

@gopherbot please backport this to Go 1.21. This is a regression in Go 1.21 affecting a specific use case without workaround. Thanks.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #63509 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/534915 mentions this issue: [release-branch.go1.21 runtime/cgo: avoid taking the address of crosscall2 in code

algitbot pushed a commit to alpinelinux/aports that referenced this issue Oct 14, 2023
Go upstream patch for this issue has been backported.

See: golang/go#62556
bell-sw pushed a commit to bell-sw/alpaquita-aports that referenced this issue Oct 17, 2023
yunginnanet pushed a commit to yunginnanet/go that referenced this issue Oct 20, 2023
Currently, set_crosscall2 takes the address of crosscall2 without
using the GOT, which, on some architectures, results in a
PC-relative relocation (e.g. R_AARCH64_ADR_PREL_PG_HI21 on ARM64)
to the crosscall2 symbol. But crosscall2 is dynamically exported,
so the C linker thinks it may bind to a symbol from a different
DSO. Some C linker may not like a PC-relative relocation to such a
symbol. Using a local trampoline to avoid taking the address of a
dynamically exported symbol.

It may be possible to not dynamically export crosscall2. But this
CL is safer for backport. Later we may remove the trampolines
after unexport crosscall2, if they are not needed.

Fixes golang#62556.

Change-Id: Id28457f65ef121d3f87d8189803abc65ed453283
Reviewed-on: https://go-review.googlesource.com/c/go/+/533535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit that referenced this issue Oct 30, 2023
…scall2 in code

Currently, set_crosscall2 takes the address of crosscall2 without
using the GOT, which, on some architectures, results in a
PC-relative relocation (e.g. R_AARCH64_ADR_PREL_PG_HI21 on ARM64)
to the crosscall2 symbol. But crosscall2 is dynamically exported,
so the C linker thinks it may bind to a symbol from a different
DSO. Some C linker may not like a PC-relative relocation to such a
symbol. Using a local trampoline to avoid taking the address of a
dynamically exported symbol.

It may be possible to not dynamically export crosscall2. But this
CL is safer for backport. Later we may remove the trampolines
after unexport crosscall2, if they are not needed.

Fixes #63509.
Updates #62556.

Change-Id: Id28457f65ef121d3f87d8189803abc65ed453283
Reviewed-on: https://go-review.googlesource.com/c/go/+/533535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit 872d718)
Reviewed-on: https://go-review.googlesource.com/c/go/+/534915
Reviewed-by: David Chase <drchase@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants