Skip to content

Commit 06314b6

Browse files
dr2chasedr2chase
committed
cmd/compile: add explanations to escape-analysis JSON/LSP logging
For 1.15. From the test: {"range":{"start":{"line":7,"character":13},"end":{...},"severity":3,"code":"leaks","source":"go compiler","message":"parameter z leaks to ~r2 with derefs=0","relatedInformation":[ {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: flow: y = z:"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from y = \u003cN\u003e (assign-pair)"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: flow: ~r1 = y:"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":11},"end":{...}},"message":"inlineLoc"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from y.b (dot of pointer)"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":11},"end":{...}},"message":"inlineLoc"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from \u0026y.b (address-of)"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":9},"end":...}},"message":"inlineLoc"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow: from ~r1 = \u003cN\u003e (assign-pair)"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":3},"end":...}},"message":"escflow: flow: ~r2 = ~r1:"}, {"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":3},"end":...}},"message":"escflow: from return (*int)(~r1) (return)"}]} Change-Id: Idf02438801f63e487c35a928cf5a0b6d3cc48674 Reviewed-on: https://go-review.googlesource.com/c/go/+/206658 Run-TryBot: David Chase <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]>
1 parent fced302 commit 06314b6

File tree

3 files changed

+139
-41
lines changed

3 files changed

+139
-41
lines changed

‎src/cmd/compile/internal/gc/escape.go

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package gc
77
import (
88
"cmd/compile/internal/logopt"
99
"cmd/compile/internal/types"
10+
"cmd/internal/src"
1011
"fmt"
1112
"math"
1213
"strings"
@@ -945,7 +946,7 @@ func (k EscHole) note(where *Node, why string) EscHole {
945946
if where == nil || why == "" {
946947
Fatalf("note: missing where/why")
947948
}
948-
if Debug['m'] >= 2 {
949+
if Debug['m'] >= 2 || logopt.Enabled() {
949950
k.notes = &EscNote{
950951
next: k.notes,
951952
where: where,
@@ -1092,10 +1093,16 @@ func (e *Escape) flow(k EscHole, src *EscLocation) {
10921093
return
10931094
}
10941095
if dst.escapes && k.derefs < 0 { // dst = &src
1095-
if Debug['m'] >= 2 {
1096+
if Debug['m'] >= 2 || logopt.Enabled() {
10961097
pos := linestr(src.n.Pos)
1097-
fmt.Printf("%s: %v escapes to heap:\n", pos, src.n)
1098-
e.explainFlow(pos, dst, src, k.derefs, k.notes)
1098+
if Debug['m'] >= 2 {
1099+
fmt.Printf("%s: %v escapes to heap:\n", pos, src.n)
1100+
}
1101+
explanation := e.explainFlow(pos, dst, src, k.derefs, k.notes, []*logopt.LoggedOpt{})
1102+
if logopt.Enabled() {
1103+
logopt.LogOpt(src.n.Pos, "escapes", "escape", e.curfn.funcname(), fmt.Sprintf("%v escapes to heap", src.n), explanation)
1104+
}
1105+
10991106
}
11001107
src.escapes = true
11011108
return
@@ -1187,9 +1194,15 @@ func (e *Escape) walkOne(root *EscLocation, walkgen uint32, enqueue func(*EscLoc
11871194
// that value flow for tagging the function
11881195
// later.
11891196
if l.isName(PPARAM) {
1190-
if Debug['m'] >= 2 && !l.escapes {
1191-
fmt.Printf("%s: parameter %v leaks to %s with derefs=%d:\n", linestr(l.n.Pos), l.n, e.explainLoc(root), base)
1192-
e.explainPath(root, l)
1197+
if (logopt.Enabled() || Debug['m'] >= 2) && !l.escapes {
1198+
if Debug['m'] >= 2 {
1199+
fmt.Printf("%s: parameter %v leaks to %s with derefs=%d:\n", linestr(l.n.Pos), l.n, e.explainLoc(root), base)
1200+
}
1201+
explanation := e.explainPath(root, l)
1202+
if logopt.Enabled() {
1203+
logopt.LogOpt(l.n.Pos, "leak", "escape", e.curfn.funcname(),
1204+
fmt.Sprintf("parameter %v leaks to %s with derefs=%d", l.n, e.explainLoc(root), base), explanation)
1205+
}
11931206
}
11941207
l.leakTo(root, base)
11951208
}
@@ -1198,9 +1211,14 @@ func (e *Escape) walkOne(root *EscLocation, walkgen uint32, enqueue func(*EscLoc
11981211
// outlives it, then l needs to be heap
11991212
// allocated.
12001213
if addressOf && !l.escapes {
1201-
if Debug['m'] >= 2 {
1202-
fmt.Printf("%s: %v escapes to heap:\n", linestr(l.n.Pos), l.n)
1203-
e.explainPath(root, l)
1214+
if logopt.Enabled() || Debug['m'] >= 2 {
1215+
if Debug['m'] >= 2 {
1216+
fmt.Printf("%s: %v escapes to heap:\n", linestr(l.n.Pos), l.n)
1217+
}
1218+
explanation := e.explainPath(root, l)
1219+
if logopt.Enabled() {
1220+
logopt.LogOpt(l.n.Pos, "escape", "escape", e.curfn.funcname(), fmt.Sprintf("%v escapes to heap", l.n), explanation)
1221+
}
12041222
}
12051223
l.escapes = true
12061224
enqueue(l)
@@ -1225,43 +1243,67 @@ func (e *Escape) walkOne(root *EscLocation, walkgen uint32, enqueue func(*EscLoc
12251243
}
12261244

12271245
// explainPath prints an explanation of how src flows to the walk root.
1228-
func (e *Escape) explainPath(root, src *EscLocation) {
1246+
func (e *Escape) explainPath(root, src *EscLocation) []*logopt.LoggedOpt {
12291247
visited := make(map[*EscLocation]bool)
1230-
12311248
pos := linestr(src.n.Pos)
1249+
var explanation []*logopt.LoggedOpt
12321250
for {
12331251
// Prevent infinite loop.
12341252
if visited[src] {
1235-
fmt.Printf("%s: warning: truncated explanation due to assignment cycle; see golang.org/issue/35518\n", pos)
1253+
if Debug['m'] >= 2 {
1254+
fmt.Printf("%s: warning: truncated explanation due to assignment cycle; see golang.org/issue/35518\n", pos)
1255+
}
12361256
break
12371257
}
12381258
visited[src] = true
1239-
12401259
dst := src.dst
12411260
edge := &dst.edges[src.dstEdgeIdx]
12421261
if edge.src != src {
12431262
Fatalf("path inconsistency: %v != %v", edge.src, src)
12441263
}
12451264

1246-
e.explainFlow(pos, dst, src, edge.derefs, edge.notes)
1265+
explanation = e.explainFlow(pos, dst, src, edge.derefs, edge.notes, explanation)
12471266

12481267
if dst == root {
12491268
break
12501269
}
12511270
src = dst
12521271
}
1272+
1273+
return explanation
12531274
}
12541275

1255-
func (e *Escape) explainFlow(pos string, dst, src *EscLocation, derefs int, notes *EscNote) {
1276+
func (e *Escape) explainFlow(pos string, dst, srcloc *EscLocation, derefs int, notes *EscNote, explanation []*logopt.LoggedOpt) []*logopt.LoggedOpt {
12561277
ops := "&"
12571278
if derefs >= 0 {
12581279
ops = strings.Repeat("*", derefs)
12591280
}
1281+
print := Debug['m'] >= 2
1282+
1283+
flow := fmt.Sprintf(" flow: %s = %s%v:", e.explainLoc(dst), ops, e.explainLoc(srcloc))
1284+
if print {
1285+
fmt.Printf("%s:%s\n", pos, flow)
1286+
}
1287+
if logopt.Enabled() {
1288+
var epos src.XPos
1289+
if notes != nil {
1290+
epos = notes.where.Pos
1291+
} else if srcloc != nil && srcloc.n != nil {
1292+
epos = srcloc.n.Pos
1293+
}
1294+
explanation = append(explanation, logopt.NewLoggedOpt(epos, "escflow", "escape", e.curfn.funcname(), flow))
1295+
}
12601296

1261-
fmt.Printf("%s: flow: %s = %s%v:\n", pos, e.explainLoc(dst), ops, e.explainLoc(src))
12621297
for note := notes; note != nil; note = note.next {
1263-
fmt.Printf("%s: from %v (%v) at %s\n", pos, note.where, note.why, linestr(note.where.Pos))
1298+
if print {
1299+
fmt.Printf("%s: from %v (%v) at %s\n", pos, note.where, note.why, linestr(note.where.Pos))
1300+
}
1301+
if logopt.Enabled() {
1302+
explanation = append(explanation, logopt.NewLoggedOpt(note.where.Pos, "escflow", "escape", e.curfn.funcname(),
1303+
fmt.Sprintf(" from %v (%v)", note.where, note.why)))
1304+
}
12641305
}
1306+
return explanation
12651307
}
12661308

12671309
func (e *Escape) explainLoc(l *EscLocation) string {

‎src/cmd/compile/internal/logopt/log_opts.go

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -294,18 +294,23 @@ func checkLogPath(flag, destination string) {
294294
dest = destination
295295
}
296296

297-
var loggedOpts []LoggedOpt
297+
var loggedOpts []*LoggedOpt
298298
var mu = sync.Mutex{} // mu protects loggedOpts.
299299

300+
func NewLoggedOpt(pos src.XPos, what, pass, fname string, args ...interface{}) *LoggedOpt {
301+
pass = strings.Replace(pass, " ", "_", -1)
302+
return &LoggedOpt{pos, pass, fname, what, args}
303+
}
304+
300305
func LogOpt(pos src.XPos, what, pass, fname string, args ...interface{}) {
301306
if Format == None {
302307
return
303308
}
304-
pass = strings.Replace(pass, " ", "_", -1)
309+
lo := NewLoggedOpt(pos, what, pass, fname, args...)
305310
mu.Lock()
306311
defer mu.Unlock()
307312
// Because of concurrent calls from back end, no telling what the order will be, but is stable-sorted by outer Pos before use.
308-
loggedOpts = append(loggedOpts, LoggedOpt{pos, pass, fname, what, args})
313+
loggedOpts = append(loggedOpts, lo)
309314
}
310315

311316
func Enabled() bool {
@@ -321,7 +326,7 @@ func Enabled() bool {
321326
// byPos sorts diagnostics by source position.
322327
type byPos struct {
323328
ctxt *obj.Link
324-
a []LoggedOpt
329+
a []*LoggedOpt
325330
}
326331

327332
func (x byPos) Len() int { return len(x.a) }
@@ -402,15 +407,9 @@ func FlushLoggedOpts(ctxt *obj.Link, slashPkgPath string) {
402407
// For LSP, make a subdirectory for the package, and for each file foo.go, create foo.json in that subdirectory.
403408
currentFile := ""
404409
for _, x := range loggedOpts {
405-
posTmp = ctxt.AllPos(x.pos, posTmp)
406-
// Reverse posTmp to put outermost first.
407-
l := len(posTmp)
408-
for i := 0; i < l/2; i++ {
409-
posTmp[i], posTmp[l-i-1] = posTmp[l-i-1], posTmp[i]
410-
}
411-
412-
p0 := posTmp[0]
410+
posTmp, p0 := x.parsePos(ctxt, posTmp)
413411
p0f := uprootedPath(p0.Filename())
412+
414413
if currentFile != p0f {
415414
if w != nil {
416415
w.Close()
@@ -429,16 +428,27 @@ func FlushLoggedOpts(ctxt *obj.Link, slashPkgPath string) {
429428

430429
diagnostic.Code = x.what
431430
diagnostic.Message = target
432-
diagnostic.Range = Range{Start: Position{p0.Line(), p0.Col()},
433-
End: Position{p0.Line(), p0.Col()}}
431+
diagnostic.Range = newPointRange(p0)
434432
diagnostic.RelatedInformation = diagnostic.RelatedInformation[:0]
435433

436-
for i := 1; i < l; i++ {
437-
p := posTmp[i]
438-
loc := Location{URI: uriIfy(uprootedPath(p.Filename())),
439-
Range: Range{Start: Position{p.Line(), p.Col()},
440-
End: Position{p.Line(), p.Col()}}}
441-
diagnostic.RelatedInformation = append(diagnostic.RelatedInformation, DiagnosticRelatedInformation{Location: loc, Message: "inlineLoc"})
434+
appendInlinedPos(posTmp, &diagnostic)
435+
436+
// Diagnostic explanation is stored in RelatedInformation after inlining info
437+
if len(x.target) > 1 {
438+
switch y := x.target[1].(type) {
439+
case []*LoggedOpt:
440+
for _, z := range y {
441+
posTmp, p0 := z.parsePos(ctxt, posTmp)
442+
loc := newLocation(p0)
443+
msg := z.what
444+
if len(z.target) > 0 {
445+
msg = msg + ": " + fmt.Sprint(z.target[0])
446+
}
447+
448+
diagnostic.RelatedInformation = append(diagnostic.RelatedInformation, DiagnosticRelatedInformation{Location: loc, Message: msg})
449+
appendInlinedPos(posTmp, &diagnostic)
450+
}
451+
}
442452
}
443453

444454
encoder.Encode(diagnostic)
@@ -448,3 +458,33 @@ func FlushLoggedOpts(ctxt *obj.Link, slashPkgPath string) {
448458
}
449459
}
450460
}
461+
462+
func newPointRange(p src.Pos) Range {
463+
return Range{Start: Position{p.Line(), p.Col()},
464+
End: Position{p.Line(), p.Col()}}
465+
}
466+
467+
func newLocation(p src.Pos) Location {
468+
loc := Location{URI: uriIfy(uprootedPath(p.Filename())), Range: newPointRange(p)}
469+
return loc
470+
}
471+
472+
// appendInlinedPos extracts inlining information from posTmp and append it to diagnostic
473+
func appendInlinedPos(posTmp []src.Pos, diagnostic *Diagnostic) {
474+
for i := 1; i < len(posTmp); i++ {
475+
p := posTmp[i]
476+
loc := newLocation(p)
477+
diagnostic.RelatedInformation = append(diagnostic.RelatedInformation, DiagnosticRelatedInformation{Location: loc, Message: "inlineLoc"})
478+
}
479+
}
480+
481+
func (x *LoggedOpt) parsePos(ctxt *obj.Link, posTmp []src.Pos) ([]src.Pos, src.Pos) {
482+
posTmp = ctxt.AllPos(x.pos, posTmp)
483+
// Reverse posTmp to put outermost first.
484+
l := len(posTmp)
485+
for i := 0; i < l/2; i++ {
486+
posTmp[i], posTmp[l-i-1] = posTmp[l-i-1], posTmp[i]
487+
}
488+
p0 := posTmp[0]
489+
return posTmp, p0
490+
}

‎src/cmd/compile/internal/logopt/logopt_test.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,11 @@ func n() int {
4141
`
4242

4343
func want(t *testing.T, out string, desired string) {
44-
if !strings.Contains(out, desired) {
45-
t.Errorf("did not see phrase %s in \n%s", desired, out)
44+
// On Windows, Unicode escapes in the JSON output end up "normalized" elsewhere to /u....,
45+
// so "normalize" what we're looking for to match that.
46+
s := strings.ReplaceAll(desired, string(os.PathSeparator), "/")
47+
if !strings.Contains(out, s) {
48+
t.Errorf("did not see phrase %s in \n%s", s, out)
4649
}
4750
}
4851

@@ -178,7 +181,20 @@ func s15a8(x *[15]int64) [15]int64 {
178181
want(t, slogged, `{"range":{"start":{"line":11,"character":6},"end":{"line":11,"character":6}},"severity":3,"code":"isInBounds","source":"go compiler","message":""}`)
179182
want(t, slogged, `{"range":{"start":{"line":7,"character":6},"end":{"line":7,"character":6}},"severity":3,"code":"canInlineFunction","source":"go compiler","message":"cost: 35"}`)
180183
want(t, slogged, `{"range":{"start":{"line":21,"character":21},"end":{"line":21,"character":21}},"severity":3,"code":"cannotInlineCall","source":"go compiler","message":"foo cannot be inlined (escaping closure variable)"}`)
181-
184+
// escape analysis explanation
185+
want(t, slogged, `{"range":{"start":{"line":7,"character":13},"end":{"line":7,"character":13}},"severity":3,"code":"leak","source":"go compiler","message":"parameter z leaks to ~r2 with derefs=0",`+
186+
`"relatedInformation":[`+
187+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: flow: y = z:"},`+
188+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from y = \u003cN\u003e (assign-pair)"},`+
189+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: flow: ~r1 = y:"},`+
190+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"},`+
191+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from y.b (dot of pointer)"},`+
192+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":11},"end":{"line":4,"character":11}}},"message":"inlineLoc"},`+
193+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from \u0026y.b (address-of)"},`+
194+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":4,"character":9},"end":{"line":4,"character":9}}},"message":"inlineLoc"},`+
195+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":13},"end":{"line":9,"character":13}}},"message":"escflow: from ~r1 = \u003cN\u003e (assign-pair)"},`+
196+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":3},"end":{"line":9,"character":3}}},"message":"escflow: flow: ~r2 = ~r1:"},`+
197+
`{"location":{"uri":"file://tmpdir/file.go","range":{"start":{"line":9,"character":3},"end":{"line":9,"character":3}}},"message":"escflow: from return (*int)(~r1) (return)"}]}`)
182198
})
183199
}
184200

0 commit comments

Comments
 (0)