Skip to content

Commit

Permalink
interp: do not panic in case of invalid constant definition
Browse files Browse the repository at this point in the history
 This is a follow-up of #1014, where an invalid constant definition  involving a builtin is now checked at CFG. In addition, some missing arithmetic operators are now detected for assign optimization.
  • Loading branch information
mvertes authored Jan 27, 2021
1 parent 100d090 commit 61b4980
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
34 changes: 21 additions & 13 deletions interp/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,11 +564,11 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
//
switch {
case n.action != aAssign:
// Do not optimize assign combined with another operator.
// Do not skip assign operation if it is combined with another operator.
case src.rval.IsValid():
// Do not skip assign operation when setting from a constant value.
// Do not skip assign operation if setting from a constant value.
case isMapEntry(dest):
// Setting a map entry needs an additional step, do not optimize.
// Setting a map entry requires an additional step, do not optimize.
// As we only write, skip the default useless getIndexMap dest action.
dest.gen = nop
case isCall(src) && dest.typ.cat != interfaceT && !isRecursiveField(dest):
Expand All @@ -590,19 +590,19 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
break
}
if dest.action == aGetIndex {
// Optimization does not work when assigning to a struct field.
// Skip optimization, as it does not work when assigning to a struct field.
break
}
n.gen = nop
src.findex = dest.findex
src.level = level
case len(n.child) < 4 && !src.rval.IsValid() && isArithmeticAction(src):
case len(n.child) < 4 && isArithmeticAction(src):
// Optimize single assignments from some arithmetic operations.
src.typ = dest.typ
src.findex = dest.findex
src.level = level
n.gen = nop
case src.kind == basicLit && !src.rval.IsValid():
case src.kind == basicLit:
// Assign to nil.
src.rval = reflect.New(dest.typ.TypeOf()).Elem()
}
Expand Down Expand Up @@ -877,11 +877,21 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
// Store result directly to frame output location, to avoid a frame copy.
n.findex = 0
case bname == "cap" && isInConstOrTypeDecl(n):
capConst(n)
switch n.child[1].typ.TypeOf().Kind() {
case reflect.Array, reflect.Chan:
capConst(n)
default:
err = n.cfgErrorf("cap argument is not an array or channel")
}
n.findex = notInFrame
n.gen = nop
case bname == "len" && isInConstOrTypeDecl(n):
lenConst(n)
switch n.child[1].typ.TypeOf().Kind() {
case reflect.Array, reflect.Chan, reflect.String:
lenConst(n)
default:
err = n.cfgErrorf("len argument is not an array, channel or string")
}
n.findex = notInFrame
n.gen = nop
default:
Expand Down Expand Up @@ -2570,18 +2580,16 @@ func isValueUntyped(v reflect.Value) bool {
// isArithmeticAction returns true if the node action is an arithmetic operator.
func isArithmeticAction(n *node) bool {
switch n.action {
case aAdd, aAnd, aAndNot, aBitNot, aMul, aQuo, aRem, aShl, aShr, aSub, aXor:
case aAdd, aAnd, aAndNot, aBitNot, aMul, aNeg, aOr, aPos, aQuo, aRem, aShl, aShr, aSub, aXor:
return true
default:
return false
}
return false
}

func isBoolAction(n *node) bool {
switch n.action {
case aEqual, aGreater, aGreaterEqual, aLand, aLor, aLower, aLowerEqual, aNot, aNotEqual:
return true
default:
return false
}
return false
}
1 change: 1 addition & 0 deletions interp/gta.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func (interp *Interpreter) gta(root *node, rpath, importPath string) ([]*node, e
// values which may be used in further declarations.
if _, err = interp.cfg(n, importPath); err != nil {
// No error processing here, to allow recovery in subtree nodes.
// TODO(marc): check for a non recoverable error and return it for better diagnostic.
err = nil
}

Expand Down
1 change: 1 addition & 0 deletions interp/interp_eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func TestOpVarConst(t *testing.T) {
{src: "b := uint(5); a+b", res: "15"},
{src: "b := uint(5); b+a", res: "15"},
{src: "b := uint(5); b>a", res: "false"},
{src: "const maxlen = cap(aa); var aa = []int{1,2}", err: "1:20: constant definition loop"},
})
}

Expand Down

0 comments on commit 61b4980

Please sign in to comment.