cmd/link: fix handling of visibility hidden symbols

There is a TODO comment that checking hidden visibility is
probably not the right thing to do. I think it is indeed not. Here
we are not referencing symbols across DSO boundaries, just within
an executable binary. The hidden visibility is for references from
another DSO. So it doesn't actually matter.

This makes cgo internal linking tests work on ARM64 with newer
GCC. It failed and was disabled due to a visibility hidden symbol
in libgcc.a that we didn't handle correctly. Specifically, the
problem is that we didn't mark visibility hidden symbol references
SXREF, which caused the loader to not think it is an unresolved
external symbol, which in turn made it not loading an object file
from the libgcc.a archive which contains the actual definition.
Later stage when we try to resolve the relocation, we couldn't
resolve it. Enable the test as it works now.

Fixes #39466.

Change-Id: I2759e3ae15e7a7a1ab9a820223b688ad894509ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/404296
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index be4c552fd712d48f84f456c2a167472ba77ab6d2..817ea4a7c5c06695b5a505506fd7137844660e3e 100644 (file)
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -1188,11 +1188,7 @@ func (t *tester) cgoTest(dt *distTest) error {
        cmd := t.addCmd(dt, "misc/cgo/test", t.goTest())
        setEnv(cmd, "GOFLAGS", "-ldflags=-linkmode=auto")
 
-       // Skip internal linking cases on arm64 to support GCC-9.4 and above.
-       // See issue #39466.
-       skipInternalLink := goarch == "arm64" && goos != "darwin"
-
-       if t.internalLink() && !skipInternalLink {
+       if t.internalLink() {
                cmd := t.addCmd(dt, "misc/cgo/test", t.goTest(), "-tags=internal")
                setEnv(cmd, "GOFLAGS", "-ldflags=-linkmode=internal")
        }
@@ -1268,7 +1264,7 @@ func (t *tester) cgoTest(dt *distTest) error {
 
                        if t.supportedBuildmode("pie") {
                                t.addCmd(dt, "misc/cgo/test", t.goTest(), "-buildmode=pie")
-                               if t.internalLink() && t.internalLinkPIE() && !skipInternalLink {
+                               if t.internalLink() && t.internalLinkPIE() {
                                        t.addCmd(dt, "misc/cgo/test", t.goTest(), "-buildmode=pie", "-ldflags=-linkmode=internal", "-tags=internal,internal_pie")
                                }
                                t.addCmd(dt, "misc/cgo/testtls", t.goTest(), "-buildmode=pie")

diff --git a/src/cmd/link/internal/amd64/asm.go b/src/cmd/link/internal/amd64/asm.go
index fb960491de7215817ccbe9f285f8f8751ad63190..f4832efcf73694f397ead033430709e4f4f40770 100644 (file)
--- a/src/cmd/link/internal/amd64/asm.go
+++ b/src/cmd/link/internal/amd64/asm.go
@@ -88,9 +88,7 @@ func adddynrel(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s loade
                if targType == sym.SDYNIMPORT {
                        ldr.Errorf(s, "unexpected R_X86_64_PC32 relocation for dynamic symbol %s", ldr.SymName(targ))
                }
-               // TODO(mwhudson): the test of VisibilityHidden here probably doesn't make
-               // sense and should be removed when someone has thought about it properly.
-               if (targType == 0 || targType == sym.SXREF) && !ldr.AttrVisibilityHidden(targ) {
+               if targType == 0 || targType == sym.SXREF {
                        ldr.Errorf(s, "unknown symbol %s in pcrel", ldr.SymName(targ))
                }
                su := ldr.MakeSymbolUpdater(s)

diff --git a/src/cmd/link/internal/arm64/asm.go b/src/cmd/link/internal/arm64/asm.go
index 229a4d300bfc2e240c5c41d97728988f4e28d2e3..9937683a133190fdb8a4a6d03449a12408b93b81 100644 (file)
--- a/src/cmd/link/internal/arm64/asm.go
+++ b/src/cmd/link/internal/arm64/asm.go
@@ -91,9 +91,7 @@ func adddynrel(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s loade
                if targType == sym.SDYNIMPORT {
                        ldr.Errorf(s, "unexpected R_AARCH64_PREL32 relocation for dynamic symbol %s", ldr.SymName(targ))
                }
-               // TODO(mwhudson): the test of VisibilityHidden here probably doesn't make
-               // sense and should be removed when someone has thought about it properly.
-               if (targType == 0 || targType == sym.SXREF) && !ldr.AttrVisibilityHidden(targ) {
+               if targType == 0 || targType == sym.SXREF {
                        ldr.Errorf(s, "unknown symbol %s in pcrel", ldr.SymName(targ))
                }
                su := ldr.MakeSymbolUpdater(s)
@@ -121,7 +119,7 @@ func adddynrel(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s loade
                        su.SetRelocSym(rIdx, syms.PLT)
                        su.SetRelocAdd(rIdx, r.Add()+int64(ldr.SymPlt(targ)))
                }
-               if (targType == 0 || targType == sym.SXREF) && !ldr.AttrVisibilityHidden(targ) {
+               if targType == 0 || targType == sym.SXREF {
                        ldr.Errorf(s, "unknown symbol %s in callarm64", ldr.SymName(targ))
                }
                su := ldr.MakeSymbolUpdater(s)

diff --git a/src/cmd/link/internal/loadelf/ldelf.go b/src/cmd/link/internal/loadelf/ldelf.go
index d05d8e3b4b691ae2e6e39ae9bc6d60ae172d975b..03813909de9fd9dfa5ccf968728ade23fdfa6f80 100644 (file)
--- a/src/cmd/link/internal/loadelf/ldelf.go
+++ b/src/cmd/link/internal/loadelf/ldelf.go
@@ -934,9 +934,7 @@ func readelfsym(newSym, lookup func(string, int) loader.Sym, l *loader.Loader, a
                }
        }
 
-       // TODO(mwhudson): the test of VisibilityHidden here probably doesn't make
-       // sense and should be removed when someone has thought about it properly.
-       if s != 0 && l.SymType(s) == 0 && !l.AttrVisibilityHidden(s) && elfsym.type_ != elf.STT_SECTION {
+       if s != 0 && l.SymType(s) == 0 && elfsym.type_ != elf.STT_SECTION {
                sb := l.MakeSymbolUpdater(s)
                sb.SetType(sym.SXREF)
        }