Bug 531353

This bug was reported a while ago:
  http://mx.gw.com/pipermail/tcsh-bugs/2006-June/000444.html

However, it's still a bug with the latest upstream release, version 6.17.00 from July 2009.
  ftp://ftp.astron.com/pub/tcsh/tcsh-6.17.00.tar.gz

Comparing RHEL 4 and RHEL 5 tcsh code, this chunk is new:

--- RHEL-4/tcsh-6.13.00/sh.sem.c        2009-10-27 15:47:02.906665630 -0500   
+++ RHEL-5/tcsh-6.14.00/sh.sem.c        2009-10-27 15:47:07.391667628 -0500
@@ -656,6 +652,13 @@ execute(t, wanttty, pipein, pipeout, do_                   
            func(t, bifunc);                                                    
            if (forked)                                                         
                exitstat();                                                     
+           else {                                                              
+               if (adrof(STRprintexitvalue)) {                                 
+                   int rv = getn(varval(STRstatus));                           
+                   if (rv != 0)                                                
+                       xprintf(CGETS(17, 2, "Exit %d\n"), rv);                 
+               }                                                               
+           }                                                                   
            break;                                                              
        }                                                                       
        if (t->t_dtyp != NODE_PAREN) {


Now, compare that new chunk to the pjwait() function in sh.proc.c:
    /*
     * Don't report on backquoted jobs, cause it will mess up
     * their output.
     */
    if ((reason != 0) && (adrof(STRprintexitvalue)) &&
        (pp->p_flags & PBACKQ) == 0)
        xprintf(CGETS(17, 2, "Exit %d\n"), reason);


The new chunk above does not check pp->p_flags for the PBACKQ flag; I wonder if it should?

Ignore comment 2...  I just tested for backquotes in that chunk but that didn't solve the bug.

diff --git a/sh.sem.c b/sh.sem.c
index 0210e54..7fe828e 100644
--- a/sh.sem.c
+++ b/sh.sem.c
@@ -653,7 +653,8 @@ execute(t, wanttty, pipein, pipeout, do_glob)
            if (forked)
                exitstat();
            else {
-               if (adrof(STRprintexitvalue)) {
+               if ((adrof(STRprintexitvalue)) &&
+                    ((t->t_dflg & F_BACKQ) == 0)) {
                    int rv = getn(varval(STRstatus));
                    if (rv != 0)
                        xprintf(CGETS(17, 2, "Exit %d\n"), rv);

Well, maybe there is something to the above chunk after all.  I ran tcsh through gdb and set breakpoints on all the print statements that print an 'Exit %d' type string, and it stopped on the xprintf in the new chunk:


$ rpm -q tcsh tcsh-debuginfo
tcsh-6.14-14.el5.x86_64
tcsh-debuginfo-6.14-14.el5.x86_64

$ cat tcsh-gdb-cmds.txt
b sh.proc.c:640
b sh.proc.c:1160
b sh.sem.c:659
run -f

$ gdb -q -x tcsh-gdb-cmds.txt /bin/tcsh
Breakpoint 1 at 0x41e86a: file sh.proc.c, line 640.
Breakpoint 2 at 0x41d18e: file sh.proc.c, line 1160.
Breakpoint 3 at 0x41fb06: file sh.sem.c, line 659.
> set printexitvalue
> set i=`/bin/false`
Detaching after fork from child process 19285.

Breakpoint 3, execute (t=0xb395930, wanttty=19279, pipein=0x0, pipeout=0x0,
    do_glob=1) at sh.sem.c:659
659                             xprintf(CGETS(17, 2, "Exit %d\n"), rv);
(gdb) print rv
$1 = 1
(gdb) c
Continuing.
Exit 1
> exit

Program exited with code 01.
(gdb)

I did some more testing and I see how the two xprintf() statements work with each other now: the new chunk of code in sh.sem.c prints the exit value for built-in commands whereas the chunk in sh.proc.c handles normal commands.

I modified each xprintf() slightly so I could tell the difference:
   1. sh.proc.c
   -       xprintf(CGETS(17, 2, "Exit %d\n"), reason);
   +       xprintf(CGETS(17, 2, "BAR %d\n"), reason);

   2. sh.sem.c 
   -                       xprintf(CGETS(17, 2, "Exit %d\n"), rv);
   +                       xprintf(CGETS(17, 2, "FOO %d\n"), rv);

NOTE: You have to unset the LANG environment variable or the CGETS() call will get the string to print from the i18n files.

Running with this patch
   $ ./tcsh -f
   > unsetenv LANG
   > set printexitvalue
   > /bin/false
   BAR 1
   > set i=`/bin/false`
   FOO 1
   > echo $i
   BAR 1
   > exit

Thus, the bug lies in sh.proc.c since $i should be an empty string.

It looks like the PBACKQ flag is not getting set in pp->p_flags; I made the xprintf() a little more verbose:

    if ((reason != 0) && (adrof(STRprintexitvalue)) &&
        (pp->p_flags & PBACKQ) == 0)
        xprintf("Exit %d, pp->p_flags & PBACKQ => 0x%x & 0x%x => 0x%x\n",
                reason, pp->p_flags, PBACKQ, (pp->p_flags & PBACKQ));


Running it:

$ ./tcsh -f
> unsetenv LANG
> set printexitvalue
> /bin/false
Exit 1, pp->p_flags & PBACKQ => 0x8 & 0x10000 => 0x0
> set i=`/bin/false`
Exit 1
> echo $i
Exit 1, pp->p_flags & PBACKQ => 0x8 & 0x10000 => 0x0
> exit

This is strange: according to gdb, the PBACKQ flag is set, but the xprintf shows that it's NOT set.

> unsetenv LANG
> set printexitvalue
> set i=`/bin/false`
Detaching after fork from child process 14239.

Breakpoint 1, pjwait (pp=0x1b54ffd0) at sh.proc.c:638
638         if ((reason != 0) && (adrof(STRprintexitvalue)) &&
(gdb) p pp->p_flags
$1 = 65544
(gdb) c
Continuing.
Exit 1
> echo $i
Exit 1, pp->p_flags & PBACKQ => 0x8 & 0x10000 => 0x0
> exit


65544 = 0x10008, which means the PBACKQ (0x10000) and PAEXITED (0x8) flags are set, thus it should not call the xprintf(), but it clearly does from the value of $i.

It must be the `/bin/false` child process that's calling xprintf().  So, maybe the PBACKQ flag is not set in the child?  Or maybe printexitvalue should be disabled in back-quoted child processes?

gdb was having problems following the child process, so I added a bunch of xprintf("DEBUG...") statements and verified that the PBACKQ flag is getting lost somewhere in the fork-and-exec calls.

I'm working on a patch to make sure the PBACKQ follows the child process.

Created attachment 366728 [details]
patch to fix backquoted output

Attached is a patch to set and preserve the F_BACKQ flag (and, in turn, the PBACKQ flag) for backquoted commands.  This fixes the bug in my testing.

I sent the patch upstream at
  http://mx.gw.com/pipermail/tcsh-bugs/2009-October/000648.html

Event posted on 2009-11-02 10:42 CET by rdassen

The test packages that Jeff Bastian provided for customer testing (based on
the patch from comment #9) have been successfully tested by the customer:
they confirm the test packages fix the issue and report no new problems.


This event sent from IssueTracker by rdassen 
 issue 358655