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.

Sign up for GitHub

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

Jump to bottom

x/text: building as a plugin failure on darwin/arm64 #58826

Closed
zoncoen opened this issue Mar 2, 2023 · 12 comments
Closed

x/text: building as a plugin failure on darwin/arm64 #58826

zoncoen opened this issue Mar 2, 2023 · 12 comments
Assignees
@cherrymui @erifan
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@zoncoen
Copy link

zoncoen commented Mar 2, 2023

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

$ go version
go version go1.20.1 darwin/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="/Users/zoncoen/Library/Caches/go-build"
GOENV="/Users/zoncoen/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/zoncoen/dev/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/zoncoen/dev"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.1"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/zoncoen/dev/src/github.com/zoncoen-sample/go1.20-plugin-issue/go.mod"
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7b/75304vxx7szc4b0lph6y311c0000gp/T/go-build328686675=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

go build fails with -buildmode=plugin flag on darwin/arm64 if the code imports golang.org/x/text.

$ go build -buildmode=plugin -o plugin.so .

$ cat main.go
package main

import "golang.org/x/text/date"

func EtcUTC() int {
        return date.EtcUTC
}

The complete reproducing code is here.

What did you expect to see?

go build succeeds without error.

What did you see instead?

$ go build -buildmode=plugin -o plugin.so .
# golang.org/x/text/internal/language
:1: REGTMP used in large offset load: 00204 (/Users/zoncoen/dev/pkg/mod/golang.org/x/text@v0.7.0/internal/language/coverage.go:18)       MOVD    2189(R27), R5

I have confirmed that it builds correctly on linux/amd64.
@gopherbot gopherbot added this to the Unreleased milestone Mar 2, 2023
@dmitshur
Copy link
Contributor

dmitshur commented Mar 2, 2023

CC @mpvl, @golang/runtime.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 2, 2023
@cherrymui
Copy link
Member

This looks like some issue in the assembler. I can look into that.

@cherrymui cherrymui self-assigned this Mar 2, 2023
@dtrudg
Copy link

dtrudg commented Mar 7, 2023 •
edited

We are seeing the same issue on linux/arm64 (RHEL8, RHEL9, SLES15) when building a plugin with go 1.20.1

# golang.org/x/text/internal/language
    <autogenerated>:1: REGTMP used in large offset load: 00204 (golang.org/x/text@v0.7.0/internal/language/coverage.go:18)  MOVD    2189(R27), R5
    FATAL:   Plugin compile failed with error: while building plugin .so: exit status 1
    error: Bad exit status from /var/tmp/rpm-tmp.CfiMwQ (%build)
        Bad exit status from /var/tmp/rpm-tmp.CfiMwQ (%build)

The build completes without errors on linux/amd64, and on arm64 with go 1.19.5

@cherrymui
Copy link
Member

I think this is due to CL https://golang.org/cl/445535 . The CL description says

If off is very large, it becomes:
  MOVD sym@GOT, REGTMP
  MOVD $off, Rt
  ADD  Rt, REGTMP
  MOVx (REGTMP), Ry

The question is what is Rt? It cannot use REGTMP. Maybe it can use Ry but currently it doesn't do so. Also, Ry only works for load, not store.

cc @golang/arm @erifan

@gopherbot
Copy link

Change https://go.dev/cl/474175 mentions this issue: Revert "cmd/compile: enable address folding for global symbols of shared library"

@cherrymui
Copy link
Member

@gopherbot please backport this to Go 1.20. This can cause code failing to build. Thanks.

@gopherbot gopherbot mentioned this issue Mar 7, 2023
Closed
@gopherbot
Copy link

Backport issue(s) opened: #58920 (for 1.20).

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

Change https://go.dev/cl/474235 mentions this issue: [release-branch.go1.20] Revert "cmd/compile: enable address folding for global symbols of shared library"

@gopherbot
Copy link

Change https://go.dev/cl/473999 mentions this issue: cmd/compile: enable address folding for globals on ARM64, just not -dynlink mode

@cherrymui
Copy link
Member

The culprit CL is reverted. I sent CL 473999 to re-enable the optimization but not in -dynlink mode, which is -buildmode=plugin uses and where the problem actually is.

gopherbot pushed a commit that referenced this issue Mar 7, 2023
@cherrymui
cmd/compile: enable address folding for globals on ARM64, just not -d…
b675a75 
…ynlink mode

On ARM64, in -dynlink mode (building a shared library or a plugin),
accessing global variable is made using the GOT. Currently, the
GOT accessing instruction sequence our assembler generates doesn't
handle large offset well, so we don't fold the offset into loads
and stores in the compiler. Currently, the rewrite rules are
guarded with the -shared flag. However, the GOT access
instructions are only generated in the -dynlink mode (which
implies -shared, but not the other direction).

CL 445535 attempted to remove the guard althgether. But that
causes build failure for -dynlink mode for the reason above. This
CL changes it to guard specifically on -dynlink mode, allowing
the optimization in more cases (-shared but not -dynlink build
modes).

Updates #58826.

Change-Id: I1391db6a33e8d0455a304e7cae7fcfdeb49bfdab
Reviewed-on: https://go-review.googlesource.com/c/go/+/473999
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@zoncoen
Copy link
Author

zoncoen commented Mar 9, 2023

I confirmed gotip (devel go1.21-fc45eb3 Thu Mar 9 00:51:36 2023 +0000 darwin/arm64) could build actual code. Thank you.

@erifan
Copy link

erifan commented Mar 9, 2023

I'm sorry to introduce this bug, and thanks @cherrymui for taking care of this.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 10, 2023
gopherbot pushed a commit that referenced this issue Mar 15, 2023
@cherrymui
[release-branch.go1.20] Revert "cmd/compile: enable address folding f…
5c7cc46 
…or global symbols of shared library"

This reverts CL 445535.

Reason for revert: see issue #58826. It doesn't handle large offset well.

Updates #58826.
Fixes #58920.

Change-Id: Ic4a33f4c510c88628ea7e16207a60977a04cf798
Reviewed-on: https://go-review.googlesource.com/c/go/+/474175
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
(cherry picked from commit a4cf4fd)
Reviewed-on: https://go-review.googlesource.com/c/go/+/474235
Reviewed-by: Keith Randall <khr@golang.org>
@golang golang locked and limited conversation to collaborators Mar 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@cherrymui cherrymui

@erifan erifan

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@dmitshur @zoncoen @dtrudg @gopherbot @cherrymui @erifan