]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.link] cmd/link: remove holes from global index space
authorCherry Zhang <cherryyz@google.com>
Mon, 3 Feb 2020 17:37:35 +0000 (12:37 -0500)
committerCherry Zhang <cherryyz@google.com>
Wed, 5 Feb 2020 20:53:18 +0000 (20:53 +0000)
In CL 217064, we made symbol's global index unique, but we still
reserve index space for each object file, which means we may
leave holes in the index space if the symbol is a dup or is
overwritten. In this CL, we stop reserving index spaces. Instead,
symbols are added one at a time, and only added if it does not
already exist. There is no more holes in the index space.

Change-Id: I3c4e67163c556ba1198e13065706510dac4692fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/217519
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
src/cmd/link/internal/ld/deadcode2.go
src/cmd/link/internal/loader/loader.go
src/cmd/link/internal/loader/loader_test.go

index 3d3a03215e50f67bc85e25ef5830986f443fbb82..992b1c206bc7830a070c56a8b206574d8cbf547e 100644 (file)
@@ -58,9 +58,7 @@ func (d *deadcodePass2) init() {
                n := d.ldr.NDef()
                for i := 1; i < n; i++ {
                        s := loader.Sym(i)
-                       if !d.ldr.IsDup(s) {
-                               d.mark(s, 0)
-                       }
+                       d.mark(s, 0)
                }
                return
        }
index 1fd8c8d94a26d5663d9d1c95419d800c179149f6..40bae9cc6d20972a685928ffc873a0f09303d1a6 100644 (file)
@@ -67,7 +67,6 @@ type oReader struct {
 type objIdx struct {
        r *oReader
        i Sym // start index
-       e Sym // end index
 }
 
 // objSym represents a symbol in an object file. It is a tuple of
@@ -130,9 +129,8 @@ func growBitmap(reqLen int, b bitmap) bitmap {
 // TODO: rework index space reservation.
 //
 // - Go object files are read before host object files; each Go object
-//   read allocates a new chunk of global index space of size P + NP,
-//   where P is the number of package defined symbols in the object and
-//   NP is the number of non-package defined symbols.
+//   read adds its defined (package + non-package) symbols to the global
+//   index space.
 //
 // - In loader.LoadRefs(), the loader makes a sweep through all of the
 //   non-package references in each object file and allocates sym indices
@@ -161,9 +159,6 @@ func growBitmap(reqLen int, b bitmap) bitmap {
 // - Each symbol gets a unique global index. For duplicated and
 //   overwriting/overwritten symbols, the second (or later) appearance
 //   of the symbol gets the same global index as the first appearance.
-//   This means, currently, there may be holes in the index space --
-//   the index reserved for a duplicated symbol does not actually
-//   point to any symbol.
 type Loader struct {
        start       map[*oReader]Sym // map from object file to its start index
        objs        []objIdx         // sorted by start index (i.e. objIdx.i)
@@ -297,11 +292,6 @@ func NewLoader(flags uint32, elfsetstring elfsetstringFunc) *Loader {
        }
 }
 
-// Return the start index in the global index space for a given object file.
-func (l *Loader) startIndex(r *oReader) Sym {
-       return l.start[r]
-}
-
 // Add object file r, return the start index.
 func (l *Loader) addObj(pkg string, r *oReader) Sym {
        if _, ok := l.start[r]; ok {
@@ -314,68 +304,67 @@ func (l *Loader) addObj(pkg string, r *oReader) Sym {
        n := r.NSym() + r.NNonpkgdef()
        i := l.max + 1
        l.start[r] = i
-       l.objs = append(l.objs, objIdx{r, i, i + Sym(n) - 1})
-       l.max += Sym(n)
-       l.growValues(int(l.max))
+       l.objs = append(l.objs, objIdx{r, i})
+       l.growValues(int(l.max) + n)
        return i
 }
 
-// Add a symbol with a given index, return the global index and whether it is added.
+// Add a symbol from an object file, return the global index and whether it is added.
 // If the symbol already exist, it returns the index of that symbol.
-func (l *Loader) AddSym(name string, ver int, i Sym, r *oReader, li int, dupok bool, typ sym.SymKind) (Sym, bool) {
+func (l *Loader) AddSym(name string, ver int, r *oReader, li int, dupok bool, typ sym.SymKind) (Sym, bool) {
        if l.extStart != 0 {
                panic("AddSym called after AddExtSym is called")
        }
-       if int(i) != len(l.objSyms) {
-               fmt.Println(i, len(l.objSyms), name, ver)
-               panic("XXX AddSym inconsistency")
+       i := Sym(len(l.objSyms))
+       addToGlobal := func() {
+               l.max++
+               l.objSyms = append(l.objSyms, objSym{r, li})
        }
-       l.objSyms = append(l.objSyms, objSym{r, li})
        if name == "" {
+               addToGlobal()
                return i, true // unnamed aux symbol
        }
        if ver == r.version {
                // Static symbol. Add its global index but don't
                // add to name lookup table, as it cannot be
                // referenced by name.
+               addToGlobal()
                return i, true
        }
-       if oldi, ok := l.symsByName[ver][name]; ok {
-               if dupok {
-                       if l.flags&FlagStrictDups != 0 {
-                               l.checkdup(name, i, r, oldi)
-                       }
-                       l.objSyms[i] = objSym{} // nil this out -- this is a hole in the index space
-                       return oldi, false
-               }
-               oldr, oldli := l.toLocal(oldi)
-               oldsym := goobj2.Sym{}
-               oldsym.Read(oldr.Reader, oldr.SymOff(oldli))
-               if oldsym.Dupok() {
-                       l.objSyms[i] = objSym{} // nil this out -- this is a hole in the index space
-                       return oldi, false
-               }
-               overwrite := r.DataSize(li) != 0
-               if overwrite {
-                       // new symbol overwrites old symbol.
-                       oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type)]
-                       if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) {
-                               log.Fatalf("duplicated definition of symbol " + name)
-                       }
-                       l.objSyms[oldi] = objSym{r, li}
-                       l.objSyms[i] = objSym{} // nil this out -- this is a hole in the index space
-                       return oldi, true
-               } else {
-                       // old symbol overwrites new symbol.
-                       if !typ.IsData() { // only allow overwriting data symbol
-                               log.Fatalf("duplicated definition of symbol " + name)
-                       }
-                       l.objSyms[i] = objSym{} // nil this out -- this is a hole in the index space
-                       return oldi, false
+       oldi, existed := l.symsByName[ver][name]
+       if !existed {
+               l.symsByName[ver][name] = i
+               addToGlobal()
+               return i, true
+       }
+       // symbol already exists
+       if dupok {
+               if l.flags&FlagStrictDups != 0 {
+                       l.checkdup(name, r, li, oldi)
+               }
+               return oldi, false
+       }
+       oldr, oldli := l.toLocal(oldi)
+       oldsym := goobj2.Sym{}
+       oldsym.Read(oldr.Reader, oldr.SymOff(oldli))
+       if oldsym.Dupok() {
+               return oldi, false
+       }
+       overwrite := r.DataSize(li) != 0
+       if overwrite {
+               // new symbol overwrites old symbol.
+               oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type)]
+               if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) {
+                       log.Fatalf("duplicated definition of symbol " + name)
+               }
+               l.objSyms[oldi] = objSym{r, li}
+       } else {
+               // old symbol overwrites new symbol.
+               if !typ.IsData() { // only allow overwriting data symbol
+                       log.Fatalf("duplicated definition of symbol " + name)
                }
        }
-       l.symsByName[ver][name] = i
-       return i, true
+       return oldi, true
 }
 
 // newExtSym creates a new external sym with the specified
@@ -551,17 +540,8 @@ func (l *Loader) Lookup(name string, ver int) Sym {
        return l.symsByName[ver][name]
 }
 
-// Returns whether i is a dup of another symbol, and i is not
-// "primary", i.e. i is a hole in the global index space.
-// TODO: get rid of the holes.
-func (l *Loader) IsDup(i Sym) bool {
-       r, _ := l.toLocal(i)
-       return r == nil
-}
-
 // Check that duplicate symbols have same contents.
-func (l *Loader) checkdup(name string, i Sym, r *oReader, dup Sym) {
-       li := int(i - l.startIndex(r))
+func (l *Loader) checkdup(name string, r *oReader, li int, dup Sym) {
        p := r.Data(li)
        if strings.HasPrefix(name, "go.info.") {
                p, _ = patchDWARFName1(p, r)
@@ -1503,7 +1483,6 @@ func (l *Loader) Preload(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib *
        }
 
        istart := l.addObj(lib.Pkg, or)
-
        l.growAttrBitmaps(int(istart) + ndef + nnonpkgdef)
        for i, n := 0, ndef+nnonpkgdef; i < n; i++ {
                osym := goobj2.Sym{}
@@ -1511,7 +1490,7 @@ func (l *Loader) Preload(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib *
                name := strings.Replace(osym.Name, "\"\".", pkgprefix, -1)
                v := abiToVer(osym.ABI, localSymVersion)
                dupok := osym.Dupok()
-               gi, added := l.AddSym(name, v, istart+Sym(i), or, i, dupok, sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type)])
+               gi, added := l.AddSym(name, v, or, i, dupok, sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type)])
                or.syms[i] = gi
                if !added {
                        continue
@@ -1973,7 +1952,6 @@ func (l *Loader) CreateExtSym(name string) Sym {
 }
 
 func loadObjFull(l *Loader, r *oReader) {
-       istart := l.startIndex(r)
        lib := r.unit.Lib
        resolveSymRef := func(s goobj2.SymRef) *sym.Symbol {
                i := l.resolve(r, s)
@@ -1986,38 +1964,39 @@ func loadObjFull(l *Loader, r *oReader) {
        pcdataBase := r.PcdataBase()
        rslice := []Reloc{}
        for i, n := 0, r.NSym()+r.NNonpkgdef(); i < n; i++ {
+               // A symbol may be a dup or overwritten. In this case, its
+               // content will actually be provided by a different object
+               // (to which its global index points). Skip those symbols.
+               gi := l.toGlobal(r, i)
+               var isdup bool
+               if r2, i2 := l.toLocal(gi); r2 != r || i2 != i {
+                       isdup = true
+               }
+
                osym := goobj2.Sym{}
                osym.Read(r.Reader, r.SymOff(i))
                name := strings.Replace(osym.Name, "\"\".", r.pkgprefix, -1)
                if name == "" {
                        continue
                }
-               ver := abiToVer(osym.ABI, r.version)
                dupok := osym.Dupok()
-               if dupok {
-                       if dupsym := l.symsByName[ver][name]; dupsym != istart+Sym(i) {
-                               if l.attrReachable.has(dupsym) {
-                                       // A dupok symbol is resolved to another package. We still need
-                                       // to record its presence in the current package, as the trampoline
-                                       // pass expects packages are laid out in dependency order.
-                                       s := l.Syms[dupsym]
-                                       if s.Type == sym.STEXT {
-                                               lib.DupTextSyms = append(lib.DupTextSyms, s)
-                                               lib.DupTextSyms2 = append(lib.DupTextSyms2, sym.LoaderSym(dupsym))
-                                       }
+               if dupok && isdup {
+                       if l.attrReachable.has(gi) {
+                               // A dupok symbol is resolved to another package. We still need
+                               // to record its presence in the current package, as the trampoline
+                               // pass expects packages are laid out in dependency order.
+                               s := l.Syms[gi]
+                               if s.Type == sym.STEXT {
+                                       lib.DupTextSyms = append(lib.DupTextSyms, s)
+                                       lib.DupTextSyms2 = append(lib.DupTextSyms2, sym.LoaderSym(gi))
                                }
-                               continue
                        }
+                       continue
                }
 
-               // A symbol may be a dup or overwritten. In this case, its
-               // content will actually be provided by a different object
-               // (to which its global index points). Skip those symbols.
-               gi := l.toGlobal(r, i)
-               if r2, i2 := l.toLocal(gi); r2 != r || i2 != i {
+               if isdup {
                        continue // come from a different object
                }
-
                s := l.Syms[gi]
                if s == nil {
                        continue
@@ -2301,9 +2280,6 @@ func (l *Loader) UndefinedRelocTargets(limit int) []Sym {
        result := []Sym{}
        rslice := []Reloc{}
        for si := Sym(1); si <= l.max; si++ {
-               if l.IsDup(si) {
-                       continue
-               }
                relocs := l.Relocs(si)
                rslice = relocs.ReadAll(rslice)
                for ri := 0; ri < relocs.Count; ri++ {
@@ -2342,10 +2318,6 @@ func (l *Loader) Dump() {
                if s != nil {
                        fmt.Println(i, s, s.Type, pi)
                } else {
-                       if l.IsDup(i) {
-                               fmt.Println(i, "<overwritten>")
-                               continue
-                       }
                        fmt.Println(i, l.SymName(i), "<not loaded>", pi)
                }
        }
index 71036b3a0abbb0e1d7ef56d1024c26e515ed7d53..8f0678397777cb2bc8b71724f4c149d047c8ee10 100644 (file)
@@ -20,8 +20,7 @@ import (
 // data or relocations).
 func addDummyObjSym(t *testing.T, ldr *Loader, or *oReader, name string) Sym {
        idx := ldr.max + 1
-       ldr.max++
-       if _, ok := ldr.AddSym(name, 0, idx, or, int(idx-ldr.startIndex(or)), false, sym.SRODATA); !ok {
+       if _, ok := ldr.AddSym(name, 0, or, int(idx), false, sym.SRODATA); !ok {
                t.Errorf("AddrSym failed for '" + name + "'")
        }