Commit c03e75e5 authored by Josh Bleecher Snyder's avatar Josh Bleecher Snyder

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 <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
parent 604455a4
// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package gc
import (
"cmd/internal/src"
)
// checkcontrolflow checks fn's control flow structures for correctness.
// It catches:
// * misplaced breaks and continues
// * bad labeled break and continues
// * invalid, unused, duplicate, and missing labels
// * gotos jumping over declarations and into blocks
func checkcontrolflow(fn *Node) {
c := controlflow{
labels: make(map[string]*cfLabel),
labeledNodes: make(map[*Node]*cfLabel),
}
c.pushPos(fn.Pos)
c.stmtList(fn.Nbody)
// Check that we used all labels.
for name, lab := range c.labels {
if !lab.used() && !lab.reported && !lab.defNode.Used() {
yyerrorl(lab.defNode.Pos, "label %v defined and not used", name)
lab.reported = true
}
if lab.used() && !lab.defined() && !lab.reported {
yyerrorl(lab.useNode.Pos, "label %v not defined", name)
lab.reported = true
}
}
// Check any forward gotos. Non-forward gotos have already been checked.
for _, n := range c.fwdGotos {
lab := c.labels[n.Left.Sym.Name]
// If the label is undefined, we have already have printed an error.
if lab.defined() {
c.checkgoto(n, lab.defNode)
}
}
}
type controlflow struct {
// Labels and labeled control flow nodes (OFOR, OFORUNTIL, OSWITCH, OSELECT) in f.
labels map[string]*cfLabel
labeledNodes map[*Node]*cfLabel
// Gotos that jump forward; required for deferred checkgoto calls.
fwdGotos []*Node
// Unlabeled break and continue statement tracking.
innerloop *Node
// Position stack. The current position is top of stack.
pos []src.XPos
}
// cfLabel is a label tracked by a controlflow.
type cfLabel struct {
ctlNode *Node // associated labeled control flow node
defNode *Node // label definition Node (OLABEL)
// Label use Node (OGOTO, OBREAK, OCONTINUE).
// There might be multiple uses, but we only need to track one.
useNode *Node
reported bool // reported indicates whether an error has already been reported for this label
}
// defined reports whether the label has a definition (OLABEL node).
func (l *cfLabel) defined() bool { return l.defNode != nil }
// used reports whether the label has a use (OGOTO, OBREAK, or OCONTINUE node).
func (l *cfLabel) used() bool { return l.useNode != nil }
// label returns the label associated with sym, creating it if necessary.
func (c *controlflow) label(sym *Sym) *cfLabel {
lab := c.labels[sym.Name]
if lab == nil {
lab = new(cfLabel)
c.labels[sym.Name] = lab
}
return lab
}
// stmtList checks l.
func (c *controlflow) stmtList(l Nodes) {
for _, n := range l.Slice() {
c.stmt(n)
}
}
// stmt checks n.
func (c *controlflow) stmt(n *Node) {
c.pushPos(n.Pos)
defer c.popPos()
c.stmtList(n.Ninit)
checkedNbody := false
switch n.Op {
case OLABEL:
sym := n.Left.Sym
lab := c.label(sym)
// Associate label with its control flow node, if any
if ctl := n.labeledControl(); ctl != nil {
c.labeledNodes[ctl] = lab
}
if !lab.defined() {
lab.defNode = n
} else {
c.err("label %v already defined at %v", sym, linestr(lab.defNode.Pos))
lab.reported = true
}
case OGOTO:
lab := c.label(n.Left.Sym)
if !lab.used() {
lab.useNode = n
}
if lab.defined() {
c.checkgoto(n, lab.defNode)
} else {
c.fwdGotos = append(c.fwdGotos, n)
}
case OCONTINUE, OBREAK:
if n.Left == nil {
// plain break/continue
if c.innerloop == nil {
c.err("%v is not in a loop", n.Op)
}
break
}
// labeled break/continue; look up the target
sym := n.Left.Sym
lab := c.label(sym)
if !lab.used() {
lab.useNode = n.Left
}
if !lab.defined() {
c.err("%v label not defined: %v", n.Op, sym)
lab.reported = true
break
}
ctl := lab.ctlNode
if n.Op == OCONTINUE && ctl != nil && (ctl.Op == OSWITCH || ctl.Op == OSELECT) {
// Cannot continue in a switch or select.
ctl = nil
}
if ctl == nil {
// Valid label but not usable with a break/continue here, e.g.:
// for {
// continue abc
// }
// abc:
// for {}
c.err("invalid %v label %v", n.Op, sym)
lab.reported = true
}
case OFOR, OFORUNTIL, OSWITCH, OSELECT:
// set up for continue/break in body
innerloop := c.innerloop
c.innerloop = n
lab := c.labeledNodes[n]
if lab != nil {
// labeled for loop
lab.ctlNode = n
}
// check body
c.stmtList(n.Nbody)
checkedNbody = true
// tear down continue/break
c.innerloop = innerloop
if lab != nil {
lab.ctlNode = nil
}
}
if !checkedNbody {
c.stmtList(n.Nbody)
}
c.stmtList(n.List)
c.stmtList(n.Rlist)
}
// pushPos pushes a position onto the position stack.
func (c *controlflow) pushPos(pos src.XPos) {
if !pos.IsKnown() {
pos = c.peekPos()
if Debug['K'] != 0 {
Warn("controlflow: unknown position")
}
}
c.pos = append(c.pos, pos)
}
// popLine pops the top of the position stack.
func (c *controlflow) popPos() { c.pos = c.pos[:len(c.pos)-1] }
// peekPos peeks at the top of the position stack.
func (c *controlflow) peekPos() src.XPos { return c.pos[len(c.pos)-1] }
// err reports a control flow error at the current position.
func (c *controlflow) err(msg string, args ...interface{}) {
yyerrorl(c.peekPos(), msg, args...)
}
// checkgoto checks that a goto from from to to does not
// jump into a block or jump over variable declarations.
func (c *controlflow) checkgoto(from *Node, to *Node) {
if from.Op != OGOTO || to.Op != OLABEL {
Fatalf("bad from/to in checkgoto: %v -> %v", from, to)
}
// from and to's Sym fields record dclstack's value at their
// position, which implicitly encodes their block nesting
// level and variable declaration position within that block.
//
// For valid gotos, to.Sym will be a tail of from.Sym.
// Otherwise, any link in to.Sym not also in from.Sym
// indicates a block/declaration being jumped into/over.
//
// TODO(mdempsky): We should only complain about jumping over
// variable declarations, but currently we reject type and
// constant declarations too (#8042).
if from.Sym == to.Sym {
return
}
nf := dcldepth(from.Sym)
nt := dcldepth(to.Sym)
// Unwind from.Sym so it's no longer than to.Sym. It's okay to
// jump out of blocks or backwards past variable declarations.
fs := from.Sym
for ; nf > nt; nf-- {
fs = fs.Link
}
if fs == to.Sym {
return
}
// Decide what to complain about. Unwind to.Sym until where it
// forked from from.Sym, and keep track of the innermost block
// and declaration we jumped into/over.
var block *Sym
var dcl *Sym
// If to.Sym is longer, unwind until it's the same length.
ts := to.Sym
for ; nt > nf; nt-- {
if ts.Pkg == nil {
block = ts
} else {
dcl = ts
}
ts = ts.Link
}
// Same length; unwind until we find their common ancestor.
for ts != fs {
if ts.Pkg == nil {
block = ts
} else {
dcl = ts
}
ts = ts.Link
fs = fs.Link
}
// Prefer to complain about 'into block' over declarations.
pos := from.Left.Pos
if block != nil {
yyerrorl(pos, "goto %v jumps into block starting at %v", from.Left.Sym, linestr(block.Lastlineno))
} else {
yyerrorl(pos, "goto %v jumps over declaration of %v at %v", from.Left.Sym, dcl, linestr(dcl.Lastlineno))
}
}
// dcldepth returns the declaration depth for a dclstack Sym; that is,
// the sum of the block nesting level and the number of declarations
// in scope.
func dcldepth(s *Sym) int {
n := 0
for ; s != nil; s = s.Link {
n++
}
return n
}
......@@ -345,6 +345,10 @@ func compile(fn *Node) {
if nerrors != 0 {
return
}
checkcontrolflow(fn)
if nerrors != 0 {
return
}
if instrumenting {
instrument(fn)
}
......
......@@ -138,27 +138,6 @@ func buildssa(fn *Node) *ssa.Func {
s.popLine()
}
// Check that we used all labels
for name, lab := range s.labels {
if !lab.used() && !lab.reported && !lab.defNode.Used() {
yyerrorl(lab.defNode.Pos, "label %v defined and not used", name)
lab.reported = true
}
if lab.used() && !lab.defined() && !lab.reported {
yyerrorl(lab.useNode.Pos, "label %v not defined", name)
lab.reported = true
}
}
// Check any forward gotos. Non-forward gotos have already been checked.
for _, n := range s.fwdGotos {
lab := s.labels[n.Left.Sym.Name]
// If the label is undefined, we have already have printed an error.
if lab.defined() {
s.checkgoto(n, lab.defNode)
}
}
if nerrors > 0 {
s.f.Free()
return nil
......@@ -186,8 +165,6 @@ type state struct {
labels map[string]*ssaLabel
labeledNodes map[*Node]*ssaLabel
// gotos that jump forward; required for deferred checkgoto calls
fwdGotos []*Node
// Code that must precede any return
// (e.g., copying value of heap-escaped paramout back to true paramout)
exitCode Nodes
......@@ -250,20 +227,8 @@ type ssaLabel struct {
target *ssa.Block // block identified by this label
breakTarget *ssa.Block // block to break to in control flow node identified by this label
continueTarget *ssa.Block // block to continue to in control flow node identified by this label
defNode *Node // label definition Node (OLABEL)
// Label use Node (OGOTO, OBREAK, OCONTINUE).
// Used only for error detection and reporting.
// There might be multiple uses, but we only need to track one.
useNode *Node
reported bool // reported indicates whether an error has already been reported for this label
}
// defined reports whether the label has a definition (OLABEL node).
func (l *ssaLabel) defined() bool { return l.defNode != nil }
// used reports whether the label has a use (OGOTO, OBREAK, or OCONTINUE node).
func (l *ssaLabel) used() bool { return l.useNode != nil }
// label returns the label associated with sym, creating it if necessary.
func (s *state) label(sym *Sym) *ssaLabel {
lab := s.labels[sym.Name]
......@@ -493,20 +458,10 @@ func (s *state) stmt(n *Node) {
s.pushLine(n.Pos)
defer s.popLine()
// If s.curBlock is nil, then we're about to generate dead code.
// We can't just short-circuit here, though,
// because we check labels and gotos as part of SSA generation.
// Provide a block for the dead code so that we don't have
// to add special cases everywhere else.
if s.curBlock == nil {
switch n.Op {
case OLABEL, OBREAK, OCONTINUE:
// These statements don't need a block,
// and they commonly occur without one.
default:
dead := s.f.NewBlock(ssa.BlockPlain)
s.startBlock(dead)
}
// If s.curBlock is nil, and n isn't a label (which might have an associated goto somewhere),
// then this code is dead. Stop here.
if s.curBlock == nil && n.Op != OLABEL {
return
}
s.stmtList(n.Ninit)
......@@ -585,29 +540,13 @@ func (s *state) stmt(n *Node) {
case OLABEL:
sym := n.Left.Sym
if isblanksym(sym) {
// Empty identifier is valid but useless.
// See issues 11589, 11593.
return
}
lab := s.label(sym)
// Associate label with its control flow node, if any
if ctl := n.Name.Defn; ctl != nil {
switch ctl.Op {
case OFOR, OFORUNTIL, OSWITCH, OSELECT:
s.labeledNodes[ctl] = lab
}
if ctl := n.labeledControl(); ctl != nil {
s.labeledNodes[ctl] = lab
}
if !lab.defined() {
lab.defNode = n
} else {
s.Error("label %v already defined at %v", sym, linestr(lab.defNode.Pos))
lab.reported = true
}
// The label might already have a target block via a goto.
if lab.target == nil {
lab.target = s.f.NewBlock(ssa.BlockPlain)
......@@ -628,15 +567,6 @@ func (s *state) stmt(n *Node) {
if lab.target == nil {
lab.target = s.f.NewBlock(ssa.BlockPlain)
}
if !lab.used() {
lab.useNode = n
}
if lab.defined() {
s.checkgoto(n, lab.defNode)
} else {
s.fwdGotos = append(s.fwdGotos, n)
}
b := s.endBlock()
b.AddEdgeTo(lab.target)
......@@ -790,58 +720,29 @@ func (s *state) stmt(n *Node) {
b.Aux = Linksym(n.Left.Sym)
case OCONTINUE, OBREAK:
var op string
var to *ssa.Block
switch n.Op {
case OCONTINUE:
op = "continue"
to = s.continueTo
case OBREAK:
op = "break"
to = s.breakTo
}
if n.Left == nil {
// plain break/continue
if to == nil {
s.Error("%s is not in a loop", op)
return
switch n.Op {
case OCONTINUE:
to = s.continueTo
case OBREAK:
to = s.breakTo
}
// nothing to do; "to" is already the correct target
} else {
// labeled break/continue; look up the target
sym := n.Left.Sym
lab := s.label(sym)
if !lab.used() {
lab.useNode = n.Left
}
if !lab.defined() {
s.Error("%s label not defined: %v", op, sym)
lab.reported = true
return
}
switch n.Op {
case OCONTINUE:
to = lab.continueTarget
case OBREAK:
to = lab.breakTarget
}
if to == nil {
// Valid label but not usable with a break/continue here, e.g.:
// for {
// continue abc
// }
// abc:
// for {}
s.Error("invalid %s label %v", op, sym)
lab.reported = true
return
}
}
if s.curBlock != nil {
b := s.endBlock()
b.AddEdgeTo(to)
}
b := s.endBlock()
b.AddEdgeTo(to)
case OFOR, OFORUNTIL:
// OFOR: for Ninit; Left; Right { Nbody }
......@@ -4199,91 +4100,6 @@ func (s *state) dottype(n *Node, commaok bool) (res, resok *ssa.Value) {
return res, resok
}
// checkgoto checks that a goto from from to to does not
// jump into a block or jump over variable declarations.
func (s *state) checkgoto(from *Node, to *Node) {
if from.Op != OGOTO || to.Op != OLABEL {
Fatalf("bad from/to in checkgoto: %v -> %v", from, to)
}
// from and to's Sym fields record dclstack's value at their
// position, which implicitly encodes their block nesting
// level and variable declaration position within that block.
//
// For valid gotos, to.Sym will be a tail of from.Sym.
// Otherwise, any link in to.Sym not also in from.Sym
// indicates a block/declaration being jumped into/over.
//
// TODO(mdempsky): We should only complain about jumping over
// variable declarations, but currently we reject type and
// constant declarations too (#8042).
if from.Sym == to.Sym {
return
}
nf := dcldepth(from.Sym)
nt := dcldepth(to.Sym)
// Unwind from.Sym so it's no longer than to.Sym. It's okay to
// jump out of blocks or backwards past variable declarations.
fs := from.Sym
for ; nf > nt; nf-- {
fs = fs.Link
}
if fs == to.Sym {
return
}
// Decide what to complain about. Unwind to.Sym until where it
// forked from from.Sym, and keep track of the innermost block
// and declaration we jumped into/over.
var block *Sym
var dcl *Sym
// If to.Sym is longer, unwind until it's the same length.
ts := to.Sym
for ; nt > nf; nt-- {
if ts.Pkg == nil {
block = ts
} else {
dcl = ts
}
ts = ts.Link
}
// Same length; unwind until we find their common ancestor.
for ts != fs {
if ts.Pkg == nil {
block = ts
} else {
dcl = ts
}
ts = ts.Link
fs = fs.Link
}
// Prefer to complain about 'into block' over declarations.
lno := from.Left.Pos
if block != nil {
yyerrorl(lno, "goto %v jumps into block starting at %v", from.Left.Sym, linestr(block.Lastlineno))
} else {
yyerrorl(lno, "goto %v jumps over declaration of %v at %v", from.Left.Sym, dcl, linestr(dcl.Lastlineno))
}
}
// dcldepth returns the declaration depth for a dclstack Sym; that is,
// the sum of the block nesting level and the number of declarations
// in scope.
func dcldepth(s *Sym) int {
n := 0
for ; s != nil; s = s.Link {
n++
}
return n
}
// variable returns the value of a variable at the current location.
func (s *state) variable(name *Node, t ssa.Type) *ssa.Value {
v := s.vars[name]
......
......@@ -1078,6 +1078,23 @@ func (o Op) IsSlice3() bool {
return false
}
// labeledControl returns the control flow Node (for, switch, select)
// associated with the label n, if any.
func (n *Node) labeledControl() *Node {
if n.Op != OLABEL {
Fatalf("labeledControl %v", n.Op)
}
ctl := n.Name.Defn
if ctl == nil {
return nil
}
switch ctl.Op {
case OFOR, OFORUNTIL, OSWITCH, OSELECT:
return ctl
}
return nil
}
func syslook(name string) *Node {
s := Pkglookup(name, Runtimepkg)
if s == nil || s.Def == nil {
......
......@@ -2006,6 +2006,13 @@ OpSwitch:
case OLABEL:
ok |= Etop
decldepth++
if isblanksym(n.Left.Sym) {
// Empty identifier is valid but useless.
// Eliminate now to simplify life later.
// See issues 7538, 11589, 11593.
n.Op = OEMPTY
n.Left = nil
}
break OpSwitch
case ODEFER:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment