cmd/link: more idiomatic object reader

name       old secs    new secs    delta
LinkCmdGo   0.52 ± 3%   0.52 ± 7%    ~     (p=0.325 n=93+100)

name       old MaxRSS  new MaxRSS  delta
LinkCmdGo   120k ± 1%   118k ± 4%  -1.10%   (p=0.000 n=87+96)

Change-Id: I967660b8dc6036d28eeea1b6b30f400fadd57b05
Reviewed-on: https://go-review.googlesource.com/21372
Run-TryBot: Shahar Kohanim <skohanim@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

src/cmd/link/internal/ld/lib.go

@@ -1347,7 +1347,7 @@ func ldobj(f *obj.Biobuf, pkg string, length int64, pn string, file string, when
 	ldpkg(f, pkg, import1-import0-2, pn, whence) // -2 for !\n
 	obj.Bseek(f, import1, 0)
 
-	ldobjfile(Ctxt, f, pkg, eof-obj.Boffset(f), pn)
+	LoadObjFile(Ctxt, f, pkg, eof-obj.Boffset(f), pn)
 	return nil
 }
 

src/cmd/link/internal/ld/link.go

@@ -189,7 +189,6 @@ type Link struct {
 	Filesyms   *LSym
 	Moduledata *LSym
 	LSymBatch  []LSym
-	CurRefs    []*LSym // List of symbol references for the file being read.
 }
 
 // The smallest possible offset from the hardware stack pointer to a local

src/cmd/link/internal/ld/objfile.go

@@ -108,8 +108,10 @@ package ld
 //	- There are SymID in the object file that should really just be strings.
 
 import (
+	"bufio"
 	"bytes"
 	"cmd/internal/obj"
+	"io"
 	"log"
 	"strconv"
 	"strings"
@@ -120,70 +122,21 @@ const (
 	endmagic   = "\xff\xffgo13ld"
 )
 
-func ldobjfile(ctxt *Link, f *obj.Biobuf, pkg string, length int64, pn string) {
+var emptyPkg = []byte(`"".`)
-	start := obj.Boffset(f)
-	ctxt.IncVersion()
-	var buf [8]uint8
-	obj.Bread(f, buf[:])
-	if string(buf[:]) != startmagic {
-		log.Fatalf("%s: invalid file start %x %x %x %x %x %x %x %x", pn, buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7])
-	}
-	c := obj.Bgetc(f)
-	if c != 1 {
-		log.Fatalf("%s: invalid file version number %d", pn, c)
-	}
 
-	var lib string
+// objReader reads Go object files.
-	for {
+type objReader struct {
-		lib = rdstring(f)
+	rd   *bufio.Reader
-		if lib == "" {
+	ctxt *Link
-			break
+	pkg  string
-		}
+	pn   string
-		addlib(ctxt, pkg, pn, lib)
+	// List of symbol references for the file being read.
-	}
+	dupSym *LSym
 
-	ctxt.CurRefs = []*LSym{nil} // zeroth ref is nil
+	// rdBuf is used by readString and readSymName as scratch for reading strings.
-	for {
+	rdBuf []byte
-		c, err := f.Peek(1)
-		if err != nil {
-			log.Fatalf("%s: peeking: %v", pn, err)
-		}
-		if c[0] == 0xff {
-			obj.Bgetc(f)
-			break
-		}
-		readref(ctxt, f, pkg, pn)
-	}
 
-	sl := rdslices(f)
+	refs        []*LSym
-
-	obj.Bread(f, sl.data)
-
-	for {
-		c, err := f.Peek(1)
-		if err != nil {
-			log.Fatalf("%s: peeking: %v", pn, err)
-		}
-		if c[0] == 0xff {
-			break
-		}
-		readsym(ctxt, f, sl, pkg, pn)
-	}
-
-	buf = [8]uint8{}
-	obj.Bread(f, buf[:])
-	if string(buf[:]) != endmagic {
-		log.Fatalf("%s: invalid file end", pn)
-	}
-
-	if obj.Boffset(f) != start+length {
-		log.Fatalf("%s: unexpected end at %d, want %d", pn, int64(obj.Boffset(f)), int64(start+length))
-	}
-}
-
-var dupSym = &LSym{Name: ".dup"}
-
-type slices struct {
 	data        []byte
 	reloc       []Reloc
 	pcdata      []Pcdata
@@ -193,38 +146,119 @@ type slices struct {
 	file        []*LSym
 }
 
-func rdslices(f *obj.Biobuf) *slices {
+func LoadObjFile(ctxt *Link, f *obj.Biobuf, pkg string, length int64, pn string) {
-	sl := &slices{}
+	start := obj.Boffset(f)
-
+	r := &objReader{
-	n := rdint(f)
+		rd:     f.Reader(),
-	sl.data = make([]byte, n)
+		pkg:    pkg,
-	n = rdint(f)
+		ctxt:   ctxt,
-	sl.reloc = make([]Reloc, n)
+		pn:     pn,
-	n = rdint(f)
+		dupSym: &LSym{Name: ".dup"},
-	sl.pcdata = make([]Pcdata, n)
+	}
-	n = rdint(f)
+	r.loadObjFile()
-	sl.autom = make([]Auto, n)
+	if obj.Boffset(f) != start+length {
-	n = rdint(f)
+		log.Fatalf("%s: unexpected end at %d, want %d", pn, int64(obj.Boffset(f)), int64(start+length))
-	sl.funcdata = make([]*LSym, n)
+	}
-	sl.funcdataoff = make([]int64, n)
-	n = rdint(f)
-	sl.file = make([]*LSym, n)
-	return sl
 }
 
-func readsym(ctxt *Link, f *obj.Biobuf, sl *slices, pkg string, pn string) {
+func (r *objReader) loadObjFile() {
-	if obj.Bgetc(f) != 0xfe {
+	// Increment context version, versions are used to differentiate static files in different packages
-		log.Fatalln("readsym out of sync")
+	r.ctxt.IncVersion()
+
+	// Magic header
+	var buf [8]uint8
+	r.readFull(buf[:])
+	if string(buf[:]) != startmagic {
+		log.Fatalf("%s: invalid file start %x %x %x %x %x %x %x %x", r.pn, buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6], buf[7])
 	}
-	t := rdint(f)
+
-	s := rdsym(ctxt, f, pkg)
+	// Version
-	flags := rdint(f)
+	c, err := r.rd.ReadByte()
+	if err != nil || c != 1 {
+		log.Fatalf("%s: invalid file version number %d", r.pn, c)
+	}
+
+	// Autolib
+	for {
+		lib := r.readString()
+		if lib == "" {
+			break
+		}
+		addlib(r.ctxt, r.pkg, r.pn, lib)
+	}
+
+	// Symbol references
+	r.refs = []*LSym{nil} // zeroth ref is nil
+	for {
+		c, err := r.rd.Peek(1)
+		if err != nil {
+			log.Fatalf("%s: peeking: %v", r.pn, err)
+		}
+		if c[0] == 0xff {
+			r.rd.ReadByte()
+			break
+		}
+		r.readRef()
+	}
+
+	// Lengths
+	r.readSlices()
+
+	// Data section
+	r.readFull(r.data)
+
+	// Defined symbols
+	for {
+		c, err := r.rd.Peek(1)
+		if err != nil {
+			log.Fatalf("%s: peeking: %v", r.pn, err)
+		}
+		if c[0] == 0xff {
+			break
+		}
+		r.readSym()
+	}
+
+	// Magic footer
+	buf = [8]uint8{}
+	r.readFull(buf[:])
+	if string(buf[:]) != endmagic {
+		log.Fatalf("%s: invalid file end", r.pn)
+	}
+}
+
+func (r *objReader) readSlices() {
+	n := r.readInt()
+	r.data = make([]byte, n)
+	n = r.readInt()
+	r.reloc = make([]Reloc, n)
+	n = r.readInt()
+	r.pcdata = make([]Pcdata, n)
+	n = r.readInt()
+	r.autom = make([]Auto, n)
+	n = r.readInt()
+	r.funcdata = make([]*LSym, n)
+	r.funcdataoff = make([]int64, n)
+	n = r.readInt()
+	r.file = make([]*LSym, n)
+}
+
+// Symbols are prefixed so their content doesn't get confused with the magic footer.
+const symPrefix = 0xfe
+
+func (r *objReader) readSym() {
+	if c, err := r.rd.ReadByte(); c != symPrefix || err != nil {
+		log.Fatalln("readSym out of sync")
+	}
+	t := r.readInt()
+	s := r.readSymIndex()
+	flags := r.readInt()
 	dupok := flags&1 != 0
 	local := flags&2 != 0
-	size := rdint(f)
+	size := r.readInt()
-	typ := rdsym(ctxt, f, pkg)
+	typ := r.readSymIndex()
-	data := rddata(f, &sl.data)
+	data := r.readData()
-	nreloc := rdint(f)
+	nreloc := r.readInt()
 	isdup := false
 
 	var dup *LSym
@@ -243,17 +277,17 @@ func readsym(ctxt *Link, f *obj.Biobuf, sl *slices, pkg string, pn string) {
 			goto overwrite
 		}
 		if s.Type != obj.SBSS && s.Type != obj.SNOPTRBSS && !dupok && !s.Attr.DuplicateOK() {
-			log.Fatalf("duplicate symbol %s (types %d and %d) in %s and %s", s.Name, s.Type, t, s.File, pn)
+			log.Fatalf("duplicate symbol %s (types %d and %d) in %s and %s", s.Name, s.Type, t, s.File, r.pn)
 		}
 		if len(s.P) > 0 {
 			dup = s
-			s = dupSym
+			s = r.dupSym
 			isdup = true
 		}
 	}
 
 overwrite:
-	s.File = pkg
+	s.File = r.pkg
 	if dupok {
 		s.Attr |= AttrDuplicateOK
 	}
@@ -261,7 +295,7 @@ overwrite:
 		log.Fatalf("bad sxref")
 	}
 	if t == 0 {
-		log.Fatalf("missing type for %s in %s", s.Name, pn)
+		log.Fatalf("missing type for %s in %s", s.Name, r.pn)
 	}
 	if t == obj.SBSS && (s.Type == obj.SRODATA || s.Type == obj.SNOPTRBSS) {
 		t = int(s.Type)
@@ -279,80 +313,80 @@ overwrite:
 	}
 	s.P = data
 	if nreloc > 0 {
-		s.R = sl.reloc[:nreloc:nreloc]
+		s.R = r.reloc[:nreloc:nreloc]
 		if !isdup {
-			sl.reloc = sl.reloc[nreloc:]
+			r.reloc = r.reloc[nreloc:]
 		}
 
-		var r *Reloc
 		for i := 0; i < nreloc; i++ {
-			r = &s.R[i]
+			s.R[i] = Reloc{
-			r.Off = rdint32(f)
+				Off:  r.readInt32(),
-			r.Siz = rduint8(f)
+				Siz:  r.readUint8(),
-			r.Type = rdint32(f)
+				Type: r.readInt32(),
-			r.Add = rdint64(f)
+				Add:  r.readInt64(),
-			r.Sym = rdsym(ctxt, f, pkg)
+				Sym:  r.readSymIndex(),
+			}
 		}
 	}
 
 	if s.Type == obj.STEXT {
-		s.Args = rdint32(f)
+		s.Args = r.readInt32()
-		s.Locals = rdint32(f)
+		s.Locals = r.readInt32()
-		if rduint8(f) != 0 {
+		if r.readUint8() != 0 {
 			s.Attr |= AttrNoSplit
 		}
-		flags := rdint(f)
+		flags := r.readInt()
 		if flags&(1<<2) != 0 {
 			s.Attr |= AttrReflectMethod
 		}
-		n := rdint(f)
+		n := r.readInt()
-		s.Autom = sl.autom[:n:n]
+		s.Autom = r.autom[:n:n]
 		if !isdup {
-			sl.autom = sl.autom[n:]
+			r.autom = r.autom[n:]
 		}
 
 		for i := 0; i < n; i++ {
 			s.Autom[i] = Auto{
-				Asym:    rdsym(ctxt, f, pkg),
+				Asym:    r.readSymIndex(),
-				Aoffset: rdint32(f),
+				Aoffset: r.readInt32(),
-				Name:    rdint16(f),
+				Name:    r.readInt16(),
-				Gotype:  rdsym(ctxt, f, pkg),
+				Gotype:  r.readSymIndex(),
 			}
 		}
 
 		s.Pcln = new(Pcln)
 		pc := s.Pcln
-		pc.Pcsp.P = rddata(f, &sl.data)
+		pc.Pcsp.P = r.readData()
-		pc.Pcfile.P = rddata(f, &sl.data)
+		pc.Pcfile.P = r.readData()
-		pc.Pcline.P = rddata(f, &sl.data)
+		pc.Pcline.P = r.readData()
-		n = rdint(f)
+		n = r.readInt()
-		pc.Pcdata = sl.pcdata[:n:n]
+		pc.Pcdata = r.pcdata[:n:n]
 		if !isdup {
-			sl.pcdata = sl.pcdata[n:]
+			r.pcdata = r.pcdata[n:]
 		}
 		for i := 0; i < n; i++ {
-			pc.Pcdata[i].P = rddata(f, &sl.data)
+			pc.Pcdata[i].P = r.readData()
 		}
-		n = rdint(f)
+		n = r.readInt()
-		pc.Funcdata = sl.funcdata[:n:n]
+		pc.Funcdata = r.funcdata[:n:n]
-		pc.Funcdataoff = sl.funcdataoff[:n:n]
+		pc.Funcdataoff = r.funcdataoff[:n:n]
 		if !isdup {
-			sl.funcdata = sl.funcdata[n:]
+			r.funcdata = r.funcdata[n:]
-			sl.funcdataoff = sl.funcdataoff[n:]
+			r.funcdataoff = r.funcdataoff[n:]
 		}
 		for i := 0; i < n; i++ {
-			pc.Funcdata[i] = rdsym(ctxt, f, pkg)
+			pc.Funcdata[i] = r.readSymIndex()
 		}
 		for i := 0; i < n; i++ {
-			pc.Funcdataoff[i] = rdint64(f)
+			pc.Funcdataoff[i] = r.readInt64()
 		}
-		n = rdint(f)
+		n = r.readInt()
-		pc.File = sl.file[:n:n]
+		pc.File = r.file[:n:n]
 		if !isdup {
-			sl.file = sl.file[n:]
+			r.file = r.file[n:]
 		}
 		for i := 0; i < n; i++ {
-			pc.File[i] = rdsym(ctxt, f, pkg)
+			pc.File[i] = r.readSymIndex()
 		}
 
 		if !isdup {
@@ -360,30 +394,37 @@ overwrite:
 				log.Fatalf("symbol %s listed multiple times", s.Name)
 			}
 			s.Attr |= AttrOnList
-			if ctxt.Etextp != nil {
+			if r.ctxt.Etextp != nil {
-				ctxt.Etextp.Next = s
+				r.ctxt.Etextp.Next = s
 			} else {
-				ctxt.Textp = s
+				r.ctxt.Textp = s
 			}
-			ctxt.Etextp = s
+			r.ctxt.Etextp = s
 		}
 	}
 }
 
-func readref(ctxt *Link, f *obj.Biobuf, pkg string, pn string) {
+func (r *objReader) readFull(b []byte) {
-	if obj.Bgetc(f) != 0xfe {
+	_, err := io.ReadFull(r.rd, b)
-		log.Fatalf("readsym out of sync")
+	if err != nil {
+		log.Fatalf("%s: error reading %s", r.pn, err)
 	}
-	name := rdsymName(f, pkg)
+}
-	v := rdint(f)
+
+func (r *objReader) readRef() {
+	if c, err := r.rd.ReadByte(); c != symPrefix || err != nil {
+		log.Fatalf("readSym out of sync")
+	}
+	name := r.readSymName()
+	v := r.readInt()
 	if v != 0 && v != 1 {
 		log.Fatalf("invalid symbol version %d", v)
 	}
 	if v == 1 {
-		v = ctxt.Version
+		v = r.ctxt.Version
 	}
-	s := Linklookup(ctxt, name, v)
+	s := Linklookup(r.ctxt, name, v)
-	ctxt.CurRefs = append(ctxt.CurRefs, s)
+	r.refs = append(r.refs, s)
 
 	if s == nil || v != 0 {
 		return
@@ -400,9 +441,9 @@ func readref(ctxt *Link, f *obj.Biobuf, pkg string, pn string) {
 			if uint64(uint32(x)) != x {
 				log.Panicf("$-symbol %s too large: %d", s.Name, x)
 			}
-			Adduint32(ctxt, s, uint32(x))
+			Adduint32(r.ctxt, s, uint32(x))
 		case "$f64.", "$i64.":
-			Adduint64(ctxt, s, x)
+			Adduint64(r.ctxt, s, x)
 		default:
 			log.Panicf("unrecognized $-symbol: %s", s.Name)
 		}
@@ -413,14 +454,13 @@ func readref(ctxt *Link, f *obj.Biobuf, pkg string, pn string) {
 	}
 }
 
-func rdint64(f *obj.Biobuf) int64 {
+func (r *objReader) readInt64() int64 {
-	r := f.Reader()
 	uv := uint64(0)
 	for shift := uint(0); ; shift += 7 {
 		if shift >= 64 {
 			log.Fatalf("corrupt input")
 		}
-		c, err := r.ReadByte()
+		c, err := r.rd.ReadByte()
 		if err != nil {
 			log.Fatalln("error reading input: ", err)
 		}
@@ -433,63 +473,61 @@ func rdint64(f *obj.Biobuf) int64 {
 	return int64(uv>>1) ^ (int64(uint64(uv)<<63) >> 63)
 }
 
-func rdint(f *obj.Biobuf) int {
+func (r *objReader) readInt() int {
-	n := rdint64(f)
+	n := r.readInt64()
 	if int64(int(n)) != n {
 		log.Panicf("%v out of range for int", n)
 	}
 	return int(n)
 }
 
-func rdint32(f *obj.Biobuf) int32 {
+func (r *objReader) readInt32() int32 {
-	n := rdint64(f)
+	n := r.readInt64()
 	if int64(int32(n)) != n {
 		log.Panicf("%v out of range for int32", n)
 	}
 	return int32(n)
 }
 
-func rdint16(f *obj.Biobuf) int16 {
+func (r *objReader) readInt16() int16 {
-	n := rdint64(f)
+	n := r.readInt64()
 	if int64(int16(n)) != n {
 		log.Panicf("%v out of range for int16", n)
 	}
 	return int16(n)
 }
 
-func rduint8(f *obj.Biobuf) uint8 {
+func (r *objReader) readUint8() uint8 {
-	n := rdint64(f)
+	n := r.readInt64()
 	if int64(uint8(n)) != n {
 		log.Panicf("%v out of range for uint8", n)
 	}
 	return uint8(n)
 }
 
-// rdBuf is used by rdstring and rdsymName as scratch for reading strings.
+func (r *objReader) readString() string {
-var rdBuf []byte
+	n := r.readInt()
-var emptyPkg = []byte(`"".`)
+	if len(r.rdBuf) < n {
-
+		r.rdBuf = make([]byte, n)
-func rdstring(f *obj.Biobuf) string {
-	n := rdint(f)
-	if len(rdBuf) < n {
-		rdBuf = make([]byte, n)
 	}
-	obj.Bread(f, rdBuf[:n])
+	r.readFull(r.rdBuf[:n])
-	return string(rdBuf[:n])
+	return string(r.rdBuf[:n])
 }
 
-func rddata(f *obj.Biobuf, buf *[]byte) []byte {
+func (r *objReader) readData() []byte {
-	n := rdint(f)
+	n := r.readInt()
-	p := (*buf)[:n:n]
+	p := r.data[:n:n]
-	*buf = (*buf)[n:]
+	r.data = r.data[n:]
 	return p
 }
 
-// rdsymName reads a symbol name, replacing all "". with pkg.
+// readSymName reads a symbol name, replacing all "". with pkg.
-func rdsymName(f *obj.Biobuf, pkg string) string {
+func (r *objReader) readSymName() string {
-	n := rdint(f)
+	rdBuf := r.rdBuf
+	pkg := r.pkg
+	n := r.readInt()
 	if n == 0 {
-		rdint64(f)
+		r.readInt64()
 		return ""
 	}
 
@@ -497,7 +535,7 @@ func rdsymName(f *obj.Biobuf, pkg string) string {
 		rdBuf = make([]byte, n, 2*n)
 	}
 	origName := rdBuf[:n]
-	obj.Bread(f, origName)
+	r.readFull(origName)
 	adjName := rdBuf[n:n]
 	for {
 		i := bytes.Index(origName, emptyPkg)
@@ -512,12 +550,13 @@ func rdsymName(f *obj.Biobuf, pkg string) string {
 	}
 	name := string(adjName)
 	if len(adjName) > len(rdBuf) {
-		rdBuf = adjName // save the larger buffer for reuse
+		r.rdBuf = adjName // save the larger buffer for reuse
 	}
 	return name
 }
 
-func rdsym(ctxt *Link, f *obj.Biobuf, pkg string) *LSym {
+// Reads the index of a symbol reference and resolves it to a symbol
-	i := rdint(f)
+func (r *objReader) readSymIndex() *LSym {
-	return ctxt.CurRefs[i]
+	i := r.readInt()
+	return r.refs[i]
 }