From 861546543a9a69173d0c6a85bda9e207c2e8d459 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 8 Mar 2015 22:41:48 -0400 Subject: [PATCH] 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/7623 Reviewed-by: Rob Pike --- src/cmd/internal/gc/lex.go | 22 +- src/cmd/internal/obj/line_test.go | 21 +- src/cmd/internal/obj/link.go | 20 +- src/cmd/internal/obj/obj.go | 489 +++++++++++++++--------------- src/cmd/internal/obj/objfile.go | 6 +- src/cmd/internal/obj/sym.go | 5 +- 6 files changed, 287 insertions(+), 276 deletions(-) diff --git a/src/cmd/internal/gc/lex.go b/src/cmd/internal/gc/lex.go index 03e874929d6bd..10964e4913726 100644 --- a/src/cmd/internal/gc/lex.go +++ b/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 diff --git a/src/cmd/internal/obj/line_test.go b/src/cmd/internal/obj/line_test.go index 6e6cc33912c1c..dde5d64e17bd6 100644 --- a/src/cmd/internal/obj/line_test.go +++ b/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", } diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go index caa24e02abfe4..10946348705cb 100644 --- a/src/cmd/internal/obj/link.go +++ b/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 +} diff --git a/src/cmd/internal/obj/obj.go b/src/cmd/internal/obj/obj.go index ac22fd3f57f26..39db2396e7d73 100644 --- a/src/cmd/internal/obj/obj.go +++ b/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 != "" { - 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 - } - } 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) +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 "". + abs := file + if h.Dir != "" && !filepath.IsAbs(file) && !strings.HasPrefix(file, "<") && !stk.Directive { + abs = filepath.Join(h.Dir, file) } - var fp string - for i := n - 1; i >= 0; i-- { - if i != n-1 { - if !showAll { - break - } - fp += " " - } - 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) + + // Remove leading TrimPathPrefix, or else rewrite $GOROOT to $GOROOT_FINAL. + if h.TrimPathPrefix != "" && hasPathPrefix(abs, h.TrimPathPrefix) { + if abs == h.TrimPathPrefix { + abs = "" } else { - fp += fmt.Sprintf("%s:%d", a[i].incl.Name, lno-a[i].idel+1) + abs = abs[len(h.TrimPathPrefix)+1:] } - lno = a[i].incl.Line - 1 // now print out start of this file + } else if h.GOROOT_FINAL != "" && h.GOROOT_FINAL != h.GOROOT && hasPathPrefix(abs, h.GOROOT) { + abs = h.GOROOT_FINAL + abs[len(h.GOROOT):] } - if n == 0 { - fp += fmt.Sprintf("", lno1, ctxt.Hist.Offset, ctxt.Hist.Line, ctxt.Hist.Name) + if abs == "" { + abs = "??" + } + 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 +// 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, } - 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 != "" { - 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 - } + 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 } - if n > HISTSZ { - n = HISTSZ + if top.Directive && top.Parent != nil { // pop #line level too + top = top.Parent } - if n <= 0 { - *f = Linklookup(ctxt, "??", HistVersion) - *l = 0 + next := top.Parent + if next == nil { + h.Top = nil + h.Ranges = append(h.Ranges, LineRange{lineno, 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 + + // 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) +} + +// 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 } - 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 + var stk *LineStack + if top.Directive { + // Update existing entry, except make copy to avoid changing earlier history. + stk = new(LineStack) + *stk = *top + } else { + // Push new entry. + stk = &LineStack{ + Parent: top, + Directive: true, } } - lno -= dlno - *f = sym - *l = lno + stk.Lineno = lineno + if stk.File != file { + h.setFile(stk, file) // only retain string if needed + } + stk.FileLine = line + 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) - } else { - fmt.Printf("%4d: %s\n", lineno, f) - } - } else { - fmt.Printf("%4d: \n", lineno) - } - } +// AddImport adds a package to the list of imported packages. +func (ctxt *Link) AddImport(pkg string) { + ctxt.Imports = append(ctxt.Imports, pkg) +} - 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 +// 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 } - - ctxt.Ehist.Link = h - ctxt.Ehist = h + 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 != "" { - if h.Offset == 0 { - if n >= 0 && n < HISTSZ { - a[n] = *h - } - n++ - continue - } - if n > 0 && n < HISTSZ { - if a[n-1].Offset == 0 { - a[n] = *h - n++ - } else { - a[n-1] = *h - } - } - continue - } - n-- - if n >= 0 && n < HISTSZ { - d = h.Line - a[n].Line - for i = 0; i < n; i++ { - a[i].Line += d +// 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 "" + } + + 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)) + } + 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 } - for i := 0; i < n; i++ { - fmt.Printf("%s:%d ", a[i].Name, int(l-a[i].Line+a[i].Offset+1)) + 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) } -/* - * 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 +// 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 + } + if stk.Sym == nil { + stk.Sym = Linklookup(ctxt, stk.AbsFile, HistVersion) } - ctxt.Plast = pl + *f = stk.Sym + *l = int32(stk.fileLineAt(int(lineno))) +} + +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 != "" and n == 0, the call pushes the start of a new file named f at lineno. +// If f != "" and n > 0, the call amends the top of the stack to record that lineno +// now corresponds to f at line n. +// If f == "", 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 == "": + ctxt.LineHist.Pop(lineno) - return pl + case n == 0: + ctxt.LineHist.Push(lineno, f) + + default: + ctxt.LineHist.Update(lineno, f, n) + } } diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go index e69e246e2c40a..1f6857840dc55 100644 --- a/src/cmd/internal/obj/objfile.go +++ b/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, "") diff --git a/src/cmd/internal/obj/sym.go b/src/cmd/internal/obj/sym.go index 7d9e469da7369..410ed8410528b 100644 --- a/src/cmd/internal/obj/sym.go +++ 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())