sh: Don't do optimized command substitution if expansions have side effects.

Before considering to execute a command substitution in the same process,
check if any of the expansions may have a side effect; if so, execute it in
a new process just like happens if it is not a single simple command.

Although the check happens at run time, it is a static check that does not
depend on current state. It is triggered by:
- expanding $! (which may cause the job to be remembered)
- ${var=value} default value assignment
- assignment operators in arithmetic
- parameter substitutions in arithmetic except ${#param}, $$, $# and $?
- command substitutions in arithmetic

This means that $((v+1)) does not prevent optimized command substitution,
whereas $(($v+1)) does, because $v might expand to something containing
assignment operators.

Scripts should not depend on these exact details for correctness. It is also
imaginable to have the shell fork if and when a side effect is encountered
or to create a new temporary namespace for variables.

Due to the $! change, the construct $(jobs $!) no longer works. The value of
$! should be stored in a variable outside command substitution first.

bin/sh/eval.c

@@ -94,6 +94,7 @@ static void evalsubshell(union node *, int);
 static void evalredir(union node *, int);
 static void expredir(union node *);
 static void evalpipe(union node *);
+static int is_valid_fast_cmdsubst(union node *n);
 static void evalcommand(union node *, int, struct backcmd *);
 static void prehash(union node *);
 
@@ -565,6 +566,19 @@ evalpipe(union node *n)
 
 
 
+static int
+is_valid_fast_cmdsubst(union node *n)
+{
+	union node *argp;
+
+	if (n->type != NCMD)
+		return 0;
+	for (argp = n->ncmd.args ; argp ; argp = argp->narg.next)
+		if (expandhassideeffects(argp->narg.text))
+			return 0;
+	return 1;
+}
+
 /*
  * Execute a command inside back quotes.  If it's a builtin command, we
  * want to save its output in a block obtained from malloc.  Otherwise
@@ -590,7 +604,7 @@ evalbackcmd(union node *n, struct backcmd *result)
 		exitstatus = 0;
 		goto out;
 	}
-	if (n->type == NCMD) {
+	if (is_valid_fast_cmdsubst(n)) {
 		exitstatus = oexitstatus;
 		savehandler = handler;
 		if (setjmp(jmploc.loc)) {

bin/sh/expand.c

@@ -1569,6 +1569,78 @@ cvtnum(int num, char *buf)
 	return buf;
 }
 
+/*
+ * Check statically if expanding a string may have side effects.
+ */
+int
+expandhassideeffects(const char *p)
+{
+	int c;
+	int arinest;
+
+	arinest = 0;
+	while ((c = *p++) != '\0') {
+		switch (c) {
+		case CTLESC:
+			p++;
+			break;
+		case CTLVAR:
+			c = *p++;
+			/* Expanding $! sets the job to remembered. */
+			if (*p == '!')
+				return 1;
+			if ((c & VSTYPE) == VSASSIGN)
+				return 1;
+			/*
+			 * If we are in arithmetic, the parameter may contain
+			 * '=' which may cause side effects. Exceptions are
+			 * the length of a parameter and $$, $# and $? which
+			 * are always numeric.
+			 */
+			if ((c & VSTYPE) == VSLENGTH) {
+				while (*p != '=')
+					p++;
+				p++;
+				break;
+			}
+			if ((*p == '$' || *p == '#' || *p == '?') &&
+			    p[1] == '=') {
+				p += 2;
+				break;
+			}
+			if (arinest > 0)
+				return 1;
+			break;
+		case CTLBACKQ:
+		case CTLBACKQ | CTLQUOTE:
+			if (arinest > 0)
+				return 1;
+			break;
+		case CTLARI:
+			arinest++;
+			break;
+		case CTLENDARI:
+			arinest--;
+			break;
+		case '=':
+			if (*p == '=') {
+				/* Allow '==' operator. */
+				p++;
+				continue;
+			}
+			if (arinest > 0)
+				return 1;
+			break;
+		case '!': case '<': case '>':
+			/* Allow '!=', '<=', '>=' operators. */
+			if (*p == '=')
+				p++;
+			break;
+		}
+	}
+	return 0;
+}
+
 /*
  * Do most of the work for wordexp(3).
  */

bin/sh/expand.h

@@ -63,4 +63,5 @@ void expari(int);
 int patmatch(const char *, const char *, int);
 void rmescapes(char *);
 int casematch(union node *, const char *);
+int expandhassideeffects(const char *);
 int wordexpcmd(int, char **);

tools/regression/bin/sh/expansion/cmdsubst7.0

@@ -0,0 +1,31 @@
+# $FreeBSD$
+
+failures=''
+ok=''
+
+testcase() {
+	code="$1"
+
+	unset v
+	eval ": \$($code)"
+
+	if [ "${v:+bad}" = "" ]; then
+		ok=x$ok
+	else
+		failures=x$failures
+		echo "Failure for $code"
+	fi
+}
+
+testcase ': ${v=0}'
+testcase ': ${v:=0}'
+testcase ': $((v=1))'
+testcase ': $((v+=1))'
+w='v=1'
+testcase ': $(($w))'
+testcase ': $((${$+v=1}))'
+testcase ': $((v${$+=1}))'
+testcase ': $((v $(echo =) 1))'
+testcase ': $(($(echo $w)))'
+
+test "x$failures" = x