[dev.link] cmd/link: make symbol attribute setting more reliable

For dupOK symbols, their attributes should be OR'd. Most of the
attributes are expected to be set consistently across multiple
definitions, but UsedInIface must be OR'd, and for alignment we
need to pick the largest one. Currently the attributes are not
always OR'd, depending on addSym returning true or false. This
doesn't cause any real problem, but it would be a problem if we
make type descriptor symbols content-addressable.

This CL removes the second result of addSym, and lets preloadSyms
always set the attributes. Also removes the alignment handling on
addSym, handles it in preloadSyms only.

Change-Id: I06b3f0adb733f6681956ea9ef54736baa86ae7bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/245720
Reviewed-by: Jeremy Faller <jeremy@golang.org>
This commit is contained in:
Cherry Zhang 2020-07-29 19:32:31 -04:00
parent c4ee16eda9
commit 847b9be3f6
2 changed files with 33 additions and 51 deletions

View file

@ -172,10 +172,9 @@ func growBitmap(reqLen int, b Bitmap) Bitmap {
return b return b
} }
type symSizeAlign struct { type symAndSize struct {
sym Sym sym Sym
size uint32 size uint32
align uint32
} }
// A Loader loads new object files and resolves indexed symbol references. // A Loader loads new object files and resolves indexed symbol references.
@ -205,10 +204,10 @@ type Loader struct {
objSyms []objSym // global index mapping to local index objSyms []objSym // global index mapping to local index
hashed64Syms map[uint64]symSizeAlign // short hashed (content-addressable) symbols, keyed by content hash hashed64Syms map[uint64]symAndSize // short hashed (content-addressable) symbols, keyed by content hash
hashedSyms map[goobj2.HashType]symSizeAlign // hashed (content-addressable) symbols, keyed by content hash hashedSyms map[goobj2.HashType]symAndSize // hashed (content-addressable) symbols, keyed by content hash
symsByName [2]map[string]Sym // map symbol name to index, two maps are for ABI0 and ABIInternal symsByName [2]map[string]Sym // map symbol name to index, two maps are for ABI0 and ABIInternal
extStaticSyms map[nameVer]Sym // externally defined static symbols, keyed by name extStaticSyms map[nameVer]Sym // externally defined static symbols, keyed by name
extReader *oReader // a dummy oReader, for external symbols extReader *oReader // a dummy oReader, for external symbols
payloadBatch []extSymPayload payloadBatch []extSymPayload
@ -331,8 +330,8 @@ func NewLoader(flags uint32, elfsetstring elfsetstringFunc, reporter *ErrorRepor
objs: []objIdx{{}, {extReader, 0}}, // reserve index 0 for nil symbol, 1 for external symbols objs: []objIdx{{}, {extReader, 0}}, // reserve index 0 for nil symbol, 1 for external symbols
objSyms: make([]objSym, 1, 100000), // reserve index 0 for nil symbol objSyms: make([]objSym, 1, 100000), // reserve index 0 for nil symbol
extReader: extReader, extReader: extReader,
hashed64Syms: make(map[uint64]symSizeAlign, 10000), // TODO: adjust preallocation sizes hashed64Syms: make(map[uint64]symAndSize, 10000), // TODO: adjust preallocation sizes
hashedSyms: make(map[goobj2.HashType]symSizeAlign, 20000), // TODO: adjust preallocation sizes hashedSyms: make(map[goobj2.HashType]symAndSize, 20000), // TODO: adjust preallocation sizes
symsByName: [2]map[string]Sym{make(map[string]Sym, 80000), make(map[string]Sym, 50000)}, // preallocate ~2MB for ABI0 and ~1MB for ABI1 symbols symsByName: [2]map[string]Sym{make(map[string]Sym, 80000), make(map[string]Sym, 50000)}, // preallocate ~2MB for ABI0 and ~1MB for ABI1 symbols
objByPkg: make(map[string]*oReader), objByPkg: make(map[string]*oReader),
outer: make(map[Sym]Sym), outer: make(map[Sym]Sym),
@ -382,9 +381,9 @@ func (l *Loader) addObj(pkg string, r *oReader) Sym {
return i return i
} }
// Add a symbol from an object file, return the global index and whether it is added. // Add a symbol from an object file, return the global index.
// If the symbol already exist, it returns the index of that symbol. // If the symbol already exist, it returns the index of that symbol.
func (l *Loader) addSym(name string, ver int, r *oReader, li uint32, kind int, osym *goobj2.Sym) (Sym, bool) { func (l *Loader) addSym(name string, ver int, r *oReader, li uint32, kind int, osym *goobj2.Sym) Sym {
if l.extStart != 0 { if l.extStart != 0 {
panic("addSym called after external symbol is created") panic("addSym called after external symbol is created")
} }
@ -394,14 +393,14 @@ func (l *Loader) addSym(name string, ver int, r *oReader, li uint32, kind int, o
} }
if name == "" && kind != hashed64Def && kind != hashedDef { if name == "" && kind != hashed64Def && kind != hashedDef {
addToGlobal() addToGlobal()
return i, true // unnamed aux symbol return i // unnamed aux symbol
} }
if ver == r.version { if ver == r.version {
// Static symbol. Add its global index but don't // Static symbol. Add its global index but don't
// add to name lookup table, as it cannot be // add to name lookup table, as it cannot be
// referenced by name. // referenced by name.
addToGlobal() addToGlobal()
return i, true return i
} }
switch kind { switch kind {
case pkgDef: case pkgDef:
@ -412,33 +411,32 @@ func (l *Loader) addSym(name string, ver int, r *oReader, li uint32, kind int, o
// referenced by name (e.g. through linkname). // referenced by name (e.g. through linkname).
l.symsByName[ver][name] = i l.symsByName[ver][name] = i
addToGlobal() addToGlobal()
return i, true return i
case hashed64Def, hashedDef: case hashed64Def, hashedDef:
// Hashed (content-addressable) symbol. Check the hash // Hashed (content-addressable) symbol. Check the hash
// but don't add to name lookup table, as they are not // but don't add to name lookup table, as they are not
// referenced by name. Also no need to do overwriting // referenced by name. Also no need to do overwriting
// check, as same hash indicates same content. // check, as same hash indicates same content.
var checkHash func() (symSizeAlign, bool) var checkHash func() (symAndSize, bool)
var addToHashMap func(symSizeAlign) var addToHashMap func(symAndSize)
var h64 uint64 // only used for hashed64Def var h64 uint64 // only used for hashed64Def
var h *goobj2.HashType // only used for hashedDef var h *goobj2.HashType // only used for hashedDef
if kind == hashed64Def { if kind == hashed64Def {
checkHash = func() (symSizeAlign, bool) { checkHash = func() (symAndSize, bool) {
h64 = r.Hash64(li - uint32(r.ndef)) h64 = r.Hash64(li - uint32(r.ndef))
s, existed := l.hashed64Syms[h64] s, existed := l.hashed64Syms[h64]
return s, existed return s, existed
} }
addToHashMap = func(ss symSizeAlign) { l.hashed64Syms[h64] = ss } addToHashMap = func(ss symAndSize) { l.hashed64Syms[h64] = ss }
} else { } else {
checkHash = func() (symSizeAlign, bool) { checkHash = func() (symAndSize, bool) {
h = r.Hash(li - uint32(r.ndef+r.nhashed64def)) h = r.Hash(li - uint32(r.ndef+r.nhashed64def))
s, existed := l.hashedSyms[*h] s, existed := l.hashedSyms[*h]
return s, existed return s, existed
} }
addToHashMap = func(ss symSizeAlign) { l.hashedSyms[*h] = ss } addToHashMap = func(ss symAndSize) { l.hashedSyms[*h] = ss }
} }
siz := osym.Siz() siz := osym.Siz()
align := osym.Align()
if s, existed := checkHash(); existed { if s, existed := checkHash(); existed {
// The content hash is built from symbol data and relocations. In the // The content hash is built from symbol data and relocations. In the
// object file, the symbol data may not always contain trailing zeros, // object file, the symbol data may not always contain trailing zeros,
@ -449,25 +447,16 @@ func (l *Loader) addSym(name string, ver int, r *oReader, li uint32, kind int, o
// hash("A") == hash("A\0\0\0"). // hash("A") == hash("A\0\0\0").
// So when two symbols have the same hash, we need to use the one with // So when two symbols have the same hash, we need to use the one with
// larger size. // larger size.
if siz <= s.size { if siz > s.size {
if align > s.align { // we need to use the biggest alignment
l.SetSymAlign(s.sym, int32(align))
addToHashMap(symSizeAlign{s.sym, s.size, align})
}
} else {
// New symbol has larger size, use the new one. Rewrite the index mapping. // New symbol has larger size, use the new one. Rewrite the index mapping.
l.objSyms[s.sym] = objSym{r.objidx, li} l.objSyms[s.sym] = objSym{r.objidx, li}
if align < s.align { addToHashMap(symAndSize{s.sym, siz})
align = s.align // keep the biggest alignment
l.SetSymAlign(s.sym, int32(align))
}
addToHashMap(symSizeAlign{s.sym, siz, align})
} }
return s.sym, false return s.sym
} }
addToHashMap(symSizeAlign{i, siz, align}) addToHashMap(symAndSize{i, siz})
addToGlobal() addToGlobal()
return i, true return i
} }
// Non-package (named) symbol. Check if it already exists. // Non-package (named) symbol. Check if it already exists.
@ -475,19 +464,19 @@ func (l *Loader) addSym(name string, ver int, r *oReader, li uint32, kind int, o
if !existed { if !existed {
l.symsByName[ver][name] = i l.symsByName[ver][name] = i
addToGlobal() addToGlobal()
return i, true return i
} }
// symbol already exists // symbol already exists
if osym.Dupok() { if osym.Dupok() {
if l.flags&FlagStrictDups != 0 { if l.flags&FlagStrictDups != 0 {
l.checkdup(name, r, li, oldi) l.checkdup(name, r, li, oldi)
} }
return oldi, false return oldi
} }
oldr, oldli := l.toLocal(oldi) oldr, oldli := l.toLocal(oldi)
oldsym := oldr.Sym(oldli) oldsym := oldr.Sym(oldli)
if oldsym.Dupok() { if oldsym.Dupok() {
return oldi, false return oldi
} }
overwrite := r.DataSize(li) != 0 overwrite := r.DataSize(li) != 0
if overwrite { if overwrite {
@ -504,7 +493,7 @@ func (l *Loader) addSym(name string, ver int, r *oReader, li uint32, kind int, o
log.Fatalf("duplicated definition of symbol " + name) log.Fatalf("duplicated definition of symbol " + name)
} }
} }
return oldi, true return oldi
} }
// newExtSym creates a new external sym with the specified // newExtSym creates a new external sym with the specified
@ -2113,11 +2102,8 @@ func (l *Loader) preloadSyms(r *oReader, kind int) {
} }
v = abiToVer(osym.ABI(), r.version) v = abiToVer(osym.ABI(), r.version)
} }
gi, added := l.addSym(name, v, r, i, kind, osym) gi := l.addSym(name, v, r, i, kind, osym)
r.syms[i] = gi r.syms[i] = gi
if !added {
continue
}
if osym.TopFrame() { if osym.TopFrame() {
l.SetAttrTopFrame(gi, true) l.SetAttrTopFrame(gi, true)
} }
@ -2137,8 +2123,8 @@ func (l *Loader) preloadSyms(r *oReader, kind int) {
l.builtinSyms[bi] = gi l.builtinSyms[bi] = gi
} }
} }
if a := osym.Align(); a != 0 { if a := int32(osym.Align()); a != 0 && a > l.SymAlign(gi) {
l.SetSymAlign(gi, int32(a)) l.SetSymAlign(gi, a)
} }
} }
} }

View file

@ -21,11 +21,7 @@ import (
// data or relocations). // data or relocations).
func addDummyObjSym(t *testing.T, ldr *Loader, or *oReader, name string) Sym { func addDummyObjSym(t *testing.T, ldr *Loader, or *oReader, name string) Sym {
idx := uint32(len(ldr.objSyms)) idx := uint32(len(ldr.objSyms))
s, ok := ldr.addSym(name, 0, or, idx, nonPkgDef, &goobj2.Sym{}) return ldr.addSym(name, 0, or, idx, nonPkgDef, &goobj2.Sym{})
if !ok {
t.Errorf("AddrSym failed for '" + name + "'")
}
return s
} }
func mkLoader() *Loader { func mkLoader() *Loader {