cmd/link: process is_stmt data into dwarf line tables

To improve debugging, instructions should be annotated with
DWARF is_stmt.  The DWARF default before was is_stmt=1, and
to remove "jumpy" stepping the optimizer was tagging
instructions with a no-position position, which interferes
with the accuracy of profiling information.  This allows
that to be corrected, and also allows more "jumpy" positions
to be annotated with is_stmt=0 (these changes were not made
for 1.10 because of worries about further messing up
profiling).

The is_stmt values are placed in a pc-encoded table and
passed through a symbol derived from the name of the
function and processed in the linker alongside its
processing of each function's pc/line tables.

The only change in binary size is in the .debug_line tables
measured with "objdump -h --section=.debug_line go1.test"
For go1.test, these are 2614 bytes larger,
or 0.72% of the size of .debug_line,
or 0.025% of the file size.

This will increase in proportion to how much the is_stmt
flag is used (toggled).

Change-Id: Ic1f1aeccff44591ad0494d29e1a0202a3c506a7a
Reviewed-on: https://go-review.googlesource.com/93664
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>

diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go
index 7bf4fb227a6ad887f9a679d745999dfdf97ab51c..e9271149a176240f3ab33087c16959ec407a2974 100644 (file)
--- a/src/cmd/compile/internal/gc/pgen.go
+++ b/src/cmd/compile/internal/gc/pgen.go
@@ -265,7 +265,7 @@ func compileSSA(fn *Node, worker int) {
        }
        pp := newProgs(fn, worker)
        genssa(f, pp)
-       pp.Flush()
+       pp.Flush() // assemble, fill in boilerplate, etc.
        // fieldtrack must be called after pp.Flush. See issue 20014.
        fieldtrack(pp.Text.From.Sym, fn.Func.FieldTrack)
        pp.Free()

diff --git a/src/cmd/compile/internal/ssa/testdata/hist.dlv-dbg.nexts b/src/cmd/compile/internal/ssa/testdata/hist.dlv-dbg.nexts
index ec79b77de277c8dc9742a62a15c99e2cf139b1de..9c70bb587a912c42df76e9830b3ac66e3ad6a3fc 100644 (file)
--- a/src/cmd/compile/internal/ssa/testdata/hist.dlv-dbg.nexts
+++ b/src/cmd/compile/internal/ssa/testdata/hist.dlv-dbg.nexts
@@ -96,4 +96,5 @@
 87:                    if a == 0 { //gdb-opt=(a,n,t)
 88:                            continue
 86:            for i, a := range hist {
+92:                    fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
 98:    }

diff --git a/src/cmd/compile/internal/ssa/testdata/hist.dlv-opt.nexts b/src/cmd/compile/internal/ssa/testdata/hist.dlv-opt.nexts
index a7bcbb1ade307675bd0bc2e0fc381ed17e65d103..988e3938f80721f557eae5eba476fe05f5214696 100644 (file)
--- a/src/cmd/compile/internal/ssa/testdata/hist.dlv-opt.nexts
+++ b/src/cmd/compile/internal/ssa/testdata/hist.dlv-opt.nexts
@@ -97,4 +97,5 @@
 86:            for i, a := range hist {
 92:                    fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
 87:                    if a == 0 { //gdb-opt=(a,n,t)
+92:                    fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
 98:    }

diff --git a/src/cmd/internal/dwarf/dwarf.go b/src/cmd/internal/dwarf/dwarf.go
index 303499db7c165d778e54da835ee16821581d40d9..37fdba585a3297f375a480a78ca259195d630358 100644 (file)
--- a/src/cmd/internal/dwarf/dwarf.go
+++ b/src/cmd/internal/dwarf/dwarf.go
@@ -23,6 +23,9 @@ const LocPrefix = "go.loc."
 // RangePrefix is the prefix for all the symbols containing DWARF range lists.
 const RangePrefix = "go.range."
 
+// IsStmtPrefix is the prefix for all the symbols containing DWARF is_stmt info for the line number table.
+const IsStmtPrefix = "go.isstmt."
+
 // ConstInfoPrefix is the prefix for all symbols containing DWARF info
 // entries that contain constants.
 const ConstInfoPrefix = "go.constinfo."

diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go
index a3a9c9ffe8d62d5efffefdc8e9ffdd1bc9449b58..16e4e1410d582452d3d89ed68d265184254920f0 100644 (file)
--- a/src/cmd/internal/obj/link.go
+++ b/src/cmd/internal/obj/link.go
@@ -395,6 +395,7 @@ type FuncInfo struct {
        dwarfLocSym    *LSym
        dwarfRangesSym *LSym
        dwarfAbsFnSym  *LSym
+       dwarfIsStmtSym *LSym
 
        GCArgs   LSym
        GCLocals LSym

diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go
index b5f5790a5033b7de149ec04902baddbdcbc9fe46..91b48b5e08d277b8ebf739c282c76f45dbf02a87 100644 (file)
--- a/src/cmd/internal/obj/objfile.go
+++ b/src/cmd/internal/obj/objfile.go
@@ -497,7 +497,7 @@ func (c dwCtxt) Logf(format string, args ...interface{}) {
        c.Link.Logf(format, args...)
 }
 
-func (ctxt *Link) dwarfSym(s *LSym) (dwarfInfoSym, dwarfLocSym, dwarfRangesSym, dwarfAbsFnSym *LSym) {
+func (ctxt *Link) dwarfSym(s *LSym) (dwarfInfoSym, dwarfLocSym, dwarfRangesSym, dwarfAbsFnSym, dwarfIsStmtSym *LSym) {
        if s.Type != objabi.STEXT {
                ctxt.Diag("dwarfSym of non-TEXT %v", s)
        }
@@ -510,9 +510,10 @@ func (ctxt *Link) dwarfSym(s *LSym) (dwarfInfoSym, dwarfLocSym, dwarfRangesSym,
                if s.WasInlined() {
                        s.Func.dwarfAbsFnSym = ctxt.DwFixups.AbsFuncDwarfSym(s)
                }
+               s.Func.dwarfIsStmtSym = ctxt.LookupDerived(s, dwarf.IsStmtPrefix+s.Name)
 
        }
-       return s.Func.dwarfInfoSym, s.Func.dwarfLocSym, s.Func.dwarfRangesSym, s.Func.dwarfAbsFnSym
+       return s.Func.dwarfInfoSym, s.Func.dwarfLocSym, s.Func.dwarfRangesSym, s.Func.dwarfAbsFnSym, s.Func.dwarfIsStmtSym
 }
 
 func (s *LSym) Len() int64 {
@@ -536,13 +537,14 @@ func (ctxt *Link) fileSymbol(fn *LSym) *LSym {
 // TEXT symbol 's'. The various DWARF symbols must already have been
 // initialized in InitTextSym.
 func (ctxt *Link) populateDWARF(curfn interface{}, s *LSym, myimportpath string) {
-       info, loc, ranges, absfunc := ctxt.dwarfSym(s)
+       info, loc, ranges, absfunc, _ := ctxt.dwarfSym(s)
        if info.Size != 0 {
                ctxt.Diag("makeFuncDebugEntry double process %v", s)
        }
        var scopes []dwarf.Scope
        var inlcalls dwarf.InlCalls
        if ctxt.DebugInfo != nil {
+               stmtData(ctxt, s)
                scopes, inlcalls = ctxt.DebugInfo(s, curfn)
        }
        var err error

diff --git a/src/cmd/internal/obj/pcln.go b/src/cmd/internal/obj/pcln.go
index d1d36cf685c42611a831620289c045ca1ecec14f..1d5831d3cb64c348a6d95e7af80d3c139e52256f 100644 (file)
--- a/src/cmd/internal/obj/pcln.go
+++ b/src/cmd/internal/obj/pcln.go
@@ -4,7 +4,10 @@
 
 package obj
 
-import "log"
+import (
+       "cmd/internal/src"
+       "log"
+)
 
 func addvarint(d *Pcdata, v uint32) {
        for ; v >= 0x80; v >>= 7 {
@@ -230,6 +233,25 @@ func pctospadj(ctxt *Link, sym *LSym, oldval int32, p *Prog, phase int32, arg in
        return oldval + p.Spadj
 }
 
+// pctostmt returns either,
+// if phase==0, then whether the current instruction is a step-target (Dwarf is_stmt)
+// else (phase == 1), zero.
+//
+func pctostmt(ctxt *Link, sym *LSym, oldval int32, p *Prog, phase int32, arg interface{}) int32 {
+       if phase == 1 {
+               return 0 // Ignored; also different from initial value of -1, if that ever matters.
+       }
+       s := p.Pos.IsStmt()
+       if s == src.PosIsStmt {
+               return 1
+       }
+       if s == src.PosNotStmt { // includes NoSrcPos case
+               return 0
+       }
+       // Line numbers in .s files will have no special setting, therefore default to is_stmt=1.
+       return 1
+}
+
 // pctopcdata computes the pcdata value in effect at p.
 // A PCDATA instruction sets the value in effect at future
 // non-PCDATA instructions.
@@ -248,6 +270,13 @@ func pctopcdata(ctxt *Link, sym *LSym, oldval int32, p *Prog, phase int32, arg i
        return int32(p.To.Offset)
 }
 
+// stmtData writes out pc-linked is_stmt data for eventual use in the DWARF line numbering table.
+func stmtData(ctxt *Link, cursym *LSym) {
+       var pctostmtData Pcdata
+       funcpctab(ctxt, &pctostmtData, cursym, "pctostmt", pctostmt, nil)
+       cursym.Func.dwarfIsStmtSym.P = pctostmtData.P
+}
+
 func linkpcln(ctxt *Link, cursym *LSym) {
        pcln := &cursym.Func.Pcln
 

diff --git a/src/cmd/internal/obj/plist.go b/src/cmd/internal/obj/plist.go
index e2609da35da6eea47a722ece19a3e2544e4b3d3d..8e7040477488231b543ee7237e8d9edb830f5afd 100644 (file)
--- a/src/cmd/internal/obj/plist.go
+++ b/src/cmd/internal/obj/plist.go
@@ -136,7 +136,7 @@ func (ctxt *Link) InitTextSym(s *LSym, flag int) {
        ctxt.Text = append(ctxt.Text, s)
 
        // Set up DWARF entries for s.
-       info, loc, ranges, _ := ctxt.dwarfSym(s)
+       info, loc, ranges, _, isstmt := ctxt.dwarfSym(s)
        info.Type = objabi.SDWARFINFO
        info.Set(AttrDuplicateOK, s.DuplicateOK())
        if loc != nil {
@@ -147,6 +147,9 @@ func (ctxt *Link) InitTextSym(s *LSym, flag int) {
        ranges.Type = objabi.SDWARFRANGE
        ranges.Set(AttrDuplicateOK, s.DuplicateOK())
        ctxt.Data = append(ctxt.Data, info, ranges)
+       isstmt.Type = objabi.SDWARFMISC
+       isstmt.Set(AttrDuplicateOK, s.DuplicateOK())
+       ctxt.Data = append(ctxt.Data, isstmt)
 
        // Set up the function's gcargs and gclocals.
        // They will be filled in later if needed.

diff --git a/src/cmd/internal/objabi/symkind.go b/src/cmd/internal/objabi/symkind.go
index ea180d0bf862c64a3686282f029225f64a48b7fa..b95a0d3c701fdf69562224aeda27a1bcbb454e31 100644 (file)
--- a/src/cmd/internal/objabi/symkind.go
+++ b/src/cmd/internal/objabi/symkind.go
@@ -34,6 +34,7 @@ package objabi
 type SymKind uint8
 
 // Defined SymKind values.
+// These are used to index into cmd/link/internal/sym/AbiSymKindToSymKind
 //
 // TODO(rsc): Give idiomatic Go names.
 //go:generate stringer -type=SymKind
@@ -58,4 +59,7 @@ const (
        SDWARFINFO
        SDWARFRANGE
        SDWARFLOC
+       SDWARFMISC
+       // Update cmd/link/internal/sym/AbiSymKindToSymKind for new SymKind values.
+
 )

diff --git a/src/cmd/link/internal/ld/dwarf.go b/src/cmd/link/internal/ld/dwarf.go
index cd71ed35152ab3fc0cfcdfd6dae33d4544621887..5dedcc19cae01de89bcefa47991fd2c45105d1b8 100644 (file)
--- a/src/cmd/link/internal/ld/dwarf.go
+++ b/src/cmd/link/internal/ld/dwarf.go
@@ -1113,6 +1113,7 @@ func collectAbstractFunctions(ctxt *Link, fn *sym.Symbol, dsym *sym.Symbol, absf
 func writelines(ctxt *Link, lib *sym.Library, textp []*sym.Symbol, ls *sym.Symbol) (dwinfo *dwarf.DWDie, funcs []*sym.Symbol, absfuncs []*sym.Symbol) {
 
        var dwarfctxt dwarf.Context = dwctxt{ctxt}
+       is_stmt := uint8(1) // initially = recommended default_is_stmt = 1, tracks is_stmt toggles.
 
        unitstart := int64(-1)
        headerstart := int64(-1)
@@ -1154,7 +1155,7 @@ func writelines(ctxt *Link, lib *sym.Library, textp []*sym.Symbol, ls *sym.Symbo
 
        // cpos == unitstart + 4 + 2 + 4
        ls.AddUint8(1)                // minimum_instruction_length
-       ls.AddUint8(1)                // default_is_stmt
+       ls.AddUint8(is_stmt)          // default_is_stmt
        ls.AddUint8(LINE_BASE & 0xFF) // line_base
        ls.AddUint8(LINE_RANGE)       // line_range
        ls.AddUint8(OPCODE_BASE)      // opcode_base
@@ -1172,7 +1173,7 @@ func writelines(ctxt *Link, lib *sym.Library, textp []*sym.Symbol, ls *sym.Symbo
        // Create the file table. fileNums maps from global file
        // indexes (created by numberfile) to CU-local indexes.
        fileNums := make(map[int]int)
-       for _, s := range textp {
+       for _, s := range textp { // textp has been dead-code-eliminated already.
                for _, f := range s.FuncInfo.File {
                        if _, ok := fileNums[int(f.Value)]; ok {
                                continue
@@ -1224,27 +1225,25 @@ func writelines(ctxt *Link, lib *sym.Library, textp []*sym.Symbol, ls *sym.Symbo
 
        var pcfile Pciter
        var pcline Pciter
-       for _, s := range textp {
+       var pcstmt Pciter
+       for i, s := range textp {
                dsym := ctxt.Syms.Lookup(dwarf.InfoPrefix+s.Name, int(s.Version))
                funcs = append(funcs, dsym)
                absfuncs = collectAbstractFunctions(ctxt, s, dsym, absfuncs)
 
                finddebugruntimepath(s)
 
+               isStmtsSym := ctxt.Syms.ROLookup(dwarf.IsStmtPrefix+s.Name, int(s.Version))
+               pctostmtData := sym.Pcdata{P: isStmtsSym.P}
+
                pciterinit(ctxt, &pcfile, &s.FuncInfo.Pcfile)
                pciterinit(ctxt, &pcline, &s.FuncInfo.Pcline)
-               epc := pc
-               for pcfile.done == 0 && pcline.done == 0 {
-                       if epc-s.Value >= int64(pcfile.nextpc) {
-                               pciternext(&pcfile)
-                               continue
-                       }
-
-                       if epc-s.Value >= int64(pcline.nextpc) {
-                               pciternext(&pcline)
-                               continue
-                       }
+               pciterinit(ctxt, &pcstmt, &pctostmtData)
 
+               var thispc uint32
+               // TODO this loop looks like it could exit with work remaining.
+               for pcfile.done == 0 && pcline.done == 0 && pcstmt.done == 0 {
+                       // Only changed if it advanced
                        if int32(file) != pcfile.value {
                                ls.AddUint8(dwarf.DW_LNS_set_file)
                                idx, ok := fileNums[int(pcfile.value)]
@@ -1255,16 +1254,40 @@ func writelines(ctxt *Link, lib *sym.Library, textp []*sym.Symbol, ls *sym.Symbo
                                file = int(pcfile.value)
                        }
 
-                       putpclcdelta(ctxt, dwarfctxt, ls, uint64(s.Value+int64(pcline.pc)-pc), int64(pcline.value)-int64(line))
+                       // Only changed if it advanced
+                       if is_stmt != uint8(pcstmt.value) {
+                               is_stmt = uint8(pcstmt.value)
+                               ls.AddUint8(uint8(dwarf.DW_LNS_negate_stmt))
+                       }
+
+                       // putpcldelta makes a row in the DWARF matrix, always, even if line is unchanged.
+                       putpclcdelta(ctxt, dwarfctxt, ls, uint64(s.Value+int64(thispc)-pc), int64(pcline.value)-int64(line))
 
-                       pc = s.Value + int64(pcline.pc)
+                       pc = s.Value + int64(thispc)
                        line = int(pcline.value)
-                       if pcfile.nextpc < pcline.nextpc {
-                               epc = int64(pcfile.nextpc)
-                       } else {
-                               epc = int64(pcline.nextpc)
+
+                       // Take the minimum step forward for the three iterators
+                       thispc = pcfile.nextpc
+                       if pcline.nextpc < thispc {
+                               thispc = pcline.nextpc
+                       }
+                       if pcstmt.nextpc < thispc {
+                               thispc = pcstmt.nextpc
+                       }
+
+                       if pcfile.nextpc == thispc {
+                               pciternext(&pcfile)
+                       }
+                       if pcstmt.nextpc == thispc {
+                               pciternext(&pcstmt)
                        }
-                       epc += s.Value
+                       if pcline.nextpc == thispc {
+                               pciternext(&pcline)
+                       }
+               }
+               if is_stmt == 0 && i < len(textp)-1 {
+                       // If there is more than one function, ensure default value is established.
+                       ls.AddUint8(uint8(dwarf.DW_LNS_negate_stmt))
                }
        }
 

diff --git a/src/cmd/link/internal/sym/symkind.go b/src/cmd/link/internal/sym/symkind.go
index 1c409a673cf2def9fc42745fd1768f31da42a74b..2e21cc1f00a8a9f12c10ea5ce0b7c8fb8bff4d65 100644 (file)
--- a/src/cmd/link/internal/sym/symkind.go
+++ b/src/cmd/link/internal/sym/symkind.go
@@ -101,10 +101,13 @@ const (
        SCONST
        SDYNIMPORT
        SHOSTOBJ
+
+       // Sections for debugging information
        SDWARFSECT
        SDWARFINFO
        SDWARFRANGE
        SDWARFLOC
+       SDWARFMISC // Not really a section; informs/affects other DWARF section generation
 )
 
 // AbiSymKindToSymKind maps values read from object files (which are
@@ -121,6 +124,7 @@ var AbiSymKindToSymKind = [...]SymKind{
        SDWARFINFO,
        SDWARFRANGE,
        SDWARFLOC,
+       SDWARFMISC,
 }
 
 // ReadOnly are the symbol kinds that form read-only sections. In some