expand: avoid panics on division by zero

expand/arith.go

@@ -92,7 +92,7 @@ func Arithm(cfg *Config, expr syntax.ArithmExpr) (int, error) {
 		if err != nil {
 			return 0, err
 		}
-		return binArit(x.Op, left, right), nil
+		return binArit(x.Op, left, right)
 	default:
 		panic(fmt.Sprintf("unexpected arithm expr: %T", x))
 	}
@@ -129,8 +129,14 @@ func (cfg *Config) assgnArit(b *syntax.BinaryArithm) (int, error) {
 	case syntax.MulAssgn:
 		val *= arg
 	case syntax.QuoAssgn:
+		if arg == 0 {
+			return 0, fmt.Errorf("division by zero")
+		}
 		val /= arg
 	case syntax.RemAssgn:
+		if arg == 0 {
+			return 0, fmt.Errorf("division by zero")
+		}
 		val %= arg
 	case syntax.AndAssgn:
 		val &= arg
@@ -161,48 +167,54 @@ func intPow(a, b int) int {
 	return p
 }
 
-func binArit(op syntax.BinAritOperator, x, y int) int {
+func binArit(op syntax.BinAritOperator, x, y int) (int, error) {
 	switch op {
 	case syntax.Add:
-		return x + y
+		return x + y, nil
 	case syntax.Sub:
-		return x - y
+		return x - y, nil
 	case syntax.Mul:
-		return x * y
+		return x * y, nil
 	case syntax.Quo:
-		return x / y
+		if y == 0 {
+			return 0, fmt.Errorf("division by zero")
+		}
+		return x / y, nil
 	case syntax.Rem:
-		return x % y
+		if y == 0 {
+			return 0, fmt.Errorf("division by zero")
+		}
+		return x % y, nil
 	case syntax.Pow:
-		return intPow(x, y)
+		return intPow(x, y), nil
 	case syntax.Eql:
-		return oneIf(x == y)
+		return oneIf(x == y), nil
 	case syntax.Gtr:
-		return oneIf(x > y)
+		return oneIf(x > y), nil
 	case syntax.Lss:
-		return oneIf(x < y)
+		return oneIf(x < y), nil
 	case syntax.Neq:
-		return oneIf(x != y)
+		return oneIf(x != y), nil
 	case syntax.Leq:
-		return oneIf(x <= y)
+		return oneIf(x <= y), nil
 	case syntax.Geq:
-		return oneIf(x >= y)
+		return oneIf(x >= y), nil
 	case syntax.And:
-		return x & y
+		return x & y, nil
 	case syntax.Or:
-		return x | y
+		return x | y, nil
 	case syntax.Xor:
-		return x ^ y
+		return x ^ y, nil
 	case syntax.Shr:
-		return x >> uint(y)
+		return x >> uint(y), nil
 	case syntax.Shl:
-		return x << uint(y)
+		return x << uint(y), nil
 	case syntax.AndArit:
-		return oneIf(x != 0 && y != 0)
+		return oneIf(x != 0 && y != 0), nil
 	case syntax.OrArit:
-		return oneIf(x != 0 || y != 0)
+		return oneIf(x != 0 || y != 0), nil
 	default: // syntax.Comma
 		// x is executed but its result discarded
-		return y
+		return y, nil
 	}
 }

interp/api.go

@@ -4,6 +4,13 @@
 // Package interp implements an interpreter that executes shell
 // programs. It aims to support POSIX, but its support is not complete
 // yet. It also supports some Bash features.
+//
+// The interpreter generally aims to behave like Bash,
+// but it does not support all of its features.
+//
+// The interpreter currently aims to behave like a non-interactive shell,
+// which is how most shells run scripts, and is more useful to machines.
+// In the future, it may gain an option to behave like an interactive shell.
 package interp
 
 import (

interp/handler.go

@@ -67,6 +67,10 @@ type HandlerContext struct {
 // Returning a non-nil error will halt the Runner.
 type CallHandlerFunc func(ctx context.Context, args []string) ([]string, error)
 
+// TODO: consistently treat handler errors as non-fatal by default,
+// but have an interface or API to specify fatal errors which should make
+// the shell exit with a particular status code.
+
 // ExecHandlerFunc is a handler which executes simple commands.
 // It is called for all CallExpr nodes where the first argument is neither a
 // declared function nor a builtin.

interp/handler_test.go

@@ -21,29 +21,29 @@ import (
 	"mvdan.cc/sh/v3/syntax"
 )
 
-func blacklistBuiltinExec(name string) ExecHandlerFunc {
+func blocklistBuiltinExec(name string) ExecHandlerFunc {
 	return func(ctx context.Context, args []string) error {
 		if args[0] == name {
-			return fmt.Errorf("%s: blacklisted builtin", name)
+			return fmt.Errorf("%s: blocklisted builtin", name)
 		}
 		return testExecHandler(ctx, args)
 	}
 }
 
-func blacklistAllExec(ctx context.Context, args []string) error {
-	return fmt.Errorf("blacklisted: %s", args[0])
+func blocklistAllExec(ctx context.Context, args []string) error {
+	return fmt.Errorf("blocklisted: %s", args[0])
 }
 
-func blacklistNondevOpen(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) {
+func blocklistNondevOpen(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) {
 	if path != "/dev/null" {
 		return nil, fmt.Errorf("non-dev: %s", path)
 	}
 
 	return testOpenHandler(ctx, path, flags, mode)
 }
 
-func blacklistGlob(ctx context.Context, path string) ([]os.FileInfo, error) {
-	return nil, fmt.Errorf("blacklisted: glob")
+func blocklistGlob(ctx context.Context, path string) ([]os.FileInfo, error) {
+	return nil, fmt.Errorf("blocklisted: glob")
 }
 
 // runnerCtx allows us to give handler functions access to the Runner, if needed.
@@ -67,40 +67,40 @@ var modCases = []struct {
 	want    string
 }{
 	{
-		name: "ExecBlacklist",
-		exec: blacklistBuiltinExec("sleep"),
+		name: "ExecBlocklistOne",
+		exec: blocklistBuiltinExec("sleep"),
 		src:  "echo foo; sleep 1",
-		want: "foo\nsleep: blacklisted builtin",
+		want: "foo\nsleep: blocklisted builtin",
 	},
 	{
-		name: "ExecWhitelist",
-		exec: blacklistBuiltinExec("faa"),
+		name: "ExecBlocklistOneSubshell",
+		exec: blocklistBuiltinExec("faa"),
 		src:  "a=$(echo foo | sed 's/o/a/g'); echo $a; $a args",
-		want: "faa\nfaa: blacklisted builtin",
+		want: "faa\nfaa: blocklisted builtin",
 	},
 	{
-		name: "ExecSubshell",
-		exec: blacklistAllExec,
+		name: "ExecBlocklistAllSubshell",
+		exec: blocklistAllExec,
 		src:  "(malicious)",
-		want: "blacklisted: malicious",
+		want: "blocklisted: malicious",
 	},
 	{
 		name: "ExecPipe",
-		exec: blacklistAllExec,
+		exec: blocklistAllExec,
 		src:  "malicious | echo foo",
-		want: "foo\nblacklisted: malicious",
+		want: "foo\nblocklisted: malicious",
 	},
 	{
 		name: "ExecCmdSubst",
-		exec: blacklistAllExec,
+		exec: blocklistAllExec,
 		src:  "a=$(malicious)",
-		want: "blacklisted: malicious\nexit status 1",
+		want: "blocklisted: malicious\n", // TODO: why the newline?
 	},
 	{
 		name: "ExecBackground",
-		exec: blacklistAllExec,
+		exec: blocklistAllExec,
 		src:  "{ malicious; true; } & { malicious; true; } & wait",
-		want: "blacklisted: malicious",
+		want: "blocklisted: malicious",
 	},
 	{
 		name: "ExecBuiltin",
@@ -110,13 +110,13 @@ var modCases = []struct {
 	},
 	{
 		name: "OpenForbidNonDev",
-		open: blacklistNondevOpen,
+		open: blocklistNondevOpen,
 		src:  "echo foo >/dev/null; echo bar >/tmp/x",
 		want: "non-dev: /tmp/x",
 	},
 	{
 		name: "CallReplaceWithBlank",
-		open: blacklistNondevOpen,
+		open: blocklistNondevOpen,
 		call: func(ctx context.Context, args []string) ([]string, error) {
 			return []string{"echo", "blank"}, nil
 		},
@@ -144,9 +144,9 @@ var modCases = []struct {
 	},
 	{
 		name:    "GlobForbid",
-		readdir: blacklistGlob,
+		readdir: blocklistGlob,
 		src:     "echo *",
-		want:    "blacklisted: glob\nexit status 1",
+		want:    "blocklisted: glob\n",
 	},
 }
 

interp/interp_test.go

@@ -1818,6 +1818,14 @@ var runTests = []runTest{
 		"a=b b=a; echo $(($a))",
 		"0\n #IGNORE bash prints a warning",
 	},
+	{
+		"let x=3; let 3/0; ((3/0)); echo $((x/y)); let x/=0",
+		"division by zero\ndivision by zero\ndivision by zero\ndivision by zero\nexit status 1 #JUSTERR",
+	},
+	{
+		"let x=3; let 3%0; ((3%0)); echo $((x%y)); let x%=0",
+		"division by zero\ndivision by zero\ndivision by zero\ndivision by zero\nexit status 1 #JUSTERR",
+	},
 
 	// set/shift
 	{
@@ -2307,7 +2315,7 @@ set +o pipefail
 	},
 	{
 		`a=(b); echo ${a[-2]}`,
-		"negative array index\nexit status 1 #JUSTERR",
+		"negative array index\n #JUSTERR",
 	},
 	// TODO: also test with gaps in arrays.
 	{

interp/runner.go

@@ -6,6 +6,7 @@ package interp
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"math"
@@ -155,7 +156,19 @@ func (r *Runner) updateExpandOpts() {
 
 func (r *Runner) expandErr(err error) {
 	if err != nil {
-		r.errf("%v\n", err)
+		errMsg := err.Error()
+		fmt.Fprintln(r.stderr, errMsg)
+		switch {
+		case errors.As(err, &expand.UnsetParameterError{}):
+		case errMsg == "invalid indirect expansion":
+			// TODO: These errors are treated as fatal by bash.
+			// Make the error type reflect that.
+		case strings.HasSuffix(errMsg, "not supported"):
+			// TODO: This "has suffix" is a temporary measure until the expand
+			// package supports all syntax nodes like extended globbing.
+		default:
+			return // other cases do not exit
+		}
 		r.exitShell(context.TODO(), 1)
 	}
 }