54294: Fix %- (prevjob) picking wrong job after resuming

Also corrects exit status of "wait %%" (TBD from workers/43945)

3 files changed, 55 insertions, 18 deletions

diff --git a/Src/jobs.c b/Src/jobs.c
index 9714f4eb9..43148fc3c 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -523,6 +523,10 @@ update_job(Job jn)
 		 * fg/bg is the superjob) a SIGCONT if we need it.
 		 */
 		sjn->stat |= STAT_CHANGED | STAT_STOPPED;
+		if (!(sjn->stat & STAT_DONE)) {
+		    prevjob = curjob;
+		    curjob = i;
+		}
 		if (isset(NOTIFY) && (sjn->stat & STAT_LOCKED) &&
 		    !(sjn->stat & STAT_NOPRINT)) {
 		    /*
@@ -701,13 +705,15 @@ setprevjob(void)
 
     for (i = maxjob; i; i--)
 	if ((jobtab[i].stat & STAT_INUSE) && (jobtab[i].stat & STAT_STOPPED) &&
-	    !(jobtab[i].stat & STAT_SUBJOB) && i != curjob && i != thisjob) {
+	    !(jobtab[i].stat & (STAT_SUBJOB | STAT_NOPRINT)) &&
+	    i != curjob && i != thisjob) {
 	    prevjob = i;
 	    return;
 	}
 
     for (i = maxjob; i; i--)
-	if ((jobtab[i].stat & STAT_INUSE) && !(jobtab[i].stat & STAT_SUBJOB) &&
+	if ((jobtab[i].stat & STAT_INUSE) &&
+	    !(jobtab[i].stat & (STAT_SUBJOB | STAT_NOPRINT)) &&
 	    i != curjob && i != thisjob) {
 	    prevjob = i;
 	    return;
@@ -2073,7 +2079,7 @@ getjob(const char *s, const char *prog)
     if (*s == '%' || *s == '+' || !*s) {
 	if (curjob == -1) {
 	    if (prog && !isset(POSIXBUILTINS))
-		zwarnnam(prog, "no current job");
+		zwarnnam(prog, "%%%c: no such job", *s ? *s : '%');
 	    returnval = -1;
 	    goto done;
 	}
@@ -2084,7 +2090,7 @@ getjob(const char *s, const char *prog)
     if (*s == '-') {
 	if (prevjob == -1) {
 	    if (prog && !isset(POSIXBUILTINS))
-		zwarnnam(prog, "no previous job");
+		zwarnnam(prog, "%%-: no such job");
 	    returnval = -1;
 	    goto done;
 	}
@@ -2638,15 +2644,21 @@ bin_fg(char *name, char **argv, Options ops, int func)
 	    }
 	    /* It's time to shuffle the jobs around!  Reset the current job,
 	    and pick a sensible secondary job. */
-	    if (curjob == job) {
-		curjob = prevjob;
-		prevjob = (func == BIN_BG) ? -1 : job;
-	    }
-	    if (prevjob == job || prevjob == -1)
-		setprevjob();
-	    if (curjob == -1) {
-		curjob = prevjob;
-		setprevjob();
+	    {
+		/* Exclude this job from setprevjob() consideration. */
+		int saved_thisjob = thisjob;
+		thisjob = job;
+		if (curjob == job) {
+		    curjob = prevjob;
+		    prevjob = (func == BIN_BG) ? -1 : job;
+		}
+		if (prevjob == job || prevjob == -1)
+		    setprevjob();
+		if (curjob == -1) {
+		    curjob = prevjob;
+		    setprevjob();
+		}
+		thisjob = saved_thisjob;
 	    }
 	    if (func != BIN_WAIT)
 		/* for bg and fg -- show the job we are operating on */
diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
index 07a24f9c8..57d6b402e 100644
--- a/Test/A05execution.ztst
+++ b/Test/A05execution.ztst
@@ -375,7 +375,7 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
 ?(eval):wait:13: job not found: ?bar
 
 # Test 'wait' for unknown job/process ID (POSIX mode).
-  (setopt POSIX_BUILTINS
+  () { setopt localoptions POSIX_BUILTINS
   wait 1
   echo $?
   wait %%
@@ -388,16 +388,15 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
   echo $?
   wait %foo
   echo $?
-  wait %\?bar)
+  wait %\?bar
+  }
 127:'wait' exit status for unknown ID (POSIX mode)
 >127
->0
 >127
 >127
 >127
 >127
-# TBD: the 0 above is believed to be bogus and should also be turned
-# into 127 when the ccorresponding bug is fixed in the main shell.
+>127
 
   sleep 2 & pid=$!
   kill -STOP $pid
diff --git a/Test/W02jobs.ztst b/Test/W02jobs.ztst
index f38e90dcd..25e187257 100644
--- a/Test/W02jobs.ztst
+++ b/Test/W02jobs.ztst
@@ -304,6 +304,32 @@
 *>fg: no job control in this shell
 *>bg: no job control in this shell
 
+# The following test exercises a bug where setprevjob() could pick an
+# internal NOPRINT job (e.g. a builtin's own job entry) as the previous
+# job.  This happened when a function superjob was involved, because the
+# superjob's subjob was excluded but the builtin job was not.
+# The function's child stops itself, triggering the superjob mechanism.
+# Then fg %- resumes the superjob (which exits since the child is done).
+# Afterwards, the remaining sleep job should still be accessible.
+  zpty_start
+  zpty_input "f() { sh -c 'kill -TSTP 0' }"
+  zpty_input 'f'
+  zpty_line			# consume stopped message from f
+  zpty_input 'sleep 100 &'
+  zpty_line			# consume [N] PID
+  zpty_input 'kill -STOP %sleep'
+  zpty_line			# consume stopped message from kill
+  zpty_input 'fg %-'
+  zpty_line			# consume continued message
+  zpty_input 'jobs'
+  zpty_stop
+0:fg %- with function superjob does not pick internal job as previous
+*>zsh:*(stopped|suspended)*
+*>\[[0-9]##\] [0-9]##
+*>\[[0-9]##\]  + (stopped|suspended)*sleep*
+*>\[[0-9]##\]    continued*
+*>\[[0-9]##\]  - (stopped|suspended)*sleep*
+
 %clean
 
   zmodload -ui zsh/zpty