cmd/internal/obj: reimplement line history

In addition to possibly being clearer code,
this replaces an O(n) lookup with an O(log n) lookup.

Change-Id: I0a574c536a965a87f7ad6dcdcc30f737bc771cd5
Reviewed-on: https://go-review.googlesource.com/7623Reviewed-by: Rob Pike <r@golang.org>

src/cmd/internal/gc/lex.go

@@ -1544,14 +1544,15 @@ func getlinepragma() int {
 			}
 			cp.WriteByte(byte(c))
 		}
-
 		cp = nil
 
-		if strings.HasPrefix(lexbuf.String(), "go:cgo_") {
-			pragcgo(lexbuf.String())
+		text := lexbuf.String()
+
+		if strings.HasPrefix(text, "go:cgo_") {
+			pragcgo(text)
 		}
 
-		cmd = lexbuf.String()
+		cmd = text
 		verb = cmd
 		if i := strings.Index(verb, " "); i >= 0 {
 			verb = verb[:i]
@@ -1630,8 +1631,9 @@ func getlinepragma() int {
 	if linep == 0 {
 		return c
 	}
+	text := lexbuf.String()
 	n := 0
-	for _, c := range lexbuf.String()[linep:] {
+	for _, c := range text[linep:] {
 		if c < '0' || c > '9' {
 			goto out
 		}
@@ -1646,15 +1648,7 @@ func getlinepragma() int {
 		return c
 	}
 
-	// try to avoid allocating file name over and over
-	name = lexbuf.String()[:linep-1]
-	for h := Ctxt.Hist; h != nil; h = h.Link {
-		if h.Name != "" && h.Name == name {
-			linehist(h.Name, int32(n), 0)
-			return c
-		}
-	}
-
+	name = text[:linep-1]
 	linehist(name, int32(n), 0)
 	return c
 

src/cmd/internal/obj/line_test.go

@@ -11,6 +11,7 @@ import (
 
 func TestLineHist(t *testing.T) {
 	ctxt := new(Link)
+	ctxt.Hash = make(map[SymVer]*LSym)
 
 	Linklinehist(ctxt, 1, "a.c", 0)
 	Linklinehist(ctxt, 3, "a.h", 0)
@@ -22,18 +23,18 @@ func TestLineHist(t *testing.T) {
 
 	var expect = []string{
 		0:  "??:0",
-		1:  "/a.c:1",
-		2:  "/a.c:2",
-		3:  "/a.h:1",
-		4:  "/a.h:2",
-		5:  "/a.c:3",
-		6:  "/a.c:4",
-		7:  "/linedir:2",
-		8:  "/linedir:3",
+		1:  "a.c:1",
+		2:  "a.c:2",
+		3:  "a.h:1",
+		4:  "a.h:2",
+		5:  "a.c:3",
+		6:  "a.c:4",
+		7:  "linedir:2",
+		8:  "linedir:3",
 		9:  "??:0",
 		10: "??:0",
-		11: "/b.c:1",
-		12: "/b.c:2",
+		11: "b.c:1",
+		12: "b.c:2",
 		13: "??:0",
 		14: "??:0",
 	}

src/cmd/internal/obj/link.go

@@ -183,8 +183,8 @@ type Link struct {
 	Hash               map[SymVer]*LSym
 	Allsym             *LSym
 	Nsymbol            int32
-	Hist               *Hist
-	Ehist              *Hist
+	LineHist           LineHist
+	Imports            []string
 	Plist              *Plist
 	Plast              *Plist
 	Sym_div            *LSym
@@ -580,3 +580,19 @@ const (
 )
 
 var linkbasepointer int
+
+/*
+ * start a new Prog list.
+ */
+func Linknewplist(ctxt *Link) *Plist {
+	pl := new(Plist)
+	*pl = Plist{}
+	if ctxt.Plist == nil {
+		ctxt.Plist = pl
+	} else {
+		ctxt.Plast.Link = pl
+	}
+	ctxt.Plast = pl
+
+	return pl
+}

src/cmd/internal/obj/obj.go

@@ -7,107 +7,110 @@ package obj
 import (
 	"fmt"
 	"path/filepath"
+	"sort"
 	"strings"
 )
 
-const (
-	HISTSZ = 10
-	NSYM   = 50
-)
+// A LineHist records the history of the file input stack, which maps the virtual line number,
+// an incrementing count of lines processed in any input file and typically named lineno,
+// to a stack of file:line pairs showing the path of inclusions that led to that position.
+// The first line directive (//line in Go, #line in assembly) is treated as pushing
+// a new entry on the stack, so that errors can report both the actual and translated
+// line number.
+//
+// In typical use, the virtual lineno begins at 1, and file line numbers also begin at 1,
+// but the only requirements placed upon the numbers by this code are:
+//	- calls to Push, Update, and Pop must be monotonically increasing in lineno
+//	- except as specified by those methods, virtual and file line number increase
+//	  together, so that given (only) calls Push(10, "x.go", 1) and Pop(15),
+//	  virtual line 12 corresponds to x.go line 3.
+type LineHist struct {
+	Top            *LineStack  // current top of stack
+	Ranges         []LineRange // ranges for lookup
+	Dir            string      // directory to qualify relative paths
+	TrimPathPrefix string      // remove leading TrimPath from recorded file names
+	GOROOT         string      // current GOROOT
+	GOROOT_FINAL   string      // target GOROOT
+}
 
-type Hist struct {
-	Link    *Hist
-	Name    string
-	Sym     *LSym
-	Line    int32
-	Offset  int32
-	Printed uint8
+// A LineStack is an entry in the recorded line history.
+// Although the history at any given line number is a stack,
+// the record for all line processed forms a tree, with common
+// stack prefixes acting as parents.
+type LineStack struct {
+	Parent    *LineStack // parent in inclusion stack
+	Lineno    int        // virtual line number where this entry takes effect
+	File      string     // file name used to open source file, for error messages
+	AbsFile   string     // absolute file name, for pcln tables
+	FileLine  int        // line number in file at Lineno
+	Directive bool
+	Sym       *LSym // for linkgetline - TODO(rsc): remove
 }
 
-func Linklinefmt(ctxt *Link, lno0 int, showAll, showFullPath bool) string {
-	var a [HISTSZ]struct {
-		incl *Hist
-		idel int32
-		line *Hist
-		ldel int32
-	}
-	lno := int32(lno0)
-	lno1 := lno
-	var d int32
-	n := 0
-	for h := ctxt.Hist; h != nil; h = h.Link {
-		if h.Offset < 0 {
-			continue
-		}
-		if lno < h.Line {
-			break
-		}
-		if h.Name != "<pop>" {
-			if h.Offset > 0 {
-				// #line directive
-				if n > 0 && n < int(HISTSZ) {
-					a[n-1].line = h
-					a[n-1].ldel = h.Line - h.Offset + 1
+func (stk *LineStack) fileLineAt(lineno int) int {
+	return stk.FileLine + lineno - stk.Lineno
+}
+
+// The span of valid linenos in the recorded line history can be broken
+// into a set of ranges, each with a particular stack.
+// A LineRange records one such range.
+type LineRange struct {
+	Start int        // starting lineno
+	Stack *LineStack // top of stack for this range
+}
+
+// startRange starts a new range with the given top of stack.
+func (h *LineHist) startRange(lineno int, top *LineStack) {
+	h.Top = top
+	h.Ranges = append(h.Ranges, LineRange{top.Lineno, top})
+}
+
+// setFile sets stk.File = file and also derives stk.AbsFile.
+func (h *LineHist) setFile(stk *LineStack, file string) {
+	// Note: The exclusion of stk.Directive may be wrong but matches what we've done before.
+	// The check for < avoids putting a path prefix on "<autogenerated>".
+	abs := file
+	if h.Dir != "" && !filepath.IsAbs(file) && !strings.HasPrefix(file, "<") && !stk.Directive {
+		abs = filepath.Join(h.Dir, file)
 	}
+
+	// Remove leading TrimPathPrefix, or else rewrite $GOROOT to $GOROOT_FINAL.
+	if h.TrimPathPrefix != "" && hasPathPrefix(abs, h.TrimPathPrefix) {
+		if abs == h.TrimPathPrefix {
+			abs = ""
 		} else {
-				// beginning of file
-				if n < int(HISTSZ) {
-					a[n].incl = h
-					a[n].idel = h.Line
-					a[n].line = nil
-				}
-				n++
-			}
-			continue
-		}
-		n--
-		if n > 0 && n < int(HISTSZ) {
-			d = h.Line - a[n].incl.Line
-			a[n-1].ldel += d
-			a[n-1].idel += d
-		}
-	}
-	if n > int(HISTSZ) {
-		n = int(HISTSZ)
+			abs = abs[len(h.TrimPathPrefix)+1:]
 		}
-	var fp string
-	for i := n - 1; i >= 0; i-- {
-		if i != n-1 {
-			if !showAll {
-				break
+	} else if h.GOROOT_FINAL != "" && h.GOROOT_FINAL != h.GOROOT && hasPathPrefix(abs, h.GOROOT) {
+		abs = h.GOROOT_FINAL + abs[len(h.GOROOT):]
 	}
-			fp += " "
+	if abs == "" {
+		abs = "??"
 	}
-		if ctxt.Debugline != 0 || showFullPath {
-			fp += fmt.Sprintf("%s/", ctxt.Pathname)
-		}
-		if a[i].line != nil {
-			fp += fmt.Sprintf("%s:%d[%s:%d]", a[i].line.Name, lno-a[i].ldel+1, a[i].incl.Name, lno-a[i].idel+1)
-		} else {
-			fp += fmt.Sprintf("%s:%d", a[i].incl.Name, lno-a[i].idel+1)
-		}
-		lno = a[i].incl.Line - 1 // now print out start of this file
-	}
-	if n == 0 {
-		fp += fmt.Sprintf("<unknown line number %d %d %d %s>", lno1, ctxt.Hist.Offset, ctxt.Hist.Line, ctxt.Hist.Name)
+	abs = filepath.Clean(abs)
+	stk.AbsFile = abs
+
+	if file == "" {
+		file = "??"
 	}
-	return fp
+	stk.File = file
 }
 
 // Does s have t as a path prefix?
 // That is, does s == t or does s begin with t followed by a slash?
-// For portability, we allow ASCII case folding, so that haspathprefix("a/b/c", "A/B") is true.
-// Similarly, we allow slash folding, so that haspathprefix("a/b/c", "a\\b") is true.
-func haspathprefix(s string, t string) bool {
+// For portability, we allow ASCII case folding, so that hasPathPrefix("a/b/c", "A/B") is true.
+// Similarly, we allow slash folding, so that hasPathPrefix("a/b/c", "a\\b") is true.
+// We do not allow full Unicode case folding, for fear of causing more confusion
+// or harm than good. (For an example of the kinds of things that can go wrong,
+// see http://article.gmane.org/gmane.linux.kernel/1853266.)
+func hasPathPrefix(s string, t string) bool {
 	if len(t) > len(s) {
 		return false
 	}
 	var i int
-	var cs int
-	var ct int
 	for i = 0; i < len(t); i++ {
-		cs = int(s[i])
-		ct = int(t[i])
+		cs := int(s[i])
+		ct := int(t[i])
 		if 'A' <= cs && cs <= 'Z' {
 			cs += 'a' - 'A'
 		}
@@ -127,191 +130,187 @@ func haspathprefix(s string, t string) bool {
 	return i >= len(s) || s[i] == '/' || s[i] == '\\'
 }
 
-// This is a simplified copy of linklinefmt above.
-// It doesn't allow printing the full stack, and it returns the file name and line number separately.
-// TODO: Unify with linklinefmt somehow.
-func linkgetline(ctxt *Link, line int32, f **LSym, l *int32) {
-	var a [HISTSZ]struct {
-		incl *Hist
-		idel int32
-		line *Hist
-		ldel int32
-	}
-	var d int32
-	lno := int32(line)
-	n := 0
-	for h := ctxt.Hist; h != nil; h = h.Link {
-		if h.Offset < 0 {
-			continue
-		}
-		if lno < h.Line {
-			break
-		}
-		if h.Name != "<pop>" {
-			if h.Offset > 0 {
-				// #line directive
-				if n > 0 && n < HISTSZ {
-					a[n-1].line = h
-					a[n-1].ldel = h.Line - h.Offset + 1
-				}
-			} else {
-				// beginning of file
-				if n < HISTSZ {
-					a[n].incl = h
-					a[n].idel = h.Line
-					a[n].line = nil
-				}
-				n++
-			}
-			continue
-		}
-		n--
-		if n > 0 && n < HISTSZ {
-			d = h.Line - a[n].incl.Line
-			a[n-1].ldel += d
-			a[n-1].idel += d
-		}
-	}
-	if n > HISTSZ {
-		n = HISTSZ
-	}
-	if n <= 0 {
-		*f = Linklookup(ctxt, "??", HistVersion)
-		*l = 0
+// Push records that at that lineno a new file with the given name was pushed onto the input stack.
+func (h *LineHist) Push(lineno int, file string) {
+	stk := &LineStack{
+		Parent:   h.Top,
+		Lineno:   lineno,
+		FileLine: 1,
+	}
+	h.setFile(stk, file)
+	h.startRange(lineno, stk)
+}
+
+// Pop records that at lineno the current file was popped from the input stack.
+func (h *LineHist) Pop(lineno int) {
+	top := h.Top
+	if top == nil {
 		return
 	}
-	n--
-	var dlno int32
-	var file string
-	var sym *LSym
-	if a[n].line != nil {
-		file = a[n].line.Name
-		sym = a[n].line.Sym
-		dlno = a[n].ldel - 1
-	} else {
-		file = a[n].incl.Name
-		sym = a[n].incl.Sym
-		dlno = a[n].idel - 1
-	}
-	if sym == nil {
-		var buf string
-		if filepath.IsAbs(file) || strings.HasPrefix(file, "<") {
-			buf = file
-		} else {
-			buf = ctxt.Pathname + "/" + file
-		}
-		// Remove leading ctxt->trimpath, or else rewrite $GOROOT to $GOROOT_FINAL.
-		if ctxt.Trimpath != "" && haspathprefix(buf, ctxt.Trimpath) {
-			if len(buf) == len(ctxt.Trimpath) {
-				buf = "??"
-			} else {
-				buf1 := buf[len(ctxt.Trimpath)+1:]
-				if buf1[0] == '\x00' {
-					buf1 = "??"
-				}
-				buf = buf1
-			}
-		} else if ctxt.Goroot_final != "" && haspathprefix(buf, ctxt.Goroot) {
-			buf1 := fmt.Sprintf("%s%s", ctxt.Goroot_final, buf[len(ctxt.Goroot):])
-			buf = buf1
-		}
-		sym = Linklookup(ctxt, buf, HistVersion)
-		if a[n].line != nil {
-			a[n].line.Sym = sym
-		} else {
-			a[n].incl.Sym = sym
+	if top.Directive && top.Parent != nil { // pop #line level too
+		top = top.Parent
 	}
+	next := top.Parent
+	if next == nil {
+		h.Top = nil
+		h.Ranges = append(h.Ranges, LineRange{lineno, nil})
+		return
 	}
-	lno -= dlno
-	*f = sym
-	*l = lno
+
+	// Popping included file. Update parent offset to account for
+	// the virtual line number range taken by the included file.
+	// Cannot modify the LineStack directly, or else lookups
+	// for the earlier line numbers will get the wrong answers,
+	// so make a new one.
+	stk := new(LineStack)
+	*stk = *next
+	stk.Lineno = lineno
+	stk.FileLine = next.fileLineAt(top.Lineno)
+	h.startRange(lineno, stk)
 }
 
-func Linklinehist(ctxt *Link, lineno int, f string, offset int) {
-	if false { // debug['f']
-		if f != "" {
-			if offset != 0 {
-				fmt.Printf("%4d: %s (#line %d)\n", lineno, f, offset)
+// Update records that at lineno the file name and line number were changed using
+// a line directive (//line in Go, #line in assembly).
+func (h *LineHist) Update(lineno int, file string, line int) {
+	top := h.Top
+	if top == nil {
+		return // shouldn't happen
+	}
+	var stk *LineStack
+	if top.Directive {
+		// Update existing entry, except make copy to avoid changing earlier history.
+		stk = new(LineStack)
+		*stk = *top
 	} else {
-				fmt.Printf("%4d: %s\n", lineno, f)
+		// Push new entry.
+		stk = &LineStack{
+			Parent:    top,
+			Directive: true,
 		}
-		} else {
-			fmt.Printf("%4d: <pop>\n", lineno)
 	}
+	stk.Lineno = lineno
+	if stk.File != file {
+		h.setFile(stk, file) // only retain string if needed
 	}
+	stk.FileLine = line
+	h.startRange(lineno, stk)
+}
 
-	h := new(Hist)
-	*h = Hist{}
-	h.Name = f
-	h.Line = int32(lineno)
-	h.Offset = int32(offset)
-	h.Link = nil
-	if ctxt.Ehist == nil {
-		ctxt.Hist = h
-		ctxt.Ehist = h
-		return
-	}
+// AddImport adds a package to the list of imported packages.
+func (ctxt *Link) AddImport(pkg string) {
+	ctxt.Imports = append(ctxt.Imports, pkg)
+}
 
-	ctxt.Ehist.Link = h
-	ctxt.Ehist = h
+// At returns the input stack in effect at lineno.
+func (h *LineHist) At(lineno int) *LineStack {
+	i := sort.Search(len(h.Ranges), func(i int) bool {
+		return h.Ranges[i].Start > lineno
+	})
+	// Found first entry beyond lineno.
+	if i == 0 {
+		return nil
+	}
+	return h.Ranges[i-1].Stack
 }
 
-func Linkprfile(ctxt *Link, line int) {
-	l := int32(line)
-	var i int
-	var a [HISTSZ]Hist
-	var d int32
-	n := 0
-	for h := ctxt.Hist; h != nil; h = h.Link {
-		if l < h.Line {
-			break
-		}
-		if h.Name != "<pop>" {
-			if h.Offset == 0 {
-				if n >= 0 && n < HISTSZ {
-					a[n] = *h
-				}
-				n++
-				continue
+// LineString returns a string giving the file and line number
+// corresponding to lineno, for use in error messages.
+func (h *LineHist) LineString(lineno int) string {
+	stk := h.At(lineno)
+	if stk == nil {
+		return "<unknown line number>"
 	}
-			if n > 0 && n < HISTSZ {
-				if a[n-1].Offset == 0 {
-					a[n] = *h
-					n++
-				} else {
-					a[n-1] = *h
-				}
-			}
-			continue
+
+	text := fmt.Sprintf("%s:%d", stk.File, stk.fileLineAt(lineno))
+	if stk.Directive && stk.Parent != nil {
+		stk = stk.Parent
+		text += fmt.Sprintf("[%s:%d]", stk.File, stk.fileLineAt(lineno))
 	}
-		n--
-		if n >= 0 && n < HISTSZ {
-			d = h.Line - a[n].Line
-			for i = 0; i < n; i++ {
-				a[i].Line += d
+	const showFullStack = false // was used by old C compilers
+	if showFullStack {
+		for stk.Parent != nil {
+			lineno = stk.Lineno - 1
+			stk = stk.Parent
+			text += fmt.Sprintf(" %s:%d", stk.File, stk.fileLineAt(lineno))
+			if stk.Directive && stk.Parent != nil {
+				stk = stk.Parent
+				text += fmt.Sprintf("[%s:%d]", stk.File, stk.fileLineAt(lineno))
 			}
 		}
 	}
-	if n > HISTSZ {
-		n = HISTSZ
+	return text
+}
+
+// TODO(rsc): Replace call sites with use of ctxt.LineHist.
+// Note that all call sites use showAll=false, showFullPath=false.
+func Linklinefmt(ctxt *Link, lineno int, showAll, showFullPath bool) string {
+	return ctxt.LineHist.LineString(lineno)
+}
+
+// FileLine returns the file name and line number
+// at the top of the stack for the given lineno.
+func (h *LineHist) FileLine(lineno int) (file string, line int) {
+	stk := h.At(lineno)
+	if stk == nil {
+		return "??", 0
+	}
+	return stk.File, stk.fileLineAt(lineno)
+}
+
+// AbsFileLine returns the absolute file name and line number
+// at the top of the stack for the given lineno.
+func (h *LineHist) AbsFileLine(lineno int) (file string, line int) {
+	stk := h.At(lineno)
+	if stk == nil {
+		return "??", 0
+	}
+	return stk.AbsFile, stk.fileLineAt(lineno)
+}
+
+// This is a simplified copy of linklinefmt above.
+// It doesn't allow printing the full stack, and it returns the file name and line number separately.
+// TODO: Unify with linklinefmt somehow.
+func linkgetline(ctxt *Link, lineno int32, f **LSym, l *int32) {
+	stk := ctxt.LineHist.At(int(lineno))
+	if stk == nil || stk.AbsFile == "" {
+		*f = Linklookup(ctxt, "??", HistVersion)
+		*l = 0
+		return
 	}
-	for i := 0; i < n; i++ {
-		fmt.Printf("%s:%d ", a[i].Name, int(l-a[i].Line+a[i].Offset+1))
+	if stk.Sym == nil {
+		stk.Sym = Linklookup(ctxt, stk.AbsFile, HistVersion)
 	}
+	*f = stk.Sym
+	*l = int32(stk.fileLineAt(int(lineno)))
 }
 
-/*
- * start a new Prog list.
- */
-func Linknewplist(ctxt *Link) *Plist {
-	pl := new(Plist)
-	*pl = Plist{}
-	if ctxt.Plist == nil {
-		ctxt.Plist = pl
-	} else {
-		ctxt.Plast.Link = pl
-	}
-	ctxt.Plast = pl
+func Linkprfile(ctxt *Link, line int) {
+	fmt.Printf("%s ", ctxt.LineHist.LineString(line))
+}
+
+// Linklinehist pushes, amends, or pops an entry on the line history stack.
+// If f != "<pop>" and n == 0, the call pushes the start of a new file named f at lineno.
+// If f != "<pop>" and n > 0, the call amends the top of the stack to record that lineno
+// now corresponds to f at line n.
+// If f == "<pop>", the call pops the topmost entry from the stack, picking up
+// the parent file at the line following the one where the corresponding push occurred.
+//
+// If n < 0, linklinehist records f as a package required by the current compilation
+// (nothing to do with line numbers).
+//
+// TODO(rsc): Replace uses with direct calls to ctxt.Hist methods.
+func Linklinehist(ctxt *Link, lineno int, f string, n int) {
+	switch {
+	case n < 0:
+		ctxt.AddImport(f)
+
+	case f == "<pop>":
+		ctxt.LineHist.Pop(lineno)
 
-	return pl
+	case n == 0:
+		ctxt.LineHist.Push(lineno, f)
+
+	default:
+		ctxt.LineHist.Update(lineno, f, n)
+	}
 }

src/cmd/internal/obj/objfile.go

@@ -306,10 +306,8 @@ func Writeobjdirect(ctxt *Link, b *Biobuf) {
 	Bputc(b, 1) // version
 
 	// Emit autolib.
-	for h := ctxt.Hist; h != nil; h = h.Link {
-		if h.Offset < 0 {
-			wrstring(b, h.Name)
-		}
+	for _, pkg := range ctxt.Imports {
+		wrstring(b, pkg)
 	}
 	wrstring(b, "")
 

src/cmd/internal/obj/sym.go

@@ -142,9 +142,12 @@ func Linknew(arch *LinkArch) *Link {
 		buf = "/???"
 	}
 	buf = filepath.ToSlash(buf)
-
 	ctxt.Pathname = buf
 
+	ctxt.LineHist.GOROOT = ctxt.Goroot
+	ctxt.LineHist.GOROOT_FINAL = ctxt.Goroot_final
+	ctxt.LineHist.Dir = ctxt.Pathname
+
 	ctxt.Headtype = headtype(Getgoos())
 	if ctxt.Headtype < 0 {
 		log.Fatalf("unknown goos %s", Getgoos())