cmd/link: stop requiring gold on arm64 when GNU ld is fixed

[jcajka]

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

Any, more specifically 1.8, 1.9, master, most probably all versions supporting AArch64/arm64 target

Does this issue reproduce with the latest release?

Yes

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

linux/arm64

What did you do?

GC on linux/arm64 assumes gold linker availability see src/cmd/link/internal/ld/lib.go:1182
When gold is not available gcc fails due to it missing, when it tries to invoke it. All in shared_test.
When gold flag is remove by removing sys.ARM64 from codition on src/cmd/link/internal/ld/lib.go:1173, build/test fail with

--- FAIL: TestTrivialExecutable (0.29s)
	shared_test.go:66: executing go install -installsuffix=5577006791947779410 -linkshared trivial failed exit status 2:
		# trivial
		/root/go/pkg/tool/linux_arm64/link: running gcc failed: exit status 1
		/usr/bin/ld: /tmp/go-link-151650837/go.o(.data.rel.ro+0x2c0): unresolvable R_AARCH64_ABS64 relocation against symbol `go.link.abihash.libruntime,sync-atomic.so'
		/usr/bin/ld: final link failed: Nonrepresentable section on output
		collect2: error: ld returned 1 exit status

And so one for most of the cases shared_test.

What did you expect to see?

All tests pass. GC working with default binutils ld.

What did you see instead?

GC failing to build some binaries with binutils ld.

[ianlancetaylor]

For background see #15696 and https://sourceware.org/bugzilla/show_bug.cgi?id=19962. I think that we have to require gold until the GNU ld bug is fixed.

I'm going to close this issue since I don't see anything we can do. Please comment if you disagree.

[jcajka]

@ianlancetaylor AFAIK SWBZ is related only to ARM32, my issues is on ARM64. I could be easily wrong though or there is same issues on ARM64, I'm not that familiar with binutils\s ld code base.

Also I would like to keep this opened, even if this turns out to be fully binutils bug. As I think that the workaround code should be reverted when the issue is fixed in ld(whatever for ARM32/64 or both).

[ianlancetaylor]

OK, I guess.

[jfkw]

I have a situation where the gold linker will not be available on an ARM64 linux target and would like to avoid the gold dependency so I can package Go there. The referenced binutils bug https://sourceware.org/bugzilla/show_bug.cgi?id=19962 has a proposed patch:

--- a/bfd/elf32-arm.c	
+++ a/bfd/elf32-arm.c	
@@ -14112,11 +14112,15 @@ elf32_arm_adjust_dynamic_symbol (struct bfd_link_info * info,
   s = bfd_get_linker_section (dynobj, ".dynbss");
   BFD_ASSERT (s != NULL);
 
-  /* We must generate a R_ARM_COPY reloc to tell the dynamic linker to
-     copy the initial value out of the dynamic object and into the
-     runtime process image.  We need to remember the offset into the
+  /* If allowed, we must generate a R_ARM_COPY reloc to tell the dynamic
+     linker to copy the initial value out of the dynamic object and into
+     the runtime process image.  We need to remember the offset into the
      .rel(a).bss section we are going to use.  */
-  if ((h->root.u.def.section->flags & SEC_ALLOC) != 0 && h->size != 0)
+  if (info->nocopyreloc == 0
+      && (h->root.u.def.section->flags & SEC_ALLOC) != 0
+      /* PR 16177: A copy is only needed if the input section is readonly.  */
+      && (h->root.u.def.section->flags & SEC_READONLY) == 0
+      && h->size != 0)
     {
       asection *srel;

@ianlancetaylor @crawshaw Would it be possible to get some feedback from the Go team on whether that patch addresses the issue for Go? With that it might be possible to get the patch or a revised patch accepted by upstream binutils.

[ianlancetaylor]

I don't see why that patch will help, as as far as I can see it only affects arm, not arm64.

[jfkw]

A proposed patch for aarch64 was added to https://sourceware.org/bugzilla/show_bug.cgi?id=19962 on 2020-08-24:

--- a/bfd/elfnn-aarch64.c	
+++ a/bfd/elfnn-aarch64.c	
@@ -7474,7 +7474,10 @@ elfNN_aarch64_adjust_dynamic_symbol (struct bfd_link_info *info,
       s = htab->root.sdynbss;
       srel = htab->root.srelbss;
     }
-  if ((h->root.u.def.section->flags & SEC_ALLOC) != 0 && h->size != 0)
+  if ((h->root.u.def.section->flags & SEC_ALLOC)
+      && (h->root.u.def.section->flags & SEC_READONLY)
+      && h->size != 0)
+
     {
       srel->size += RELOC_SIZE (htab);
       h->needs_copy = 1;

[jefferyto]

The GNU ld issue has been marked as fixed (according to the last comment the issue is not present in binutils 2.36 and higher). Would it be possible to revisit the gold linker requirement at this point?

The OpenWrt toolchain does not include the gold linker and I have been advising our packages to patch out Go plugins to avoid triggering this requirement (crowdsec, rclone).

[ianlancetaylor]

I suggest that for now we change to request gold but not give an error if gold is not available. Want to test and send a patch for that? It's cmd/link/internal/ld/lib.go.

[jefferyto]

I suggest that for now we change to request gold but not give an error if gold is not available. Want to test and send a patch for that? It's cmd/link/internal/ld/lib.go.

I’ll try to find some time to work on this patch - thanks 🙂

[gopherbot]

Change https://golang.org/cl/366279 mentions this issue: cmd/link: use gold on ARM/ARM64 only if gold is available

[gopherbot]

Change https://go.dev/cl/391115 mentions this issue: cmd/link: stop forcing binutils-gold dependency on aarch64

[coyzeng]

This PR merge plan?

[williamh]

Hi all, I am the package maintainer for Go on Gentoo Linux.
This issue has now been reported to me, and I've been asked to look into just removing the broken linker workaround code.
This is our bug.
Is there any chance of removing the code that hardcodes the linker?

[ianlancetaylor]

Can someone investigate to see exactly when the GNU linker was fixed? There are two different patches submitted for this (https://go.dev/cl/366279, https://go.dev/cl/391115) and they both seem to have some discussion.

In particular, do you have concerns about the approach in https://go.dev/cl/366279?

[williamh]

@ianlancetaylor I am not an expert in the toolchain, so hopefully someone else will join this issue and comment. The position of the people wanting me to diverge from what upstream is
doing is that they want to completely move away from gold on Gentoo.

[thesamesam]

Thanks for commenting Ian.

On binutils:

• arm64: I've tried binutils 2.32..2.40 inclusive with https://sourceware.org/bugzilla/show_bug.cgi?id=19962#c5 and I don't see the issue with any of them.
• arm: Ditto (using the first comment instead).
• @Hello71 on go.dev/cl/366279 couldn't reproduce it with even older binutils anyway
• @jefferyto in a comment on https://go.dev/cl/391115 says 2.28 is the first fixed version.

For Gentoo, our usecase/concerns are:

• Go currently forces use of gold on arm/arm64 for a bug not affecting Gentoo for quite some time (we keep binutils very up to date)
• We want the linker to match the user's default is in their compiler, unless there's a good reason not to (for their benefit + making it easier to understand/reproduce bugs)
• The "automagic" (use-if-available) patch might be a good compromise Go upstream, but not ideal downstream. We're mostly moving away from gold right now and changing behaviour based on whether a package is/isn't installed leads to confusing/reproducibility issues with bugs

My preference would be therefore be https://go.dev/cl/391115 (with or without the make.bash check, as it doesn't affect us).

On making the change at all:

• I'm a bit concerned about @Hello71's comment at https://go.dev/cl/366279 -> cmd/link: go tool dist test testshared failed if linked with lld or mold #46560 (comment). Given the analysis in that latter bug, it might be worth removing the NeedsInvestigation label, but not sure how Go triage works.
• I can reproduce the test failures when using bfd-2.40 in TestGCData with https://go.dev/cl/391115 applied to Go 1.20, so I don't think we can move forward yet.

[ianlancetaylor]

@thesamesam Thanks. Just to make sure that I understand, you said:

I can reproduce the test failures when using bfd-2.40 in TestGCData with https://go.dev/cl/391115 applied to Go 1.20, so I don't think we can move forward yet.

It sounds like this means that there is still some problem using the Go linker with GNU ld 2.40 on arm64 Linux, but that using the Go linker with gold does work. Is that correct?

[thesamesam]

@ianlancetaylor Yeah, exactly. For the situation you describe: arm Linux is OK, but arm64 Linux is not.

[gopherbot]

Change https://go.dev/cl/468655 mentions this issue: cmd/link: don't switch to gold on ARM Linux

[susematz]

@thesamesam Thanks. Just to make sure that I understand, you said:

I can reproduce the test failures when using bfd-2.40 in TestGCData with https://go.dev/cl/391115 applied to Go 1.20, so I don't think we can move forward yet.

It sounds like this means that there is still some problem using the Go linker with GNU ld 2.40 on arm64 Linux, but that using the Go linker with gold does work. Is that correct?

After much fighting with the golang toolchain and how its linking process works I finally found the BFD problem:
it's https://sourceware.org/bugzilla/show_bug.cgi?id=30437 and I have posted a fix for it. With ld.bfd so patched and the change to disable hardcoding ld.gold the golang testsuite now passes everything. (Tested on release-branch.go1.20).

[ianlancetaylor]

@susematz Thanks. So after that patch is committed into a binutils release, we can remove the remaining case where the Go linker requires gold.

[jefferyto]

The binutils issue was marked as fixed on May 23 and a new version (2.41) was released on July 30.

[ianlancetaylor]

Thanks. Would somebody like to update and test https://go.dev/cl/391115?

[dirkmueller]

it was back up now, so I updated.

[jfkw]

Tests in all.bash pass with the updated CL https://go.dev/cl/391115/4. On SUSE and openSUSE we have applied this patch from go1.16 through go1.21 without issues.

[zpon]

Are there any progress on landing one of the proposed solutions to remove the "gold" check? Lots of projects are ending up patching the go code.