cmd/link: replace SHIDDEN bit in SymKind with a bit of Attribute

Change-Id: I02dab81393cc9339895f0076df41a652aded5b60 Reviewed-on: https://go-review.googlesource.com/42025 Run-TryBot: Michael Hudson-Doyle <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
Harvey-OS · Apr 30, 2017 · a69222d · a69222d
1 parent 3bcb481
commit a69222d
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 11 deletions.
diff --git a/src/cmd/link/internal/arm64/asm.go b/src/cmd/link/internal/arm64/asm.go
@@ -250,7 +250,7 @@ func archreloc(ctxt *ld.Link, r *ld.Reloc, s *ld.Symbol, val *int64) int {
 			// (https://sourceware.org/bugzilla/show_bug.cgi?id=18270). So
 			// we convert the adrp; ld64 + R_ARM64_GOTPCREL into adrp;
 			// add + R_ADDRARM64.
-			if !(r.Sym.Version != 0 || (r.Sym.Type&ld.SHIDDEN != 0) || r.Sym.Attr.Local()) && r.Sym.Type == ld.STEXT && ctxt.DynlinkingGo() {
+			if !(r.Sym.Version != 0 || r.Sym.Attr.VisibilityHidden() || r.Sym.Attr.Local()) && r.Sym.Type == ld.STEXT && ctxt.DynlinkingGo() {
 				if o2&0xffc00000 != 0xf9400000 {
 					ld.Errorf(s, "R_ARM64_GOTPCREL against unexpected instruction %x", o2)
 				}

diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go
@@ -386,7 +386,7 @@ func relocsym(ctxt *Link, s *Symbol) {
 			continue
 		}
 
-		if r.Sym != nil && (r.Sym.Type&(SMASK|SHIDDEN) == 0 || r.Sym.Type&SMASK == SXREF) {
+		if r.Sym != nil && ((r.Sym.Type == 0 && !r.Sym.Attr.VisibilityHidden()) || r.Sym.Type&SMASK == SXREF) {
 			// When putting the runtime but not main into a shared library
 			// these symbols are undefined and that's OK.
 			if Buildmode == BuildmodeShared {

diff --git a/src/cmd/link/internal/ld/ldelf.go b/src/cmd/link/internal/ld/ldelf.go
@@ -1060,8 +1060,7 @@ func readelfsym(ctxt *Link, elfobj *ElfObj, i int, sym *ElfSym, needSym int, loc
 				// set dupok generally. See http://codereview.appspot.com/5823055/
 				// comment #5 for details.
 				if s != nil && sym.other == 2 {
-					s.Type |= SHIDDEN
-					s.Attr |= AttrDuplicateOK
+					s.Attr |= AttrDuplicateOK | AttrVisibilityHidden
 				}
 			}
 
@@ -1077,7 +1076,7 @@ func readelfsym(ctxt *Link, elfobj *ElfObj, i int, sym *ElfSym, needSym int, loc
 				// so put it in the hash table.
 				if needSym != 0 {
 					s = ctxt.Syms.Lookup(sym.name, localSymVersion)
-					s.Type |= SHIDDEN
+					s.Attr |= AttrVisibilityHidden
 				}
 
 				break
@@ -1089,14 +1088,14 @@ func readelfsym(ctxt *Link, elfobj *ElfObj, i int, sym *ElfSym, needSym int, loc
 				// don't bother to add them into the hash table
 				s = ctxt.Syms.newsym(sym.name, localSymVersion)
 
-				s.Type |= SHIDDEN
+				s.Attr |= AttrVisibilityHidden
 			}
 
 		case ElfSymBindWeak:
 			if needSym != 0 {
 				s = ctxt.Syms.Lookup(sym.name, 0)
 				if sym.other == 2 {
-					s.Type |= SHIDDEN
+					s.Attr |= AttrVisibilityHidden
 				}
 			}
 

diff --git a/src/cmd/link/internal/ld/link.go b/src/cmd/link/internal/ld/link.go
@@ -135,6 +135,11 @@ const (
 	// AttrMakeTypelink Amarks types that should be added to the typelink
 	// table. See typelinks.go:typelinks().
 	AttrMakeTypelink
+	// AttrVisibilityHidden symbols are ELF symbols with
+	// visibility set to STV_HIDDEN. They become local symbols in
+	// the final executable. Only relevant when internally linking
+	// on an ELF platform.
+	AttrVisibilityHidden
 )
 
 func (a Attribute) DuplicateOK() bool      { return a&AttrDuplicateOK != 0 }
@@ -150,6 +155,7 @@ func (a Attribute) OnList() bool           { return a&AttrOnList != 0 }
 func (a Attribute) Local() bool            { return a&AttrLocal != 0 }
 func (a Attribute) ReflectMethod() bool    { return a&AttrReflectMethod != 0 }
 func (a Attribute) MakeTypelink() bool     { return a&AttrMakeTypelink != 0 }
+func (a Attribute) VisibilityHidden() bool { return a&AttrVisibilityHidden != 0 }
 
 func (a Attribute) CgoExport() bool {
 	return a.CgoExportDynamic() || a.CgoExportStatic()

diff --git a/src/cmd/link/internal/ld/pe.go b/src/cmd/link/internal/ld/pe.go
@@ -1041,7 +1041,9 @@ func writePESymTableRecords(ctxt *Link) int {
 			typ = 0x0308 // "array of structs"
 		}
 		class := IMAGE_SYM_CLASS_EXTERNAL
-		if s.Version != 0 || (s.Type&SHIDDEN != 0) || s.Attr.Local() {
+		// TODO(mwudson): I think s.Attr.VisibilityHidden()
+		// can only ever be true for an ELF link.
+		if s.Version != 0 || s.Attr.VisibilityHidden() || s.Attr.Local() {
 			class = IMAGE_SYM_CLASS_STATIC
 		}
 		writeOneSymbol(s, value, sect, typ, uint8(class))

diff --git a/src/cmd/link/internal/ld/symkind.go b/src/cmd/link/internal/ld/symkind.go
@@ -106,7 +106,6 @@ const (
 	SDWARFINFO
 	SSUB       = SymKind(1 << 8)
 	SMASK      = SymKind(SSUB - 1)
-	SHIDDEN    = SymKind(1 << 9)
 	SCONTAINER = SymKind(1 << 10) // has a sub-symbol
 )
 

diff --git a/src/cmd/link/internal/ld/symtab.go b/src/cmd/link/internal/ld/symtab.go
@@ -127,7 +127,7 @@ func putelfsym(ctxt *Link, x *Symbol, s string, t SymbolType, addr int64, go_ *S
 	// maybe one day STB_WEAK.
 	bind := STB_GLOBAL
 
-	if x.Version != 0 || (x.Type&SHIDDEN != 0) || x.Attr.Local() {
+	if x.Version != 0 || x.Attr.VisibilityHidden() || x.Attr.Local() {
 		bind = STB_LOCAL
 	}
 
@@ -144,7 +144,11 @@ func putelfsym(ctxt *Link, x *Symbol, s string, t SymbolType, addr int64, go_ *S
 		addr -= int64(xo.Sect.Vaddr)
 	}
 	other := STV_DEFAULT
-	if x.Type&SHIDDEN != 0 {
+	if x.Attr.VisibilityHidden() {
+		// TODO(mwhudson): We only set AttrVisibilityHidden in ldelf,
+		// i.e. when internally linking. But STV_HIDDEN visibility only
+		// matters in object files, i.e. when externally linking. So I
+		// don't think this makes a lot of sense.
 		other = STV_HIDDEN
 	}
 	if (Buildmode == BuildmodeCArchive || Buildmode == BuildmodePIE || ctxt.DynlinkingGo()) && SysArch.Family == sys.PPC64 && typ == STT_FUNC && x.Name != "runtime.duffzero" && x.Name != "runtime.duffcopy" {