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

runtime: Go 1.22.0 fails to build from source on armv7 Alpine Linux #65601

Closed
nmeum opened this issue Feb 8, 2024 · 18 comments
Closed

runtime: Go 1.22.0 fails to build from source on armv7 Alpine Linux #65601

nmeum opened this issue Feb 8, 2024 · 18 comments
Labels
arch-arm Issues solely affecting the 32-bit arm architecture. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-Linux
Milestone

Comments

@nmeum
Copy link

nmeum commented Feb 8, 2024

Go version

N/A

Output of go env in your module/workspace:

N/A

What did you do?

I maintain the Go package for Alpine Linux, this package builds Go from source using ./make.bash.

While upgrading from Go 1.21.6 to Go 1.22.0 we noticed that Go itself wouldn't build on our armv7 CI.

What did you see happen?

Go 1.22.0 builds successful on Alpine x86_64, x86, armhf (GOARM=6), ppc64le, aarch64 and riscv64. It only fails to build on armv7 (GOARM=7). Furthermore, Go 1.21.6 builds fine on our armv7 CI. Therefore, I believe this to be a regression in Go 1.22.0. The build failure message is not consistent, there is probably a race condition somewhere. However, it always fails either after Building Go toolchain2 using go_bootstrap and Go toolchain1 or Building Go toolchain3 using go_bootstrap and Go toolchain2.

Build logs of some of the failures we have seen so far (as I said, the exact error message differs):

Any idea what might be causing this? Any hints on debugging this further?

What did you expect to see?

A successful build of Go 1.22.0 on all architectures.

@dmitshur dmitshur added OS-Linux arch-arm Issues solely affecting the 32-bit arm architecture. labels Feb 8, 2024
@dmitshur
Copy link
Contributor

dmitshur commented Feb 8, 2024

CC @golang/arm.

@seankhliao
Copy link
Member

Are these bare metal or cross arch virtualized machines?

@cherrymui
Copy link
Member

Thank for the report. Would it be okay for you to do a bisection? Thanks.

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 8, 2024
@nmeum
Copy link
Author

nmeum commented Feb 9, 2024

Are these bare metal or cross arch virtualized machines?

These are armv7 Docker containers running on an aarch64 host.

Thank for the report. Would it be okay for you to do a bisection? Thanks.

I am currently somewhat short-pressed on time, but I will try to look into it.

@nmeum
Copy link
Author

nmeum commented Feb 9, 2024

The commit which introduced the regression is 66b8107 (CC: @randall77).

Reverting this commit fixes the build.

@cherrymui
Copy link
Member

It might be an interesting interaction between CL 525637 and CL 514907. Specifically, this code https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/work/gc.go;l=369-381 is defining assembler macros GOARM_x based on cfg.GOARM and expecting a value like "5", "6", or "7", but with CL 514907, it can also be "7,hardfloat" (which is probably the case if you set GOARM at make.bash), which caused the wrong macro to be defined, which in turn makes it possible to drop some memory barrier instructions. There is also run-time CPU feature detection based on the HWCAP, but for a Docker container running on ARM64, that could go off.

Could you try if CL https://go.dev/cl/562995 helps? Thanks.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/562995 mentions this issue: cmd/go: handle GOARM value with soft/hardfloat in defining assembly macros

@nmeum
Copy link
Author

nmeum commented Feb 10, 2024

Could you try if CL https://go.dev/cl/562995 helps? Thanks.

Still deadlocks with this patch.

@cherrymui
Copy link
Member

Thanks. I updated the CL. Could you try the new version of the CL? It seems we also need to define GOARM_x in cmd/dist.

@nmeum
Copy link
Author

nmeum commented Feb 13, 2024

Thanks for the patch. Unfortunately, it still deadlocks for me with the updated version of the CL.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/564855 mentions this issue: runtime/internal/atomic: correct GOARM=7 guard at a DMB instruction

@cherrymui
Copy link
Member

Thanks. Could you try https://go.dev/cl/564855 ? That should fix the build.
(CL 562995 is probably good to have but is not required for a successful build.)

@dmitshur dmitshur added this to the Go1.23 milestone Feb 16, 2024
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 16, 2024
gopherbot pushed a commit that referenced this issue Feb 16, 2024
…/hardfloat

CL 525637 added GOARM_x assembly macros based on GOARM value. But
it did not define the macro in cmd/dist, so the macro is not set
during bootstrapping. This CL defines them.

With CL 514907, cfg.GOARM can also take a soft/hardfloat suffix,
like "7,hardfloat". Handle that case.

For #65601.

Change-Id: I60ffe7e8b623ae693d91d6e8595067a6f76565b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/562995
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>
@cherrymui
Copy link
Member

@gopherbot please backport this to Go 1.22 release. This is a regression, causing runtime crashes with GOARM=7.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #65760 (for 1.22).

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

@cherrymui cherrymui changed the title cmd/go: Go 1.22.0 fails to build from source on armv7 Alpine Linux runtime: Go 1.22.0 fails to build from source on armv7 Alpine Linux Feb 16, 2024
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Feb 17, 2024
gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Feb 17, 2024
It needs broken-on-arm >=dev-lang/go-1.22.0 and 1.22.0 is the only version matching that.

Bug: golang/go#65601
Bug: https://bugs.gentoo.org/924649
Bug: https://bugs.gentoo.org/924744
Signed-off-by: Sam James <sam@gentoo.org>
algitbot pushed a commit to alpinelinux/aports that referenced this issue Feb 17, 2024
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
CL 525637 changed to the guard of DMB instruction from the
compiled-in runtime.goarm value to GOARM_7 macro and CPU feature
detection. It missed a place where runtime.goarm is loaded to a
register and reused later. This CL corrects the condition.

Fixes golang#65601.

Change-Id: I2ddefd03a1eb1048dbec0254c6e234c65b054279
Reviewed-on: https://go-review.googlesource.com/c/go/+/564855
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
…/hardfloat

CL 525637 added GOARM_x assembly macros based on GOARM value. But
it did not define the macro in cmd/dist, so the macro is not set
during bootstrapping. This CL defines them.

With CL 514907, cfg.GOARM can also take a soft/hardfloat suffix,
like "7,hardfloat". Handle that case.

For golang#65601.

Change-Id: I60ffe7e8b623ae693d91d6e8595067a6f76565b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/562995
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>
bell-sw pushed a commit to bell-sw/alpaquita-aports that referenced this issue Feb 20, 2024
[ commit fc4a497e2990cc62ac8bef5ce8f41a906d990b73 ]

See golang/go#65601
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/567555 mentions this issue: [release-branch.go1.22] runtime/internal/atomic: correct GOARM=7 guard at a DMB instruction

gopherbot pushed a commit that referenced this issue Feb 27, 2024
…d at a DMB instruction

CL 525637 changed to the guard of DMB instruction from the
compiled-in runtime.goarm value to GOARM_7 macro and CPU feature
detection. It missed a place where runtime.goarm is loaded to a
register and reused later. This CL corrects the condition.

Updates #65601.
Fixes #65760.

Change-Id: I2ddefd03a1eb1048dbec0254c6e234c65b054279
Reviewed-on: https://go-review.googlesource.com/c/go/+/564855
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit a0226c5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/567555
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Bypass: Carlos Amedee <carlos@golang.org>
bradfitz pushed a commit to tailscale/go that referenced this issue Mar 5, 2024
…d at a DMB instruction

CL 525637 changed to the guard of DMB instruction from the
compiled-in runtime.goarm value to GOARM_7 macro and CPU feature
detection. It missed a place where runtime.goarm is loaded to a
register and reused later. This CL corrects the condition.

Updates golang#65601.
Fixes golang#65760.

Change-Id: I2ddefd03a1eb1048dbec0254c6e234c65b054279
Reviewed-on: https://go-review.googlesource.com/c/go/+/564855
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit a0226c5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/567555
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Bypass: Carlos Amedee <carlos@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 6, 2024
…d at a DMB instruction

CL 525637 changed to the guard of DMB instruction from the
compiled-in runtime.goarm value to GOARM_7 macro and CPU feature
detection. It missed a place where runtime.goarm is loaded to a
register and reused later. This CL corrects the condition.

Updates golang#65601.
Fixes golang#65760.

Change-Id: I2ddefd03a1eb1048dbec0254c6e234c65b054279
Reviewed-on: https://go-review.googlesource.com/c/go/+/564855
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit a0226c5)
Reviewed-on: https://go-review.googlesource.com/c/go/+/567555
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Bypass: Carlos Amedee <carlos@golang.org>
@clausecker
Copy link

@cherrymui It seems that your backport was incomplete. Commit e4ebd13 of CL562995 has not been backported, but seems to be required to fix the build. As is, Go 1.22.3 doesn't build on armv7 FreeBSD. Could you backport the missing commit, too?

@cherrymui
Copy link
Member

cherrymui commented May 13, 2024

@clausecker thanks for reporting. I thought that CL 562995 is nice to have to but not a requirement: if the macro is not defined correctly it just builds a binary that is not optimized for GOARM=7. It is possible I missed something. What is the error you saw? Thanks.

@clausecker
Copy link

@cherrymui I get crashes and hangs that look very similar to what @nmeum saw. A bisection led me to 6ecadb4 for the first commit where this happens (may be incorrect) and to e4ebd13 for where this behaviour ceases.

See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=278953 for where I track the build failure in our port.

netgate-git-updates pushed a commit to pfsense/FreeBSD-ports that referenced this issue May 23, 2024
This backports a commit needed to get proper barriers during bootstrap
when building Go with more than one job on armv7.

Note that this only changes the behaviour when building the bootstrap
toolchain.  Binaries generated using the toolchain are not affected
and hence Go ports need not be bumped.

PR:		278953
Approved by:	portmgr (build fix blanket)
Obtained from:	https://go-review.googlesource.com/c/go/+/562995
See also:	golang/go#65601
MFH:		2024Q2
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue May 23, 2024
This backports a commit needed to get proper barriers during bootstrap
when building Go with more than one job on armv7.

Note that this only changes the behaviour when building the bootstrap
toolchain.  Binaries generated using the toolchain are not affected
and hence Go ports need not be bumped.

PR:		278953
Approved by:	portmgr (build fix blanket)
Obtained from:	https://go-review.googlesource.com/c/go/+/562995
See also:	golang/go#65601
MFH:		2024Q2

(cherry picked from commit 1ec5017)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-arm Issues solely affecting the 32-bit arm architecture. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-Linux
Projects
None yet
Development

No branches or pull requests

7 participants