diff --git a/src/cmd/compile/internal/gc/checkcfg.go b/src/cmd/compile/internal/gc/checkcfg.go new file mode 100644 index 00000000000..bbb93a60c3b --- /dev/null +++ b/src/cmd/compile/internal/gc/checkcfg.go @@ -0,0 +1,299 @@ +// 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 +} diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index 9d4003f35a1..3fefe67fdfe 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -345,6 +345,10 @@ func compile(fn *Node) { if nerrors != 0 { return } + checkcontrolflow(fn) + if nerrors != 0 { + return + } if instrumenting { instrument(fn) } diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index 450be95e066..e7f82861b16 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -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] diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index 4cb26edb924..4817d6f3a6c 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -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 { diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 8299e49a34d..0425996b811 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -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: