NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

bin/41764: ksh has suffered serious regressions in the past few years



>Number:         41764
>Category:       bin
>Synopsis:       ksh has suffered serious regressions in the past few years
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jul 22 04:05:00 +0000 2009
>Originator:     Greg A. Woods
>Release:        HEAD
>Organization:
Planix, Inc.; Toronto, Ontario; Canada
>Environment:
System: NetBSD 
>Description:

        It seems NetBSD's version of ksh has suffered some serious
        regressions over the past few years.

        My first hint was quite some time ago when I noticed "trap ERR"
        had stopped working (in the end that was due to a buggered and
        obviously untested change to the script that creates the signal
        list initializer).

        Upon running the original PDKSH regression tests I found there
        were some further gratuitious changes (due to defaulting
        POSIXLY_CORRECT, something that appears to have been done
        without any proper discussion, as with another critical
        incompatability change mentioned in a previous PR).

        Finally in trying to sort out some other weirdness I found that
        a massive and poorly documented change committed 2004/07/07 by
        mycroft seems to have obliterated a number of previous commits.

>How-To-Repeat:

        run the pdksh regression test suite against NetBSD ksh and wimper

>Fix:

        I think the following changes pretty much put things back the
        way they "should" be, and fixes a few other minor annoyances.

        At least all the regression tests pass now:

                Total failed: 3 (as expected)
                Total passed: 194

        (that's with a couple of new checks added already)

        The original pdksh regression tests should probably be added to
        NetBSD too, but currently the driver script is written in Perl
        (for no really good reason at all).  Someone with a spare
        Round-Tuit should tackle rewriting it in Awk, etc. so that it
        can be run in the base OS.

cvs diff: Diffing bin/ksh
Index: bin/ksh/config.h
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/config.h,v
retrieving revision 1.8
diff -u -r1.8 config.h
--- bin/ksh/config.h    19 Aug 2004 23:00:22 -0000      1.8
+++ bin/ksh/config.h    22 Jul 2009 01:03:49 -0000
@@ -158,7 +158,7 @@
 #define HAVE_MEMMOVE 1
 
 /* Define if you have a bcopy() function in your C library */
-#define HAVE_BCOPY
+/* #undef HAVE_BCOPY */
 
 /* Define if you have a lstat() function in your C library */
 #define HAVE_LSTAT 1
@@ -172,9 +172,6 @@
 /* Define if opendir() will open non-directory files */
 /* #undef OPENDIR_DOES_NONDIR */
 
-/* Define if you have a dup2() function in your C library */
-#define        HAVE_DUP2 1
-
 /* Define if the pgrp of setpgrp() can't be the pid of a zombie process */
 /* #undef NEED_PGRP_SYNC */
 
@@ -226,10 +223,10 @@
 /* #undef COMPLEX_HISTORY */
 
 /* Strict POSIX behaviour? */
-#define POSIXLY_CORRECT 1
+/* #undef POSIXLY_CORRECT */
 
 /* Specify default $ENV? */
-#define DEFAULT_ENV    "$HOME/.kshrc"
+/* #undef DEFAULT_ENV */
 
 /* Include shl(1) support? */
 /* #undef SWTCH */
@@ -249,6 +246,9 @@
 /* Define if you have the confstr function.  */
 #define HAVE_CONFSTR 1
 
+/* Define if you have the dup2 function.  */
+#define HAVE_DUP2 1
+
 /* Define if you have the flock function.  */
 #define HAVE_FLOCK 1
 
Index: bin/ksh/edit.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/edit.c,v
retrieving revision 1.22
diff -u -r1.22 edit.c
--- bin/ksh/edit.c      25 Apr 2009 05:11:37 -0000      1.22
+++ bin/ksh/edit.c      22 Jul 2009 01:58:48 -0000
@@ -40,6 +40,7 @@
                                     char ***wordsp));
 static int     x_locate_word ARGS((const char *buf, int buflen, int pos,
                                    int *startp, int *is_command));
+static int     path_order_cmp ARGS((const void *, const void *));
 
 static char vdisable_c;
 
@@ -113,7 +114,7 @@
                        if (ws.ws_col) {
                                x_cols = ws.ws_col < MIN_COLS ? MIN_COLS
                                                : ws.ws_col;
-                               
+
                                if ((vp = typeset("COLUMNS", 0, 0, 0, 0)))
                                        setint(vp, (long) ws.ws_col);
                        }
@@ -216,7 +217,7 @@
        if (onoff) {
                TTY_state       cb;
                X_chars         oldchars;
-               
+
                oldchars = edchars;
                cb = tty_state;
 
@@ -602,8 +603,30 @@
        XPput(w, NULL);
        words = (char **) XPclose(w);
 
-       for (nwords = 0; words[nwords]; nwords++)
-               ;
+       /* Mon Nov 15 20:39:41 CET 1999 Marcin Dalecki <dalecki@xxxxxxxxx>:
+        *
+        * Count the matches and use this pass to quote out matches containing
+        * white spaces or actual input field separators.
+        */
+       for (nwords = 0; words[nwords]; nwords++) {
+               char *tmp;
+
+               for (tmp = words[nwords]; *tmp; ++tmp) {
+                       if (ctype(*tmp, C_IFSWS))
+                               break;
+               }
+               if (*tmp) {
+                       int len = strlen(words[nwords]);
+
+                       tmp = alloc(len + 3, ATEMP);
+                       tmp[0] = '"';
+                       memcpy(tmp + 1, words[nwords], len);
+                       tmp[len + 1] = '"';
+                       tmp[len + 2] = '\0';
+                       afree(words[nwords], ATEMP);
+                       words[nwords] = tmp;
+               }
+       }
        if (nwords == 1) {
                struct stat statb;
 
@@ -641,8 +664,6 @@
        int path_order;
 };
 
-static int path_order_cmp(const void *aa, const void *bb);
-
 /* Compare routine used in x_command_glob() */
 static int
 path_order_cmp(aa, bb)
@@ -667,7 +688,7 @@
        char *toglob;
        char *pat;
        char *fpath;
-       int nwords;
+       int nwords, x, idx, escaping;
        XPtrV w;
        struct block *l;
 
@@ -676,6 +697,21 @@
 
        toglob = add_glob(str, slen);
 
+       /* remove all escaping backward slashes */
+       escaping = 0;
+       for (x = 0, idx = 0; toglob[x]; x++) {
+               if (toglob[x] == '\\' && !escaping) {
+                       escaping = 1;
+                       continue;
+               }
+
+               toglob[idx] = toglob[x];
+               idx++;
+               if (escaping)
+                       escaping = 0;
+       }
+       toglob[idx] = '\0';
+
        /* Convert "foo*" (toglob) to a pattern for future use */
        pat = evalstr(toglob, DOPAT|DOTILDE);
        afree(toglob, ATEMP);
@@ -791,7 +827,7 @@
                int iscmd;
 
                /* Figure out if this is a command */
-               for (p = start - 1; p >= 0 && isspace((unsigned char)buf[p]); 
p--)
+               for (p = start - 1; p >= 0 && isspace((unsigned char) buf[p]); 
p--)
                        ;
                iscmd = p < 0 || strchr(";|&()`", buf[p]);
                if (iscmd) {
@@ -853,6 +889,40 @@
        return nwords;
 }
 
+
+static const char *
+find_match(const char *s, int have)
+{
+       int     want = 0;
+       int     nest = 1;
+       int     c;
+
+       switch (have) {
+       case '(':       want = ')'; break;
+       case '{':       want = '}'; break;
+       case '[':       want = ']'; break;
+       case '\'':
+       case '"':       want = have; break;
+       }
+       if (want == 0 || s == NULL)
+               return NULL;
+
+       while (nest > 0 && (c = *s)) {
+               if (c == '\\') {
+                       s++;
+                       s++;
+                       continue;
+               }
+               if (c == want)
+                       nest--;
+               else if (c == have)
+                       nest++;
+               if (nest > 0)
+                       s++;
+       }
+       return (nest == 0) ? s : NULL;
+}
+
 /* Given a string, copy it and possibly add a '*' to the end.  The
  * new string is returned.
  */
@@ -862,7 +932,7 @@
        int slen;
 {
        char *toglob;
-       char *s;
+       const char *s;
        bool_t saw_slash = FALSE;
 
        if (slen < 0)
@@ -880,11 +950,24 @@
        for (s = toglob; *s; s++) {
                if (*s == '\\' && s[1])
                        s++;
-               else if (*s == '*' || *s == '?' || *s == '$'
+               else if (*s == '*' || *s == '[' || *s == '?'
                         || (s[1] == '(' /*)*/ && strchr("*+?@!", *s)))
                        break;
                else if (ISDIRSEP(*s))
                        saw_slash = TRUE;
+               else if (*s == '$') {
+                       if (*++s == '{') {
+                               const char *cp;
+                               
+                               if ((cp = find_match(&s[1], '{')))
+                                       s = ++cp;
+                       }
+                       if (*s)
+                               s += strcspn(s,
+                                       ".,/?-<>[]{}()'\";:\\|=+*&^%$#@!`~");
+                       if (!*s)
+                               return toglob;
+               }
        }
        if (!*s && (*toglob != '~' || saw_slash)) {
                toglob[slen] = '*';
Index: bin/ksh/emacs.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/emacs.c,v
retrieving revision 1.32
diff -u -r1.32 emacs.c
--- bin/ksh/emacs.c     25 Apr 2009 05:11:37 -0000      1.32
+++ bin/ksh/emacs.c     22 Jul 2009 00:24:14 -0000
@@ -861,7 +861,12 @@
 x_end_of_text(c)
        int c;
 {
-       return KEOL;
+       x_zotc(edchars.eof);            /* XXX from OpenBSD */
+       x_putc('\r');
+       x_putc('\n');
+       x_flush();
+
+       return KEOL;
 }
 
 static int x_beg_hist(c) int c; { x_load_hist(histlist); return KSTD;}
Index: bin/ksh/eval.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/eval.c,v
retrieving revision 1.11
diff -u -r1.11 eval.c
--- bin/ksh/eval.c      25 Apr 2009 05:11:37 -0000      1.11
+++ bin/ksh/eval.c      22 Jul 2009 02:12:54 -0000
@@ -604,22 +604,7 @@
                        /* mark any special second pass chars */
                        if (!quote)
                                switch (c) {
-                                 case '[':
-                                       {
-                                               const char *p = sp;
-                                               bool_t special = FALSE;
-                                               while (*p != EOS) {
-                                                       if (p[0] == CHAR &&
-                                                               p[1] == ']') {
-                                                               special = TRUE;
-                                                               break;
-                                                       }
-                                                               
-                                                       p += 2;
-                                               }
-                                               if (!special)
-                                                       break;
-                                       }
+                                 case '[': /* XXX rev 1.9 was WRONG!!! that 
cannot be done here! */
                                  case NOT:
                                  case '-':
                                  case ']':
Index: bin/ksh/exec.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/exec.c,v
retrieving revision 1.13
diff -u -r1.13 exec.c
--- bin/ksh/exec.c      24 Apr 2006 19:58:20 -0000      1.13
+++ bin/ksh/exec.c      22 Jul 2009 00:26:57 -0000
@@ -83,6 +83,7 @@
 {
        int i;
        volatile int rv = 0;
+       volatile int rv_prop = 0;       /* is rv being propogated, or is it 
newly generated? */
        int pv[2];
        char ** volatile ap;
        char *s, *cp;
@@ -164,6 +165,7 @@
 
          case TPAREN:
                rv = execute(t->left, flags|XFORK);
+               rv_prop = 1;
                break;
 
          case TPIPE:
@@ -284,6 +286,7 @@
                        rv = execute(t->right, flags & XERROK);
                else
                        flags |= XERROK;
+               rv_prop = 1;
                break;
 
          case TBANG:
@@ -332,6 +335,7 @@
                        }
                }
                rv = 0; /* in case of a continue */
+               rv_prop = 1;
                if (t->type == TFOR) {
                        while (*ap != NULL) {
                                setstr(global(t->str), *ap++, KSH_UNWIND_ERROR);
@@ -343,6 +347,7 @@
                        for (;;) {
                                if (!(cp = do_selectargs(ap, is_first))) {
                                        rv = 1;
+                                       rv_prop = 0;
                                        break;
                                }
                                is_first = FALSE;
@@ -374,6 +379,7 @@
                rv = 0; /* in case of a continue */
                while ((execute(t->left, XERROK) == 0) == (t->type == TWHILE))
                        rv = execute(t->right, flags & XERROK);
+               rv_prop = 1;
                break;
 
          case TIF:
@@ -383,6 +389,7 @@
                rv = execute(t->left, XERROK) == 0 ?
                        execute(t->right->left, flags & XERROK) :
                        execute(t->right->right, flags & XERROK);
+               rv_prop = 1;
                break;
 
          case TCASE:
@@ -395,10 +402,12 @@
                break;
          Found:
                rv = execute(t->left, flags & XERROK);
+               rv_prop = 1;
                break;
 
          case TBRACE:
                rv = execute(t->left, flags & XERROK);
+               rv_prop = 1;
                break;
 
          case TFUNCT:
@@ -410,6 +419,7 @@
                 * (allows "ls -l | time grep foo").
                 */
                rv = timex(t, flags & ~XEXEC);
+               rv_prop = 1;
                break;
 
          case TEXEC:           /* an eval'd TCOM */
@@ -437,10 +447,11 @@
        quitenv();              /* restores IO */
        if ((flags&XEXEC))
                unwind(LEXIT);  /* exit child */
-       if (rv != 0 && !(flags & XERROK)) {
+       if (rv != 0 && !(flags & XERROK))
+               trapsig(SIGERR_);
+       if (rv != 0 && !rv_prop && !(flags & XERROK)) {
                if (Flag(FERREXIT))
                        unwind(LERROR);
-               trapsig(SIGERR_);
        }
        return rv;
 }
Index: bin/ksh/main.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/main.c,v
retrieving revision 1.14
diff -u -r1.14 main.c
--- bin/ksh/main.c      24 Jun 2007 18:00:49 -0000      1.14
+++ bin/ksh/main.c      22 Jul 2009 00:30:27 -0000
@@ -449,8 +449,8 @@
                Flag(FTRACKALL) = 1;    /* set after ENV */
 
        setlocale(LC_CTYPE, "");
-       shell(s, TRUE); /* doesn't return */
-       return 0;
+       exit(shell(s, TRUE));
+       /* NOTREACHED */
 }
 
 int
@@ -658,8 +658,9 @@
        int i;
 {
        /* ordering for EXIT vs ERR is a bit odd (this is what at&t ksh does) */
-       if (i == LEXIT || (Flag(FERREXIT) && (i == LERROR || i == LINTR)
-                          && sigtraps[SIGEXIT_].trap))
+       if (i == LEXIT || (Flag(FERREXIT) &&
+                          (i == LERROR || i == LINTR) &&
+                          sigtraps[SIGEXIT_].trap))
        {
                runtrap(&sigtraps[SIGEXIT_]);
                i = LLEAVE;
Index: bin/ksh/sh.h
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/sh.h,v
retrieving revision 1.7
diff -u -r1.7 sh.h
--- bin/ksh/sh.h        26 Jun 2005 19:09:00 -0000      1.7
+++ bin/ksh/sh.h        22 Jul 2009 00:31:17 -0000
@@ -555,6 +555,7 @@
        char   *trap;           /* trap command */
        int     volatile set;   /* trap pending */
        int     flags;          /* TF_* */
+       pid_t   setters_pid;    /* PID of the process that set the trap */
        handler_t cursig;       /* current handler (valid if TF_ORIG_* set) */
        handler_t shtrap;       /* shell signal handler */
 } Trap;
Index: bin/ksh/siglist.in
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/siglist.in,v
retrieving revision 1.2
diff -u -r1.2 siglist.in
--- bin/ksh/siglist.in  12 Jan 1997 19:12:17 -0000      1.2
+++ bin/ksh/siglist.in  3 Jul 2007 23:58:52 -0000
@@ -21,12 +21,12 @@
 # before BUS (linux doesn't really have a BUS, but defines it to UNUSED)
     UNUSED Unused
     BUS    Bus error
-    SEGV   Memory fault
+    SEGV   Memory access violation
     SYS    Bad system call
     PIPE   Broken pipe
     ALRM   Alarm clock
     TERM   Terminated
-    STKFLT Stack fault
+    STKFLT Stack overflow
     IO     I/O possible
     XCPU   CPU time limit exceeded
     XFSZ   File size limit exceeded
Index: bin/ksh/siglist.sh
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/siglist.sh,v
retrieving revision 1.8
diff -u -r1.8 siglist.sh
--- bin/ksh/siglist.sh  25 Oct 2008 22:18:15 -0000      1.8
+++ bin/ksh/siglist.sh  21 Jul 2009 22:54:09 -0000
@@ -21,7 +21,7 @@
 # The trap here to make up for a bug in bash (1.14.3(1)) that calls the trap
 (trap $trapsigs;
  echo '#include "sh.h"';
- echo '        { QwErTy SIGNALS , "DUMMY" , "hook for number of signals" },';
+ echo '        { QwErTy .signal = SIGNALS , .name = "DUMMY", .mess = "hook for 
number of signals" },';
  ${SED} -e '/^[         ]*#/d' -e 's/^[         ]*\([^         ][^     ]*\)[   
 ][      ]*\(.*[^       ]\)[    ]*$/#ifdef SIG\1\
        { QwErTy .signal = SIG\1 , .name = "\1", .mess = "\2" },\
 #endif/') > $in
Index: bin/ksh/trap.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/bin/ksh/trap.c,v
retrieving revision 1.8
diff -u -r1.8 trap.c
--- bin/ksh/trap.c      16 Oct 2006 00:07:32 -0000      1.8
+++ bin/ksh/trap.c      22 Jul 2009 00:32:13 -0000
@@ -257,6 +257,9 @@
        if (trapstr[0] == '\0') /* SIG_IGN */
                return;
        if (i == SIGEXIT_ || i == SIGERR_) {    /* avoid recursion on these */
+               /* Only execute trap in process that requested the trap. */
+               if (procpid != p->setters_pid)
+                       return;
                old_changed = p->flags & TF_CHANGED;
                p->flags &= ~TF_CHANGED;
                p->trap = (char *) 0;
@@ -324,6 +327,7 @@
                p->trap = NULL;
                f = SIG_DFL;
        }
+       p->setters_pid = procpid;       /* recorded for use with EXIT and ERR */
        if ((p->flags & (TF_DFL_INTR|TF_FATAL)) && f == SIG_DFL)
                f = trapsig;
        else if (p->flags & TF_SHELL_USES) {



Home | Main Index | Thread Index | Old Index