PR/48843 CVS commit: src/bin/sh

["Christos Zoulas" <christos%netbsd.org@localhost>]

The following reply was made to PR bin/48843; it has been noted by GNATS.

From: "Christos Zoulas" <christos%netbsd.org@localhost>
To: gnats-bugs%gnats.NetBSD.org@localhost
Cc: 
Subject: PR/48843 CVS commit: src/bin/sh
Date: Sat, 31 May 2014 10:42:19 -0400

 Module Name:   src
 Committed By:  christos
 Date:          Sat May 31 14:42:18 UTC 2014
 
 Modified Files:
        src/bin/sh: eval.c eval.h main.c sh.1
 
 Log Message:
 PR/48843: Jarmo Jaakkola: dot commands mess up scope nesting tracking
 
 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.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.108 -r1.109 src/bin/sh/eval.c
 cvs rdiff -u -r1.15 -r1.16 src/bin/sh/eval.h
 cvs rdiff -u -r1.57 -r1.58 src/bin/sh/main.c
 cvs rdiff -u -r1.112 -r1.113 src/bin/sh/sh.1
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.