From owner-svn-src-all@FreeBSD.ORG Tue Dec 28 21:27:08 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 787851065670; Tue, 28 Dec 2010 21:27:08 +0000 (UTC) (envelope-from jilles@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 6695D8FC0C; Tue, 28 Dec 2010 21:27:08 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id oBSLR8AM077726; Tue, 28 Dec 2010 21:27:08 GMT (envelope-from jilles@svn.freebsd.org) Received: (from jilles@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id oBSLR8R9077721; Tue, 28 Dec 2010 21:27:08 GMT (envelope-from jilles@svn.freebsd.org) Message-Id: <201012282127.oBSLR8R9077721@svn.freebsd.org> From: Jilles Tjoelker Date: Tue, 28 Dec 2010 21:27:08 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r216778 - in head: bin/sh tools/regression/bin/sh/expansion X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Dec 2010 21:27:08 -0000 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