Skip to content

Commit c03e75e

Browse files
committed
cmd/compile: check labels and gotos before building SSA
This CL introduces yet another compiler pass, which checks for correct control flow constructs prior to converting from AST to SSA form. It cannot be integrated with walk, since walk rewrites switch and select statements on the fly. To reduce code duplication, this CL also does some minor refactoring. With this pass in place, the AST to SSA converter can now stop generating SSA for any known-dead code. This minor savings pays for the minor cost of the new pass. Performance is almost a wash: name old time/op new time/op delta Template 206ms ± 4% 205ms ± 4% ~ (p=0.108 n=43+43) Unicode 84.0ms ± 4% 84.0ms ± 4% ~ (p=0.979 n=43+43) GoTypes 550ms ± 3% 553ms ± 3% ~ (p=0.065 n=40+41) Compiler 2.57s ± 4% 2.58s ± 2% ~ (p=0.103 n=44+41) SSA 3.94s ± 3% 3.93s ± 2% ~ (p=0.833 n=44+42) Flate 126ms ± 6% 125ms ± 4% ~ (p=0.941 n=43+39) GoParser 147ms ± 4% 148ms ± 3% ~ (p=0.164 n=42+39) Reflect 359ms ± 3% 357ms ± 5% ~ (p=0.241 n=43+44) Tar 106ms ± 5% 106ms ± 7% ~ (p=0.853 n=40+43) XML 202ms ± 3% 203ms ± 3% ~ (p=0.488 n=42+41) name old user-ns/op new user-ns/op delta Template 240M ± 4% 239M ± 4% ~ (p=0.844 n=42+43) Unicode 107M ± 5% 107M ± 4% ~ (p=0.332 n=40+43) GoTypes 735M ± 3% 731M ± 4% ~ (p=0.141 n=43+44) Compiler 3.51G ± 3% 3.52G ± 3% ~ (p=0.208 n=42+43) SSA 5.72G ± 4% 5.72G ± 3% ~ (p=0.928 n=44+42) Flate 151M ± 7% 150M ± 8% ~ (p=0.662 n=44+43) GoParser 181M ± 5% 181M ± 4% ~ (p=0.379 n=41+44) Reflect 447M ± 4% 445M ± 4% ~ (p=0.344 n=43+43) Tar 125M ± 7% 124M ± 6% ~ (p=0.353 n=43+43) XML 248M ± 4% 250M ± 6% ~ (p=0.158 n=44+44) name old alloc/op new alloc/op delta Template 40.3MB ± 0% 40.2MB ± 0% -0.27% (p=0.000 n=10+10) Unicode 30.3MB ± 0% 30.2MB ± 0% -0.10% (p=0.015 n=10+10) GoTypes 114MB ± 0% 114MB ± 0% -0.06% (p=0.000 n=7+9) Compiler 480MB ± 0% 481MB ± 0% +0.07% (p=0.000 n=10+10) SSA 864MB ± 0% 862MB ± 0% -0.25% (p=0.000 n=9+10) Flate 25.9MB ± 0% 25.9MB ± 0% ~ (p=0.123 n=10+10) GoParser 32.1MB ± 0% 32.1MB ± 0% ~ (p=0.631 n=10+10) Reflect 79.9MB ± 0% 79.6MB ± 0% -0.39% (p=0.000 n=10+9) Tar 27.1MB ± 0% 27.0MB ± 0% -0.18% (p=0.003 n=10+10) XML 42.6MB ± 0% 42.6MB ± 0% ~ (p=0.143 n=10+10) name old allocs/op new allocs/op delta Template 401k ± 0% 401k ± 1% ~ (p=0.353 n=10+10) Unicode 322k ± 0% 322k ± 0% ~ (p=0.739 n=10+10) GoTypes 1.18M ± 0% 1.18M ± 0% +0.25% (p=0.001 n=7+8) Compiler 4.51M ± 0% 4.53M ± 0% +0.37% (p=0.000 n=10+10) SSA 7.91M ± 0% 7.93M ± 0% +0.20% (p=0.000 n=9+10) Flate 244k ± 0% 245k ± 0% ~ (p=0.123 n=10+10) GoParser 323k ± 1% 324k ± 1% +0.40% (p=0.035 n=10+10) Reflect 1.01M ± 0% 1.02M ± 0% +0.37% (p=0.000 n=10+9) Tar 258k ± 1% 258k ± 1% ~ (p=0.661 n=10+9) XML 403k ± 0% 405k ± 0% +0.47% (p=0.004 n=10+10) Updates #15756 Updates #19250 Change-Id: I647bfbb745c35630447eb79dfcaa994b490ce942 Reviewed-on: https://go-review.googlesource.com/38159 Run-TryBot: Josh Bleecher Snyder <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]>
1 parent 604455a commit c03e75e

File tree

5 files changed

+340
-197
lines changed

5 files changed

+340
-197
lines changed
+299
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,299 @@
1+
// Copyright 2017 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package gc
6+
7+
import (
8+
"cmd/internal/src"
9+
)
10+
11+
// checkcontrolflow checks fn's control flow structures for correctness.
12+
// It catches:
13+
// * misplaced breaks and continues
14+
// * bad labeled break and continues
15+
// * invalid, unused, duplicate, and missing labels
16+
// * gotos jumping over declarations and into blocks
17+
func checkcontrolflow(fn *Node) {
18+
c := controlflow{
19+
labels: make(map[string]*cfLabel),
20+
labeledNodes: make(map[*Node]*cfLabel),
21+
}
22+
c.pushPos(fn.Pos)
23+
c.stmtList(fn.Nbody)
24+
25+
// Check that we used all labels.
26+
for name, lab := range c.labels {
27+
if !lab.used() && !lab.reported && !lab.defNode.Used() {
28+
yyerrorl(lab.defNode.Pos, "label %v defined and not used", name)
29+
lab.reported = true
30+
}
31+
if lab.used() && !lab.defined() && !lab.reported {
32+
yyerrorl(lab.useNode.Pos, "label %v not defined", name)
33+
lab.reported = true
34+
}
35+
}
36+
37+
// Check any forward gotos. Non-forward gotos have already been checked.
38+
for _, n := range c.fwdGotos {
39+
lab := c.labels[n.Left.Sym.Name]
40+
// If the label is undefined, we have already have printed an error.
41+
if lab.defined() {
42+
c.checkgoto(n, lab.defNode)
43+
}
44+
}
45+
}
46+
47+
type controlflow struct {
48+
// Labels and labeled control flow nodes (OFOR, OFORUNTIL, OSWITCH, OSELECT) in f.
49+
labels map[string]*cfLabel
50+
labeledNodes map[*Node]*cfLabel
51+
52+
// Gotos that jump forward; required for deferred checkgoto calls.
53+
fwdGotos []*Node
54+
55+
// Unlabeled break and continue statement tracking.
56+
innerloop *Node
57+
58+
// Position stack. The current position is top of stack.
59+
pos []src.XPos
60+
}
61+
62+
// cfLabel is a label tracked by a controlflow.
63+
type cfLabel struct {
64+
ctlNode *Node // associated labeled control flow node
65+
defNode *Node // label definition Node (OLABEL)
66+
// Label use Node (OGOTO, OBREAK, OCONTINUE).
67+
// There might be multiple uses, but we only need to track one.
68+
useNode *Node
69+
reported bool // reported indicates whether an error has already been reported for this label
70+
}
71+
72+
// defined reports whether the label has a definition (OLABEL node).
73+
func (l *cfLabel) defined() bool { return l.defNode != nil }
74+
75+
// used reports whether the label has a use (OGOTO, OBREAK, or OCONTINUE node).
76+
func (l *cfLabel) used() bool { return l.useNode != nil }
77+
78+
// label returns the label associated with sym, creating it if necessary.
79+
func (c *controlflow) label(sym *Sym) *cfLabel {
80+
lab := c.labels[sym.Name]
81+
if lab == nil {
82+
lab = new(cfLabel)
83+
c.labels[sym.Name] = lab
84+
}
85+
return lab
86+
}
87+
88+
// stmtList checks l.
89+
func (c *controlflow) stmtList(l Nodes) {
90+
for _, n := range l.Slice() {
91+
c.stmt(n)
92+
}
93+
}
94+
95+
// stmt checks n.
96+
func (c *controlflow) stmt(n *Node) {
97+
c.pushPos(n.Pos)
98+
defer c.popPos()
99+
c.stmtList(n.Ninit)
100+
101+
checkedNbody := false
102+
103+
switch n.Op {
104+
case OLABEL:
105+
sym := n.Left.Sym
106+
lab := c.label(sym)
107+
// Associate label with its control flow node, if any
108+
if ctl := n.labeledControl(); ctl != nil {
109+
c.labeledNodes[ctl] = lab
110+
}
111+
112+
if !lab.defined() {
113+
lab.defNode = n
114+
} else {
115+
c.err("label %v already defined at %v", sym, linestr(lab.defNode.Pos))
116+
lab.reported = true
117+
}
118+
119+
case OGOTO:
120+
lab := c.label(n.Left.Sym)
121+
if !lab.used() {
122+
lab.useNode = n
123+
}
124+
if lab.defined() {
125+
c.checkgoto(n, lab.defNode)
126+
} else {
127+
c.fwdGotos = append(c.fwdGotos, n)
128+
}
129+
130+
case OCONTINUE, OBREAK:
131+
if n.Left == nil {
132+
// plain break/continue
133+
if c.innerloop == nil {
134+
c.err("%v is not in a loop", n.Op)
135+
}
136+
break
137+
}
138+
139+
// labeled break/continue; look up the target
140+
sym := n.Left.Sym
141+
lab := c.label(sym)
142+
if !lab.used() {
143+
lab.useNode = n.Left
144+
}
145+
if !lab.defined() {
146+
c.err("%v label not defined: %v", n.Op, sym)
147+
lab.reported = true
148+
break
149+
}
150+
ctl := lab.ctlNode
151+
if n.Op == OCONTINUE && ctl != nil && (ctl.Op == OSWITCH || ctl.Op == OSELECT) {
152+
// Cannot continue in a switch or select.
153+
ctl = nil
154+
}
155+
if ctl == nil {
156+
// Valid label but not usable with a break/continue here, e.g.:
157+
// for {
158+
// continue abc
159+
// }
160+
// abc:
161+
// for {}
162+
c.err("invalid %v label %v", n.Op, sym)
163+
lab.reported = true
164+
}
165+
166+
case OFOR, OFORUNTIL, OSWITCH, OSELECT:
167+
// set up for continue/break in body
168+
innerloop := c.innerloop
169+
c.innerloop = n
170+
lab := c.labeledNodes[n]
171+
if lab != nil {
172+
// labeled for loop
173+
lab.ctlNode = n
174+
}
175+
176+
// check body
177+
c.stmtList(n.Nbody)
178+
checkedNbody = true
179+
180+
// tear down continue/break
181+
c.innerloop = innerloop
182+
if lab != nil {
183+
lab.ctlNode = nil
184+
}
185+
}
186+
187+
if !checkedNbody {
188+
c.stmtList(n.Nbody)
189+
}
190+
c.stmtList(n.List)
191+
c.stmtList(n.Rlist)
192+
}
193+
194+
// pushPos pushes a position onto the position stack.
195+
func (c *controlflow) pushPos(pos src.XPos) {
196+
if !pos.IsKnown() {
197+
pos = c.peekPos()
198+
if Debug['K'] != 0 {
199+
Warn("controlflow: unknown position")
200+
}
201+
}
202+
c.pos = append(c.pos, pos)
203+
}
204+
205+
// popLine pops the top of the position stack.
206+
func (c *controlflow) popPos() { c.pos = c.pos[:len(c.pos)-1] }
207+
208+
// peekPos peeks at the top of the position stack.
209+
func (c *controlflow) peekPos() src.XPos { return c.pos[len(c.pos)-1] }
210+
211+
// err reports a control flow error at the current position.
212+
func (c *controlflow) err(msg string, args ...interface{}) {
213+
yyerrorl(c.peekPos(), msg, args...)
214+
}
215+
216+
// checkgoto checks that a goto from from to to does not
217+
// jump into a block or jump over variable declarations.
218+
func (c *controlflow) checkgoto(from *Node, to *Node) {
219+
if from.Op != OGOTO || to.Op != OLABEL {
220+
Fatalf("bad from/to in checkgoto: %v -> %v", from, to)
221+
}
222+
223+
// from and to's Sym fields record dclstack's value at their
224+
// position, which implicitly encodes their block nesting
225+
// level and variable declaration position within that block.
226+
//
227+
// For valid gotos, to.Sym will be a tail of from.Sym.
228+
// Otherwise, any link in to.Sym not also in from.Sym
229+
// indicates a block/declaration being jumped into/over.
230+
//
231+
// TODO(mdempsky): We should only complain about jumping over
232+
// variable declarations, but currently we reject type and
233+
// constant declarations too (#8042).
234+
235+
if from.Sym == to.Sym {
236+
return
237+
}
238+
239+
nf := dcldepth(from.Sym)
240+
nt := dcldepth(to.Sym)
241+
242+
// Unwind from.Sym so it's no longer than to.Sym. It's okay to
243+
// jump out of blocks or backwards past variable declarations.
244+
fs := from.Sym
245+
for ; nf > nt; nf-- {
246+
fs = fs.Link
247+
}
248+
249+
if fs == to.Sym {
250+
return
251+
}
252+
253+
// Decide what to complain about. Unwind to.Sym until where it
254+
// forked from from.Sym, and keep track of the innermost block
255+
// and declaration we jumped into/over.
256+
var block *Sym
257+
var dcl *Sym
258+
259+
// If to.Sym is longer, unwind until it's the same length.
260+
ts := to.Sym
261+
for ; nt > nf; nt-- {
262+
if ts.Pkg == nil {
263+
block = ts
264+
} else {
265+
dcl = ts
266+
}
267+
ts = ts.Link
268+
}
269+
270+
// Same length; unwind until we find their common ancestor.
271+
for ts != fs {
272+
if ts.Pkg == nil {
273+
block = ts
274+
} else {
275+
dcl = ts
276+
}
277+
ts = ts.Link
278+
fs = fs.Link
279+
}
280+
281+
// Prefer to complain about 'into block' over declarations.
282+
pos := from.Left.Pos
283+
if block != nil {
284+
yyerrorl(pos, "goto %v jumps into block starting at %v", from.Left.Sym, linestr(block.Lastlineno))
285+
} else {
286+
yyerrorl(pos, "goto %v jumps over declaration of %v at %v", from.Left.Sym, dcl, linestr(dcl.Lastlineno))
287+
}
288+
}
289+
290+
// dcldepth returns the declaration depth for a dclstack Sym; that is,
291+
// the sum of the block nesting level and the number of declarations
292+
// in scope.
293+
func dcldepth(s *Sym) int {
294+
n := 0
295+
for ; s != nil; s = s.Link {
296+
n++
297+
}
298+
return n
299+
}

src/cmd/compile/internal/gc/pgen.go

+4
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,10 @@ func compile(fn *Node) {
345345
if nerrors != 0 {
346346
return
347347
}
348+
checkcontrolflow(fn)
349+
if nerrors != 0 {
350+
return
351+
}
348352
if instrumenting {
349353
instrument(fn)
350354
}

0 commit comments

Comments
 (0)