NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
bin/48843: sh(1): break/continue/return broken inside dot commands
>Number: 48843
>Category: bin
>Synopsis: dot commands mess up scope nesting tracking
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Wed May 28 23:10:00 +0000 2014
>Originator: Jarmo Jaakkola
>Release: NetBSD 6.1.2_PATCH
>Organization:
>Environment:
System: NetBSD kotoisa.roskakori.fi 6.1.2_PATCH NetBSD 6.1.2_PATCH (KOTOISA)
#5: Mon Jan 20 17:01:44 EET 2014
jammuli%kotoisa.roskakori.fi@localhost:/usr/src/sys/arch/amd64/compile/KOTOISA
amd64
Architecture: x86_64
Machine: amd64
>Description:
Evaluation of commands goes completely haywire if a file containing
a break/continue/return command outside its "intended" scope is sourced
using a dot command inside its "intended" scope. The main symptom is
not exiting from the sourced file when supposed to, leading to evaluation
of commands that were not supposed to be evaluated. A secondary symptom
is that these extra commands are not evaluated correctly, as some of them
are skipped. Some examples are listed in the How-To-Repeat section.
According to the POSIX standard, this is how it should work:
dot:
The shell shall execute commands from the file in the current
environment.
break:
The break utility shall exit from the smallest enclosing for, while,
or until loop, [...]
continue:
The continue utility shall return to the top of the smallest
enclosing for, while, or until loop, [...]
return:
The return utility shall cause the shell to stop executing
the current function or dot script. If the shell is not currently
executing a function or dot script, the results are unspecified.
It is clear that return should return from a sourced file, which
it does not do. Whether break and continue should work from the sourced
file might be debatable. Because the dot command says "in the current
environment", I'd say yes. In any case, it should not fail in weird
ways like it does now!
The problems occur with return (a) and break/continue (b) because:
1) dotcmd() does not record the function nesting level prior to
sourcing the file nor does it touch the loopnest variable,
leading to either
2 a) returncmd() being unable to detect that it should not set
evalskip to SKIPFUNC but SKIPFILE, or
b) breakcmd() setting evalskip to SKIPCONT or SKIPBREAK,
leading to
3) cmdloop() not detecting that it should skip the rest of
the file, due to only checking for SKIPFILE.
The result is that cmdloop() keeps executing lines from the file
whilst evalskip is set, which is the main symptom. Because
evalskip is checked in multiple places in eval.c, the secondary
symptom appears.
>How-To-Repeat:
Run the following script:
printf "break\necho break1; echo break2" >break
printf "continue\necho continue1; echo continue2" >continue
printf "return\necho return1; echo return2" >return
while true; do . ./break; done
for i in 1 2; do . ./continue; done
func() {
. ./return
}
func
No output should be produced, but instead this is the result:
break1
continue1
continue1
return1
The main symptom is evident from the unexpected output and the secondary
one from the fact that there are no lines with '2' in them.
>Fix:
Here is patch to src/bin/sh to fix the above problems. It keeps
track of the function nesting level at the beginning of a dot command
to enable the return command to work properly.
I also changed the undefined-by-standard functionality of the return
command when it's not in a dot command or function from (indirectly)
exiting the shell to being silently ignored. This was done because
the previous way has at least one bug: the shell exits without asking
for confirmation when there are stopped jobs.
Because I read the standard to mean that break and continue should have
an effect outside the sourced file, that's how I implemented it. For what
it's worth, this also seems to be what bash does. Also laziness, because
this way required no changes to loopnesting tracking. If this is not
wanted, it might make sense to move the nesting tracking to the inputfile
stack.
The patch also does some clean-up to reduce the amount of global
variables by moving the dotcmd() and the find_dot_file() functions from
main.c to eval.c and making in_function() a proper function.
Index: bin/sh/eval.c
===================================================================
RCS file: /cvsroot/src/bin/sh/eval.c,v
retrieving revision 1.103
diff -u -p -u -r1.103 eval.c
--- bin/sh/eval.c 14 Nov 2011 18:24:45 -0000 1.103
+++ bin/sh/eval.c 28 May 2014 22:51:38 -0000
@@ -88,11 +88,20 @@ __RCSID("$NetBSD: eval.c,v 1.103 2011/11
#define EV_TESTED 02 /* exit status is checked; ignore -e flag */
#define EV_BACKCMD 04 /* command executing within back quotes */
-int evalskip; /* set if we are skipping commands */
+STATIC enum skipstate evalskip; /* != SKIPNONE if we are skipping
commands */
STATIC int skipcount; /* number of levels to skip */
-MKINIT int loopnest; /* current loop nesting level */
-int funcnest; /* depth of function calls */
+STATIC int loopnest; /* current loop nesting level */
+STATIC int funcnest; /* depth of function calls */
STATIC int builtin_flags; /* evalcommand flags for builtins */
+/*
+ * Base function nesting level inside a dot command. Set to 0 initially
+ * and to (funcnest + 1) before every dot command to enable
+ * 1) detection of being in a file sourced by a dot command and
+ * 2) counting of function nesting in that file for the implementation
+ * of the return command.
+ * The value is reset to its previous value after the dot command.
+ */
+STATIC int dot_funcnest;
const char *commandname;
@@ -110,6 +119,7 @@ STATIC void evalpipe(union node *);
STATIC void evalcommand(union node *, int, struct backcmd *);
STATIC void prehash(union node *);
+STATIC char *find_dot_file(char *);
/*
* Called to reset things after an exception.
@@ -119,9 +129,7 @@ STATIC void prehash(union node *);
INCLUDE "eval.h"
RESET {
- evalskip = 0;
- loopnest = 0;
- funcnest = 0;
+ reset_eval();
}
SHELLPROC {
@@ -129,6 +137,15 @@ SHELLPROC {
}
#endif
+void
+reset_eval(void)
+{
+ evalskip = SKIPNONE;
+ dot_funcnest = 0;
+ loopnest = 0;
+ funcnest = 0;
+}
+
static int
sh_pipe(int fds[2])
{
@@ -326,11 +343,11 @@ evalloop(union node *n, int flags)
evaltree(n->nbinary.ch1, EV_TESTED);
if (evalskip) {
skipping: if (evalskip == SKIPCONT && --skipcount <= 0) {
- evalskip = 0;
+ evalskip = SKIPNONE;
continue;
}
if (evalskip == SKIPBREAK && --skipcount <= 0)
- evalskip = 0;
+ evalskip = SKIPNONE;
break;
}
if (n->type == NWHILE) {
@@ -376,11 +393,11 @@ evalfor(union node *n, int flags)
status = exitstatus;
if (evalskip) {
if (evalskip == SKIPCONT && --skipcount <= 0) {
- evalskip = 0;
+ evalskip = SKIPNONE;
continue;
}
if (evalskip == SKIPBREAK && --skipcount <= 0)
- evalskip = 0;
+ evalskip = SKIPNONE;
break;
}
}
@@ -961,7 +978,7 @@ normal_fork:
popredir();
INTON;
if (evalskip == SKIPFUNC) {
- evalskip = 0;
+ evalskip = SKIPNONE;
skipcount = 0;
}
if (flags & EV_EXIT)
@@ -1102,7 +1119,24 @@ prehash(union node *n)
pathval());
}
+STATIC int
+in_function(void)
+{
+ return funcnest;
+}
+
+STATIC enum skipstate
+current_skipstate(void)
+{
+ return evalskip;
+}
+STATIC void
+stop_skipping(void)
+{
+ evalskip = SKIPNONE;
+ skipcount = 0;
+}
/*
* Builtin commands. Builtin commands whose functions are closely
@@ -1149,9 +1183,84 @@ breakcmd(int argc, char **argv)
return 0;
}
+int
+dotcmd(int argc, char **argv)
+{
+ exitstatus = 0;
+
+ if (argc >= 2) { /* That's what SVR2 does */
+ char *fullname;
+ /*
+ * dot_funcnest needs to be 0 when not in a dotcmd, so it
+ * cannot be restored with (funcnest + 1).
+ */
+ int dot_funcnest_old;
+ struct stackmark smark;
+
+ setstackmark(&smark);
+ fullname = find_dot_file(argv[1]);
+ setinputfile(fullname, 1);
+ commandname = fullname;
+ dot_funcnest_old = dot_funcnest;
+ dot_funcnest = funcnest + 1;
+ cmdloop(0);
+ dot_funcnest = dot_funcnest_old;
+ popfile();
+ popstackmark(&smark);
+ }
+ return exitstatus;
+}
+
+/*
+ * Take commands from a file. To be compatible we should do a path
+ * search for the file, which is necessary to find sub-commands.
+ */
+
+STATIC char *
+find_dot_file(char *basename)
+{
+ char *fullname;
+ const char *path = pathval();
+ struct stat statb;
+
+ /* don't try this for absolute or relative paths */
+ if (strchr(basename, '/'))
+ return basename;
+
+ while ((fullname = padvance(&path, basename)) != NULL) {
+ if ((stat(fullname, &statb) == 0) && S_ISREG(statb.st_mode)) {
+ /*
+ * Don't bother freeing here, since it will
+ * be freed by the caller.
+ */
+ return fullname;
+ }
+ stunalloc(fullname);
+ }
+
+ /* not found in the PATH */
+ error("%s: not found", basename);
+ /* NOTREACHED */
+}
+
+
/*
* The return command.
+ *
+ * Quoth the POSIX standard:
+ * The return utility shall cause the shell to stop executing the current
+ * function or dot script. If the shell is not currently executing
+ * a function or dot script, the results are unspecified.
+ *
+ * As for the unspecified part, there seems to be no de-facto standard: bash
+ * ignores the return with a warning, zsh ignores the return in interactive
+ * mode but seems to liken it to exit in a script. (checked May 2014)
+ *
+ * We choose to silently ignore the return. Older versions of this shell
+ * set evalskip to SKIPFILE causing the shell to (indirectly) exit. This
+ * had at least the problem of circumventing the check for stopped jobs,
+ * which would occur for exit or ^D.
*/
int
@@ -1159,17 +1268,19 @@ returncmd(int argc, char **argv)
{
int ret = argc > 1 ? number(argv[1]) : exitstatus;
- if (funcnest) {
+ if ((dot_funcnest == 0 && funcnest)
+ || (dot_funcnest > 0 && funcnest - (dot_funcnest - 1) > 0)) {
evalskip = SKIPFUNC;
skipcount = 1;
- return ret;
- }
- else {
- /* Do what ksh does; skip the rest of the file */
+ } else if (dot_funcnest > 0) {
evalskip = SKIPFILE;
skipcount = 1;
- return ret;
+ } else {
+ /* XXX: should a warning be issued? */
+ ret = 0;
}
+
+ return ret;
}
Index: bin/sh/eval.h
===================================================================
RCS file: /cvsroot/src/bin/sh/eval.h,v
retrieving revision 1.15
diff -u -p -u -r1.15 eval.h
--- bin/sh/eval.h 15 Feb 2008 17:26:06 -0000 1.15
+++ bin/sh/eval.h 28 May 2014 22:51:38 -0000
@@ -53,12 +53,21 @@ void evaltree(union node *, int);
void evalbackcmd(union node *, struct backcmd *);
/* in_function returns nonzero if we are currently evaluating a function */
-#define in_function() funcnest
-extern int funcnest;
-extern int evalskip;
+int in_function(void); /* return non-zero, if evaluating a function */
/* reasons for skipping commands (see comment on breakcmd routine) */
-#define SKIPBREAK 1
-#define SKIPCONT 2
-#define SKIPFUNC 3
-#define SKIPFILE 4
+enum skipstate {
+ SKIPNONE = 0, /* not skipping */
+ SKIPBREAK, /* break */
+ SKIPCONT, /* continue */
+ SKIPFUNC, /* return in a function */
+ SKIPFILE /* return in a dot command */
+};
+
+enum skipstate current_skipstate(void);
+void stop_skipping(void); /* reset internal skipping state to SKIPNONE */
+
+/*
+ * Only for use by reset() in init.c!
+ */
+void reset_eval(void);
Index: bin/sh/main.c
===================================================================
RCS file: /cvsroot/src/bin/sh/main.c,v
retrieving revision 1.57
diff -u -p -u -r1.57 main.c
--- bin/sh/main.c 18 Jun 2011 21:18:46 -0000 1.57
+++ bin/sh/main.c 28 May 2014 22:51:38 -0000
@@ -89,7 +89,6 @@ extern int etext();
#endif
STATIC void read_profile(const char *);
-STATIC char *find_dot_file(char *);
int main(int, char **);
/*
@@ -239,6 +238,7 @@ cmdloop(int top)
struct stackmark smark;
int inter;
int numeof = 0;
+ enum skipstate skip;
TRACE(("cmdloop(%d) called\n", top));
setstackmark(&smark);
@@ -270,8 +270,18 @@ cmdloop(int top)
}
popstackmark(&smark);
setstackmark(&smark);
- if (evalskip == SKIPFILE) {
- evalskip = 0;
+
+ /*
+ * Any SKIP* can occur here! SKIP(FUNC|BREAK|CONT) occur when
+ * a dotcmd is in a loop or a function body and appropriate
+ * built-ins occurs in file scope in the sourced file. Values
+ * other than SKIPFILE are reset by the appropriate eval*()
+ * that contained the dotcmd() call.
+ */
+ skip = current_skipstate();
+ if (skip != SKIPNONE) {
+ if (skip == SKIPFILE)
+ stop_skipping();
break;
}
}
@@ -337,60 +347,6 @@ readcmdfile(char *name)
-/*
- * Take commands from a file. To be compatible we should do a path
- * search for the file, which is necessary to find sub-commands.
- */
-
-
-STATIC char *
-find_dot_file(char *basename)
-{
- char *fullname;
- const char *path = pathval();
- struct stat statb;
-
- /* don't try this for absolute or relative paths */
- if (strchr(basename, '/'))
- return basename;
-
- while ((fullname = padvance(&path, basename)) != NULL) {
- if ((stat(fullname, &statb) == 0) && S_ISREG(statb.st_mode)) {
- /*
- * Don't bother freeing here, since it will
- * be freed by the caller.
- */
- return fullname;
- }
- stunalloc(fullname);
- }
-
- /* not found in the PATH */
- error("%s: not found", basename);
- /* NOTREACHED */
-}
-
-int
-dotcmd(int argc, char **argv)
-{
- exitstatus = 0;
-
- if (argc >= 2) { /* That's what SVR2 does */
- char *fullname;
- struct stackmark smark;
-
- setstackmark(&smark);
- fullname = find_dot_file(argv[1]);
- setinputfile(fullname, 1);
- commandname = fullname;
- cmdloop(0);
- popfile();
- popstackmark(&smark);
- }
- return exitstatus;
-}
-
-
int
exitcmd(int argc, char **argv)
{
Index: bin/sh/sh.1
===================================================================
RCS file: /cvsroot/src/bin/sh/sh.1,v
retrieving revision 1.106
diff -u -p -u -r1.106 sh.1
--- bin/sh/sh.1 5 Oct 2011 13:15:30 -0000 1.106
+++ bin/sh/sh.1 28 May 2014 22:51:38 -0000
@@ -1179,10 +1179,23 @@ be built in for efficiency (e.g.
.Xr test 1 ,
etc).
.Bl -tag -width 5n
-.It :
+.It : [ Ar arg ... ]
A null command that returns a 0 (true) exit value.
+Any arguments are ignored.
.It \&. file
-The commands in the specified file are read and executed by the shell.
+The dot command reads and executes the commands from the specified
+.Ar file
+in the current shell environment.
+The file does not need to be executable and is looked up from the directories
+listed in the
+.Ev PATH
+variable if it does not contain a directory separator
+.Pq Sq / .
+The return command can be used for a premature return from the sourced file.
+.Pp
+A non-obvious consequence of the file executing in the current environment
+is that loop control keywords (continue and break) can be used in the file
+to control loops surrounding the dot command.
.It alias Op Ar name Ns Op Ar "=string ..."
If
.Ar name=string
@@ -1620,6 +1633,19 @@ With the
.Fl p
option specified the output will be formatted suitably for non-interactive use.
.Pp
+.It return [ Ar n ]
+Stop executing the current function or a dot command with return value of
+.Ar n
+or the value of the last executed command, if not specified.
+For portability,
+.Ar n
+should be in the range from 0 to 255.
+.Pp
+The effects of using a return command outside a function or a dot command
+are not standardized.
+This implementation (currently) treats such a return as a no-op with
+a return value of 0 (success, true).
+Use the exit command if you want to return from a script or exit your shell.
.It set Oo { Fl options | Cm +options | Cm \-- } Oc Ar arg ...
The
.Ic set
Home |
Main Index |
Thread Index |
Old Index