30724: shell code optimisd to use execsimple() doesn't have a valid thisjob

3 files changed, 37 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 9768aa8e0..5a69f0174 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2012-10-11  Peter Stephenson  <pws@csr.com>
+
+	* 30724: Src/exec.c, Src/jobs.c: shell code optimised to use
+	execsimple() doesn't have a valid thisjob.
+
 2012-10-09  Peter Stephenson  <pws@csr.com>
 
 	* users/17318: Src/Zle/zle_utils.c: don't increment the undo
@@ -255,5 +260,5 @@
 
 *****************************************************
 * This is used by the shell to define $ZSH_PATCHLEVEL
-* $Revision: 1.5743 $
+* $Revision: 1.5744 $
 *****************************************************
diff --git a/Src/exec.c b/Src/exec.c
index 0f7c84a9f..b2224cfb3 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -404,7 +404,17 @@ execcursh(Estate state, int do_exec)
     /* Skip word only used for try/always */
     state->pc++;
 
-    if (!list_pipe && thisjob != list_pipe_job && !hasprocs(thisjob))
+    /*
+     * The test thisjob != -1 was added because sometimes thisjob
+     * can be invalid at this point.  The case in question was
+     * in a precmd function after operations involving background
+     * jobs.
+     *
+     * This is because sometimes we bypass job control to execute
+     * very simple functions via execssimple().
+     */
+    if (!list_pipe && thisjob != -1 && thisjob != list_pipe_job &&
+	!hasprocs(thisjob))
 	deletejob(jobtab + thisjob, 0);
     cmdpush(CS_CURSH);
     execlist(state, 1, do_exec);
@@ -1064,7 +1074,7 @@ static int
 execsimple(Estate state)
 {
     wordcode code = *state->pc++;
-    int lv;
+    int lv, otj;
 
     if (errflag)
 	return (lastval = 1);
@@ -1075,6 +1085,13 @@ execsimple(Estate state)
 
     code = wc_code(*state->pc++);
 
+    /*
+     * Because we're bypassing job control, ensure the called
+     * code doesn't see the current job.
+     */
+    otj = thisjob;
+    thisjob = -1;
+
     if (code == WC_ASSIGN) {
 	cmdoutval = 0;
 	addvars(state, state->pc - 1, 0);
@@ -1086,6 +1103,8 @@ execsimple(Estate state)
     } else
 	lv = (execfuncs[code - WC_CURSH])(state, 0);
 
+    thisjob = otj;
+
     return lastval = lv;
 }
 
@@ -4313,7 +4332,9 @@ execshfunc(Shfunc shf, LinkList args)
     if (errflag)
 	return;
 
-    if (!list_pipe && thisjob != list_pipe_job && !hasprocs(thisjob)) {
+    /* thisjob may be invalid if we're called via execsimple: see execcursh */
+    if (!list_pipe && thisjob != -1 && thisjob != list_pipe_job &&
+	!hasprocs(thisjob)) {
 	/* Without this deletejob the process table *
 	 * would be filled by a recursive function. */
 	last_file_list = jobtab[thisjob].filelist;
diff --git a/Src/jobs.c b/Src/jobs.c
index ddd997c49..0464d18d8 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -209,7 +209,13 @@ findproc(pid_t pid, Job *jptr, Process *pptr, int aux)
 int
 hasprocs(int job)
 {
-    Job jn = jobtab + job;
+    Job jn;
+
+    if (job < 0) {
+	DPUTS(1, "job number invalid in hasprocs");
+	return 0;
+    }
+    jn = jobtab + job;
 
     return jn->procs || jn->auxprocs;
 }