svn commit: r216778 - in head: bin/sh tools/regression/bin/sh/expansion

[Jilles Tjoelker]

Author: jilles
Date: Tue Dec 28 21:27:08 2010
New Revision: 216778
URL: http://svn.freebsd.org/changeset/base/216778

Log:
  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.

Added:
  head/tools/regression/bin/sh/expansion/cmdsubst7.0   (contents, props changed)
Modified:
  head/bin/sh/eval.c
  head/bin/sh/expand.c
  head/bin/sh/expand.h

Modified: head/bin/sh/eval.c
==============================================================================
--- head/bin/sh/eval.c	Tue Dec 28 21:22:08 2010	(r216777)
+++ head/bin/sh/eval.c	Tue Dec 28 21:27:08 2010	(r216778)
@@ -94,6 +94,7 @@ static void evalsubshell(union node *, i
 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 backcm
 		exitstatus = 0;
 		goto out;
 	}
-	if (n->type == NCMD) {
+	if (is_valid_fast_cmdsubst(n)) {
 		exitstatus = oexitstatus;
 		savehandler = handler;
 		if (setjmp(jmploc.loc)) {

Modified: head/bin/sh/expand.c
==============================================================================
--- head/bin/sh/expand.c	Tue Dec 28 21:22:08 2010	(r216777)
+++ head/bin/sh/expand.c	Tue Dec 28 21:27:08 2010	(r216778)
@@ -1570,6 +1570,78 @@ cvtnum(int num, char *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).
  */
 

Modified: head/bin/sh/expand.h
==============================================================================
--- head/bin/sh/expand.h	Tue Dec 28 21:22:08 2010	(r216777)
+++ head/bin/sh/expand.h	Tue Dec 28 21:27:08 2010	(r216778)
@@ -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 **);

Added: head/tools/regression/bin/sh/expansion/cmdsubst7.0
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/tools/regression/bin/sh/expansion/cmdsubst7.0	Tue Dec 28 21:27:08 2010	(r216778)
@@ -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