Skip to content

Commit

Permalink
fix: detect non boolean condition for IF and FOR statements
Browse files Browse the repository at this point in the history
  • Loading branch information
mvertes authored and traefiker committed Nov 25, 2019
1 parent d44e4af commit eef5915
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 5 deletions.
9 changes: 9 additions & 0 deletions _test/for7.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package main

func main() {
for i := 0; i; {
}
}

// Error:
// 4:14: non-bool used as for condition
13 changes: 13 additions & 0 deletions _test/if2.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package main

import "fmt"

func main() {
var i int
if i % 1000000 {
fmt.Println("oops")
}
}

// Error:
// 7:5: non-bool used as if condition
42 changes: 37 additions & 5 deletions interp/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {
// Assign by reading from a receiving channel
n.gen = nop
src.findex = dest.findex // Set recv address to LHS
dest.typ = src.typ.val
dest.typ = src.typ
case n.action == aAssign && src.action == aCompositeLit:
n.gen = nop
src.findex = dest.findex
Expand Down Expand Up @@ -832,6 +832,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case forStmt1: // for cond {}
cond, body := n.child[0], n.child[1]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as for condition")
}
n.start = cond.start
cond.tnext = body.start
cond.fnext = n
Expand All @@ -841,6 +844,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case forStmt2: // for init; cond; {}
init, cond, body := n.child[0], n.child[1], n.child[2]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as for condition")
}
n.start = init.start
init.tnext = cond.start
cond.tnext = body.start
Expand All @@ -851,6 +857,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case forStmt3: // for ; cond; post {}
cond, post, body := n.child[0], n.child[1], n.child[2]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as for condition")
}
n.start = cond.start
cond.tnext = body.start
cond.fnext = n
Expand All @@ -870,6 +879,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case forStmt4: // for init; cond; post {}
init, cond, post, body := n.child[0], n.child[1], n.child[2], n.child[3]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as for condition")
}
n.start = init.start
init.tnext = cond.start
cond.tnext = body.start
Expand Down Expand Up @@ -953,6 +965,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case ifStmt0: // if cond {}
cond, tbody := n.child[0], n.child[1]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as if condition")
}
n.start = cond.start
cond.tnext = tbody.start
cond.fnext = n
Expand All @@ -961,6 +976,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case ifStmt1: // if cond {} else {}
cond, tbody, fbody := n.child[0], n.child[1], n.child[2]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as if condition")
}
n.start = cond.start
cond.tnext = tbody.start
cond.fnext = fbody.start
Expand All @@ -970,6 +988,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case ifStmt2: // if init; cond {}
init, cond, tbody := n.child[0], n.child[1], n.child[2]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as if condition")
}
n.start = init.start
tbody.tnext = n
init.tnext = cond.start
Expand All @@ -979,6 +1000,9 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {

case ifStmt3: // if init; cond {} else {}
init, cond, tbody, fbody := n.child[0], n.child[1], n.child[2], n.child[3]
if !isBool(cond.typ) {
err = cond.cfgErrorf("non-bool used as if condition")
}
n.start = init.start
init.tnext = cond.start
cond.tnext = tbody.start
Expand Down Expand Up @@ -1353,6 +1377,14 @@ func (interp *Interpreter) cfg(root *node) ([]*node, error) {
case unaryExpr:
wireChild(n)
n.typ = n.child[0].typ
if n.action == aRecv {
// Channel receive operation: set type to the channel data type
if n.typ.cat == valueT {
n.typ = &itype{cat: valueT, rtype: n.typ.rtype.Elem()}
} else {
n.typ = n.typ.val
}
}
// TODO: Optimisation: avoid allocation if boolean branch op (i.e. '!' in an 'if' expr)
n.findex = sc.add(n.typ)

Expand Down Expand Up @@ -1383,9 +1415,9 @@ func compDefineX(sc *scope, n *node) error {
l := len(n.child) - 1
types := []*itype{}

switch n.child[l].kind {
switch src := n.child[l]; src.kind {
case callExpr:
funtype, err := nodeType(n.interp, sc, n.child[l].child[0])
funtype, err := nodeType(n.interp, sc, src.child[0])
if err != nil {
return err
}
Expand All @@ -1400,7 +1432,7 @@ func compDefineX(sc *scope, n *node) error {
n.gen = nop

case indexExpr:
types = append(types, n.child[l].child[0].typ.val, sc.getType("bool"))
types = append(types, src.typ, sc.getType("bool"))
n.child[l].gen = getIndexMap2
n.gen = nop

Expand All @@ -1415,7 +1447,7 @@ func compDefineX(sc *scope, n *node) error {

case unaryExpr:
if n.child[l].action == aRecv {
types = append(types, n.child[l].child[0].typ.val, sc.getType("bool"))
types = append(types, src.typ, sc.getType("bool"))
n.child[l].gen = recv2
n.gen = nop
}
Expand Down
12 changes: 12 additions & 0 deletions interp/interp_consistent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ func TestInterpConsistencyBuild(t *testing.T) {
file.Name() == "bad0.go" || // expect error
file.Name() == "export1.go" || // non-main package
file.Name() == "export0.go" || // non-main package
file.Name() == "for7.go" || // expect error
file.Name() == "if2.go" || // expect error
file.Name() == "import6.go" || // expect error
file.Name() == "io0.go" || // use random number
file.Name() == "op1.go" || // expect error
Expand Down Expand Up @@ -136,6 +138,16 @@ func TestInterpErrorConsistency(t *testing.T) {
expectedInterp: "1:1: expected 'package', found println",
expectedExec: "1:1: expected 'package', found println",
},
{
fileName: "if2.go",
expectedInterp: "7:5: non-bool used as if condition",
expectedExec: "7:2: non-bool i % 1000000 (type int) used as if condition",
},
{
fileName: "for7.go",
expectedInterp: "4:14: non-bool used as for condition",
expectedExec: "4:2: non-bool i (type int) used as for condition",
},
{
fileName: "op1.go",
expectedInterp: "5:2: illegal operand types for '+=' operator",
Expand Down
2 changes: 2 additions & 0 deletions interp/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,8 @@ func isInterface(t *itype) bool {

func isStruct(t *itype) bool { return t.TypeOf().Kind() == reflect.Struct }

func isBool(t *itype) bool { return t.TypeOf().Kind() == reflect.Bool }

func isInt(t reflect.Type) bool {
switch t.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
Expand Down

0 comments on commit eef5915

Please sign in to comment.