diff mbox series

Revert 78a00a7 and 3cd5386, they are not ready yet.

Message ID CAEtFKstthNzvFreHuPe38MoV9ZpnWau+ctNv4jMu4NM0Odmnxg@mail.gmail.com (mailing list archive)
State Rejected
Delegated to: Herbert Xu
Headers show
Series Revert 78a00a7 and 3cd5386, they are not ready yet. | expand

Commit Message

Devin Hussey Nov. 15, 2018, 2:03 p.m. UTC
From 65f9c258658b50aeb67947f5b54e926538560488 Mon Sep 17 00:00:00 2001
From: Devin Hussey <husseydevin@gmail.com>
Date: Thu, 15 Nov 2018 08:29:33 -0500
Subject: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.

They are not ready, as simply running dash on its own configure
script will reveal (note: config.cache does not exist):

    checking for a BSD-compatible install... (cached)
    checking whether build environment is sane... yes
    configure: 1: eval: --is-lightweight: not found
    configure: WARNING: 'missing' script is too old or missing
    checking for a thread-safe mkdir -p... (cached)  -p
    checking for gawk... (cached) no
    checking for mawk... (cached) no
    checking for nawk... (cached) no
    checking for awk... (cached) no
    checking whether make sets $(MAKE)... (cached) configure: 1: test:
=: unexpected operator
    no
    checking whether make supports nested variables... (cached)
    configure: 2878: test: =: unexpected operator
    checking for gcc... (cached) no
    checking for cc... (cached) no
    checking for cl.exe... (cached) no
    configure: error: in `/Users/Devin/dash2':
    configure: error: no acceptable C compiler found in $PATH
    See `config.log' for more details

3cd5386 completely broke expansion.

I got the configure script to run by changing

        return p - 1;

to

        return p;

in argstr, but variable expansion is still broken.

The most broken thing is ${FOO+BAR}:

    $ bash -c 'unset FOO; echo ${FOO+BAR}'

    $ ./dash -c 'unset FOO; echo ${FOO+BAR}'
    BAR
    $

We should really set up a test suite, as that isn't the first
time some obvious bugs have made their way into the code base.
I wonder if FreeBSD will let us use their tests, as they have
a very (too?) expansive set of tests.

At the very least, we should make sure dash can run its own
configure script, as that is the most common shell script
to run.

Signed-off-by: Devin Hussey <husseydevin@gmail.com>
---
 src/expand.c | 321 ++++++++++++++++++++++++++++-----------------------
 src/expand.h |   2 +-
 2 files changed, 176 insertions(+), 147 deletions(-)

--
2.19.1

Comments

Michael Greenberg Nov. 15, 2018, 4:53 p.m. UTC | #1
Seconded! A test suite for dash would have saved me a great deal of 
trouble. I have a number of tests I've written that I'd be happy to 
contribute.

Is dash ever tested on the POSIX test suite? Certification is expensive, 
but their test suite seems to be quite thorough... and it may even be 
possible to use their tests even if dash isn't going to be officially 
POSIX compliant.

Cheers,
Michael

On 2018-11-15 9:03 AM, Devin Hussey wrote:
>  From 65f9c258658b50aeb67947f5b54e926538560488 Mon Sep 17 00:00:00 2001
> From: Devin Hussey <husseydevin@gmail.com>
> Date: Thu, 15 Nov 2018 08:29:33 -0500
> Subject: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
>
> They are not ready, as simply running dash on its own configure
> script will reveal (note: config.cache does not exist):
>
>      checking for a BSD-compatible install... (cached)
>      checking whether build environment is sane... yes
>      configure: 1: eval: --is-lightweight: not found
>      configure: WARNING: 'missing' script is too old or missing
>      checking for a thread-safe mkdir -p... (cached)  -p
>      checking for gawk... (cached) no
>      checking for mawk... (cached) no
>      checking for nawk... (cached) no
>      checking for awk... (cached) no
>      checking whether make sets $(MAKE)... (cached) configure: 1: test:
> =: unexpected operator
>      no
>      checking whether make supports nested variables... (cached)
>      configure: 2878: test: =: unexpected operator
>      checking for gcc... (cached) no
>      checking for cc... (cached) no
>      checking for cl.exe... (cached) no
>      configure: error: in `/Users/Devin/dash2':
>      configure: error: no acceptable C compiler found in $PATH
>      See `config.log' for more details
>
> 3cd5386 completely broke expansion.
>
> I got the configure script to run by changing
>
>          return p - 1;
>
> to
>
>          return p;
>
> in argstr, but variable expansion is still broken.
>
> The most broken thing is ${FOO+BAR}:
>
>      $ bash -c 'unset FOO; echo ${FOO+BAR}'
>
>      $ ./dash -c 'unset FOO; echo ${FOO+BAR}'
>      BAR
>      $
>
> We should really set up a test suite, as that isn't the first
> time some obvious bugs have made their way into the code base.
> I wonder if FreeBSD will let us use their tests, as they have
> a very (too?) expansive set of tests.
>
> At the very least, we should make sure dash can run its own
> configure script, as that is the most common shell script
> to run.
>
> Signed-off-by: Devin Hussey <husseydevin@gmail.com>
> ---
>   src/expand.c | 321 ++++++++++++++++++++++++++++-----------------------
>   src/expand.h |   2 +-
>   2 files changed, 176 insertions(+), 147 deletions(-)
>
> diff --git a/src/expand.c b/src/expand.c
> index 14daa63..7a51766 100644
> --- a/src/expand.c
> +++ b/src/expand.c
> @@ -110,13 +110,13 @@ static struct ifsregion *ifslastp;
>   /* holds expanded arg list */
>   static struct arglist exparg;
>
> -static char *argstr(char *p, int flag);
> -static char *exptilde(char *startp, int flag);
> -static char *expari(char *start, int flag);
> +STATIC void argstr(char *, int);
> +STATIC char *exptilde(char *, char *, int);
>   STATIC void expbackq(union node *, int);
> +STATIC const char *subevalvar(char *, char *, int, int, int, int, int);
>   STATIC char *evalvar(char *, int);
>   static size_t strtodest(const char *p, int flags);
> -static size_t memtodest(const char *p, size_t len, int flags);
> +static void memtodest(const char *p, size_t len, int flags);
>   STATIC ssize_t varvalue(char *, int, int, int);
>   STATIC void expandmeta(struct strlist *, int);
>   #ifdef HAVE_GLOB
> @@ -133,7 +133,7 @@ STATIC int pmatch(const char *, const char *);
>   #else
>   #define pmatch(a, b) !fnmatch((a), (b), 0)
>   #endif
> -static size_t cvtnum(intmax_t num, int flags);
> +STATIC int cvtnum(intmax_t);
>   STATIC size_t esclen(const char *, const char *);
>   STATIC char *scanleft(char *, char *, char *, char *, int, int);
>   STATIC char *scanright(char *, char *, char *, char *, int, int);
> @@ -192,11 +192,13 @@ expandarg(union node *arg, struct arglist
> *arglist, int flag)
>       argbackq = arg->narg.backquote;
>       STARTSTACKSTR(expdest);
>       argstr(arg->narg.text, flag);
> +    p = _STPUTC('\0', expdest);
> +    expdest = p - 1;
>       if (arglist == NULL) {
>           /* here document expanded */
>           goto out;
>       }
> -    p = grabstackstr(expdest);
> +    p = grabstackstr(p);
>       exparg.lastp = &exparg.list;
>       /*
>        * TODO - EXP_REDIR
> @@ -230,7 +232,8 @@ out:
>    * $@ like $* since no splitting will be performed.
>    */
>
> -static char *argstr(char *p, int flag)
> +STATIC void
> +argstr(char *p, int flag)
>   {
>       static const char spclchars[] = {
>           '=',
> @@ -240,7 +243,6 @@ static char *argstr(char *p, int flag)
>           CTLESC,
>           CTLVAR,
>           CTLBACKQ,
> -        CTLARI,
>           CTLENDARI,
>           0
>       };
> @@ -251,41 +253,35 @@ static char *argstr(char *p, int flag)
>       size_t length;
>       int startloc;
>
> -    reject += !!(flag & EXP_VARTILDE2);
> -    reject += flag & EXP_VARTILDE ? 0 : 2;
> +    if (!(flag & EXP_VARTILDE)) {
> +        reject += 2;
> +    } else if (flag & EXP_VARTILDE2) {
> +        reject++;
> +    }
>       inquotes = 0;
>       length = 0;
>       if (flag & EXP_TILDE) {
> +        char *q;
> +
>           flag &= ~EXP_TILDE;
>   tilde:
> -        if (*p == '~')
> -            p = exptilde(p, flag);
> +        q = p;
> +        if (*q == '~')
> +            p = exptilde(p, q, flag);
>       }
>   start:
>       startloc = expdest - (char *)stackblock();
>       for (;;) {
> -        int end;
> -
>           length += strcspn(p + length, reject);
> -        end = 0;
>           c = (signed char)p[length];
> -        if (!(c & 0x80) || c == CTLENDARI || c == CTLENDVAR) {
> -            /*
> -             * c == '=' || c == ':' || c == '\0' ||
> -             * c == CTLENDARI || c == CTLENDVAR
> -             */
> +        if (c && (!(c & 0x80) || c == CTLENDARI)) {
> +            /* c == '=' || c == ':' || c == CTLENDARI */
>               length++;
> -            /* c == '\0' || c == CTLENDARI || c == CTLENDVAR */
> -            end = !!((c - 1) & 0x80);
>           }
> -        if (length > 0 && !(flag & EXP_DISCARD)) {
> +        if (length > 0) {
>               int newloc;
> -            char *q;
> -
> -            q = stnputs(p, length, expdest);
> -            q[-1] &= end - 1;
> -            expdest = q - (flag & EXP_WORD ? end : 0);
> -            newloc = expdest - (char *)stackblock() - end;
> +            expdest = stnputs(p, length, expdest);
> +            newloc = expdest - (char *)stackblock();
>               if (breakall && !inquotes && newloc > startloc) {
>                   recordregion(startloc, newloc, 0);
>               }
> @@ -294,11 +290,14 @@ start:
>           p += length + 1;
>           length = 0;
>
> -        if (end)
> -            break;
> -
>           switch (c) {
> +        case '\0':
> +            goto breakloop;
>           case '=':
> +            if (flag & EXP_VARTILDE2) {
> +                p--;
> +                continue;
> +            }
>               flag |= EXP_VARTILDE2;
>               reject++;
>               /* fall through */
> @@ -311,6 +310,11 @@ start:
>                   goto tilde;
>               }
>               continue;
> +        }
> +
> +        switch (c) {
> +        case CTLENDVAR: /* ??? */
> +            goto breakloop;
>           case CTLQUOTEMARK:
>               /* "$@" syntax adherence hack */
>               if (!inquotes && !memcmp(p, dolatstr + 1,
> @@ -335,23 +339,25 @@ addquote:
>               goto start;
>           case CTLBACKQ:
>               expbackq(argbackq->n, flag | inquotes);
> +            argbackq = argbackq->next;
>               goto start;
> -        case CTLARI:
> -            p = expari(p, flag | inquotes);
> +        case CTLENDARI:
> +            p--;
> +            expari(flag | inquotes);
>               goto start;
>           }
>       }
> -    return p - 1;
> +breakloop:
> +    ;
>   }
>
> -static char *exptilde(char *startp, int flag)
> +STATIC char *
> +exptilde(char *startp, char *p, int flag)
>   {
>       signed char c;
>       char *name;
>       const char *home;
> -    char *p;
>
> -    p = startp;
>       name = p + 1;
>
>       while ((c = *++p) != '\0') {
> @@ -370,21 +376,19 @@ static char *exptilde(char *startp, int flag)
>           }
>       }
>   done:
> -    if (flag & EXP_DISCARD)
> -        goto out;
>       *p = '\0';
>       if (*name == '\0') {
>           home = lookupvar(homestr);
>       } else {
>           home = getpwhome(name);
>       }
> -    *p = c;
>       if (!home)
>           goto lose;
> +    *p = c;
>       strtodest(home, flag | EXP_QUOTED);
> -out:
>       return (p);
>   lose:
> +    *p = c;
>       return (startp);
>   }
>
> @@ -433,43 +437,63 @@ removerecordregions(int endoff)
>    * Expand arithmetic expression.  Backup to start of expression,
>    * evaluate, place result in (backed up) result, adjust string position.
>    */
> -static char *expari(char *start, int flag)
> +void
> +expari(int flag)
>   {
>       struct stackmark sm;
> +    char *p, *start;
>       int begoff;
> -    int endoff;
>       int len;
>       intmax_t result;
> -    char *p;
>
> -    p = stackblock();
> -    begoff = expdest - p;
> -    p = argstr(start, flag & EXP_DISCARD);
> -
> -    if (flag & EXP_DISCARD)
> -        goto out;
> +    /*    ifsfree(); */
>
> +    /*
> +     * This routine is slightly over-complicated for
> +     * efficiency.  Next we scan backwards looking for the
> +     * start of arithmetic.
> +     */
>       start = stackblock();
> -    endoff = expdest - start;
> -    start += begoff;
> -    STADJUST(start - expdest, expdest);
> +    p = expdest;
> +    pushstackmark(&sm, p - start);
> +    *--p = '\0';
> +    p--;
> +    do {
> +        int esc;
> +
> +        while (*p != (char)CTLARI) {
> +            p--;
> +#ifdef DEBUG
> +            if (p < start) {
> +                sh_error("missing CTLARI (shouldn't happen)");
> +            }
> +#endif
> +        }
> +
> +        esc = esclen(start, p);
> +        if (!(esc % 2)) {
> +            break;
> +        }
> +
> +        p -= esc + 1;
> +    } while (1);
> +
> +    begoff = p - start;
>
>       removerecordregions(begoff);
>
> +    expdest = p;
> +
>       if (likely(flag & QUOTES_ESC))
> -        rmescapes(start);
> +        rmescapes(p + 1);
>
> -    pushstackmark(&sm, endoff);
> -    result = arith(start);
> +    result = arith(p + 1);
>       popstackmark(&sm);
>
> -    len = cvtnum(result, flag);
> +    len = cvtnum(result);
>
>       if (likely(!(flag & EXP_QUOTED)))
>           recordregion(begoff, begoff + len, 0);
> -
> -out:
> -    return p;
>   }
>
>
> @@ -488,9 +512,6 @@ expbackq(union node *cmd, int flag)
>       int startloc;
>       struct stackmark smark;
>
> -    if (flag & EXP_DISCARD)
> -        goto out;
> -
>       INTOFF;
>       startloc = expdest - (char *)stackblock();
>       pushstackmark(&smark, startloc);
> @@ -535,9 +556,6 @@ read:
>           (dest - (char *)stackblock()) - startloc,
>           (dest - (char *)stackblock()) - startloc,
>           stackblock() + startloc));
> -
> -out:
> -    argbackq = argbackq->next;
>   }
>
>
> @@ -608,35 +626,33 @@ scanright(
>       return 0;
>   }
>
> -static char *subevalvar(char *start, char *str, int strloc, int startloc,
> -            int varflags, int flag)
> +STATIC const char *
> +subevalvar(char *p, char *str, int strloc, int subtype, int startloc,
> int varflags, int flag)
>   {
> -    int subtype = varflags & VSTYPE;
>       int quotes = flag & QUOTES_ESC;
>       char *startp;
>       char *loc;
> -    long amount;
> +    struct nodelist *saveargbackq = argbackq;
> +    int amount;
>       char *rmesc, *rmescend;
>       int zero;
>       char *(*scan)(char *, char *, char *, char *, int , int);
> -    char *p;
> -
> -    p = argstr(start, (flag & EXP_DISCARD) | EXP_TILDE |
> -              (str ?  0 : EXP_CASE));
> -    if (flag & EXP_DISCARD)
> -        return p;
>
> +    argstr(p, EXP_TILDE | (subtype != VSASSIGN && subtype != VSQUESTION ?
> +                   EXP_CASE : 0));
> +    STPUTC('\0', expdest);
> +    argbackq = saveargbackq;
>       startp = stackblock() + startloc;
>
>       switch (subtype) {
>       case VSASSIGN:
>           setvar(str, startp, 0);
> -
> -        loc = startp;
> -        goto out;
> +        amount = startp - expdest;
> +        STADJUST(amount, expdest);
> +        return startp;
>
>       case VSQUESTION:
> -        varunset(start, str, startp, varflags);
> +        varunset(p, str, startp, varflags);
>           /* NOTREACHED */
>       }
>
> @@ -671,17 +687,10 @@ static char *subevalvar(char *start, char *str,
> int strloc, int startloc,
>               loc = startp + (str - loc) - 1;
>           }
>           *loc = '\0';
> -    } else
> -        loc = str - 1;
> -
> -out:
> -    amount = loc - expdest;
> -    STADJUST(amount, expdest);
> -
> -    /* Remove any recorded regions beyond start of variable */
> -    removerecordregions(startloc);
> -
> -    return p;
> +        amount = loc - expdest;
> +        STADJUST(amount, expdest);
> +    }
> +    return loc;
>   }
>
>
> @@ -696,6 +705,7 @@ evalvar(char *p, int flag)
>       int varflags;
>       char *var;
>       int patloc;
> +    int c;
>       int startloc;
>       ssize_t varlen;
>       int quoted;
> @@ -703,6 +713,9 @@ evalvar(char *p, int flag)
>       varflags = *p++;
>       subtype = varflags & VSTYPE;
>
> +    if (!subtype)
> +        sh_error("Bad substitution");
> +
>       quoted = flag & EXP_QUOTED;
>       var = p;
>       startloc = expdest - (char *)stackblock();
> @@ -713,30 +726,32 @@ again:
>       if (varflags & VSNUL)
>           varlen--;
>
> -    switch (subtype) {
> -    case VSPLUS:
> +    if (subtype == VSPLUS) {
>           varlen = -1 - varlen;
> -        /* fall through */
> +        goto vsplus;
> +    }
>
> -    case 0:
> -    case VSMINUS:
> -        p = argstr(p, flag | EXP_TILDE | EXP_WORD);
> -        if (varlen < 0)
> -            return p;
> +    if (subtype == VSMINUS) {
> +vsplus:
> +        if (varlen < 0) {
> +            argstr(p, flag | EXP_TILDE | EXP_WORD);
> +            goto end;
> +        }
>           goto record;
> +    }
>
> -    case VSASSIGN:
> -    case VSQUESTION:
> +    if (subtype == VSASSIGN || subtype == VSQUESTION) {
>           if (varlen >= 0)
>               goto record;
>
> -        p = subevalvar(p, var, 0, startloc, varflags,
> -                   flag & ~QUOTES_ESC);
> -
> -        if (flag & EXP_DISCARD)
> -            return p;
> -
> +        subevalvar(p, var, 0, subtype, startloc, varflags,
> +               flag & ~QUOTES_ESC);
>           varflags &= ~VSNUL;
> +        /*
> +         * Remove any recorded regions beyond
> +         * start of variable
> +         */
> +        removerecordregions(startloc);
>           goto again;
>       }
>
> @@ -744,14 +759,20 @@ again:
>           varunset(p, var, 0, 0);
>
>       if (subtype == VSLENGTH) {
> -        if (flag & EXP_DISCARD)
> -            return p;
> -        cvtnum(varlen > 0 ? varlen : 0, flag);
> +        cvtnum(varlen > 0 ? varlen : 0);
>           goto record;
>       }
>
> -    if (subtype == VSNORMAL)
> -        goto record;
> +    if (subtype == VSNORMAL) {
> +record:
> +        if (quoted) {
> +            quoted = *var == '@' && shellparam.nparam;
> +            if (!quoted)
> +                goto end;
> +        }
> +        recordregion(startloc, expdest - (char *)stackblock(), quoted);
> +        goto end;
> +    }
>
>   #ifdef DEBUG
>       switch (subtype) {
> @@ -765,28 +786,45 @@ again:
>       }
>   #endif
>
> -    flag |= varlen < 0 ? EXP_DISCARD : 0;
> -    if (!(flag & EXP_DISCARD)) {
> +    if (varlen >= 0) {
>           /*
>            * Terminate the string and start recording the pattern
>            * right after it
>            */
>           STPUTC('\0', expdest);
> +        patloc = expdest - (char *)stackblock();
> +        if (subevalvar(p, NULL, patloc, subtype,
> +                   startloc, varflags, flag) == 0) {
> +            int amount = expdest - (
> +                (char *)stackblock() + patloc - 1
> +            );
> +            STADJUST(-amount, expdest);
> +        }
> +        /* Remove any recorded regions beyond start of variable */
> +        removerecordregions(startloc);
> +        goto record;
>       }
>
> -    patloc = expdest - (char *)stackblock();
> -    p = subevalvar(p, NULL, patloc, startloc, varflags, flag);
> -
> -record:
> -    if (flag & EXP_DISCARD)
> -        return p;
> +    varlen = 0;
>
> -    if (quoted) {
> -        quoted = *var == '@' && shellparam.nparam;
> -        if (!quoted)
> -            return p;
> +end:
> +    if (subtype != VSNORMAL) {    /* skip to end of alternative */
> +        int nesting = 1;
> +        for (;;) {
> +            if ((c = (signed char)*p++) == CTLESC)
> +                p++;
> +            else if (c == CTLBACKQ) {
> +                if (varlen >= 0)
> +                    argbackq = argbackq->next;
> +            } else if (c == CTLVAR) {
> +                if ((*p++ & VSTYPE) != VSNORMAL)
> +                    nesting++;
> +            } else if (c == CTLENDVAR) {
> +                if (--nesting == 0)
> +                    break;
> +            }
> +        }
>       }
> -    recordregion(startloc, expdest - (char *)stackblock(), quoted);
>       return p;
>   }
>
> @@ -795,17 +833,15 @@ record:
>    * Put a string on the stack.
>    */
>
> -static size_t memtodest(const char *p, size_t len, int flags)
> +static void memtodest(const char *p, size_t len, int flags)
>   {
>       const char *syntax = flags & EXP_QUOTED ? DQSYNTAX : BASESYNTAX;
>       char *q;
> -    char *s;
>
>       if (unlikely(!len))
> -        return 0;
> +        return;
>
>       q = makestrspace(len * 2, expdest);
> -    s = q;
>
>       do {
>           int c = (signed char)*p++;
> @@ -820,7 +856,6 @@ static size_t memtodest(const char *p, size_t len,
> int flags)
>       } while (--len);
>
>       expdest = q;
> -    return q - s;
>   }
>
>
> @@ -847,18 +882,10 @@ varvalue(char *name, int varflags, int flags, int quoted)
>       char sepc;
>       char **ap;
>       int subtype = varflags & VSTYPE;
> -    int discard = (subtype == VSPLUS || subtype == VSLENGTH) |
> -              (flags & EXP_DISCARD);
> +    int discard = subtype == VSPLUS || subtype == VSLENGTH;
>       ssize_t len = 0;
>       char c;
>
> -    if (!subtype) {
> -        if (discard)
> -            return -1;
> -
> -        sh_error("Bad substitution");
> -    }
> -
>       flags |= EXP_KEEPNUL;
>       flags &= discard ? ~QUOTES_ESC : ~0;
>       sep = (flags & EXP_FULL) << CHAR_BIT;
> @@ -878,7 +905,7 @@ varvalue(char *name, int varflags, int flags, int quoted)
>           if (num == 0)
>               return -1;
>   numvar:
> -        len = cvtnum(num, flags);
> +        len = cvtnum(num);
>           break;
>       case '-':
>           p = makestrspace(NOPTS, expdest);
> @@ -952,7 +979,6 @@ value:
>
>       if (discard)
>           STADJUST(-len, expdest);
> -
>       return len;
>   }
>
> @@ -1704,6 +1730,7 @@ casematch(union node *pattern, char *val)
>       argbackq = pattern->narg.backquote;
>       STARTSTACKSTR(expdest);
>       argstr(pattern->narg.text, EXP_TILDE | EXP_CASE);
> +    STACKSTRNUL(expdest);
>       ifsfree();
>       result = patmatch(stackblock(), val);
>       popstackmark(&smark);
> @@ -1714,13 +1741,15 @@ casematch(union node *pattern, char *val)
>    * Our own itoa().
>    */
>
> -static size_t cvtnum(intmax_t num, int flags)
> +STATIC int
> +cvtnum(intmax_t num)
>   {
>       int len = max_int_length(sizeof(num));
> -    char buf[len];
>
> -    len = fmtstr(buf, len, "%" PRIdMAX, num);
> -    return memtodest(buf, len, flags);
> +    expdest = makestrspace(len, expdest);
> +    len = fmtstr(expdest, len, "%" PRIdMAX, num);
> +    STADJUST(len, expdest);
> +    return len;
>   }
>
>   STATIC void
> diff --git a/src/expand.h b/src/expand.h
> index c44b848..617b851 100644
> --- a/src/expand.h
> +++ b/src/expand.h
> @@ -59,11 +59,11 @@ struct arglist {
>   #define EXP_WORD    0x80    /* expand word in parameter expansion */
>   #define EXP_QUOTED    0x100    /* expand word in double quotes */
>   #define EXP_KEEPNUL    0x200    /* do not skip NUL characters */
> -#define EXP_DISCARD    0x400    /* discard result of expansion */
>
>
>   union node;
>   void expandarg(union node *, struct arglist *, int);
> +void expari(int);
>   #define rmescapes(p) _rmescapes((p), 0)
>   char *_rmescapes(char *, int);
>   int casematch(union node *, char *);
> --
> 2.19.1
Herbert Xu Nov. 19, 2018, 6:26 a.m. UTC | #2
Devin Hussey <husseydevin@gmail.com> wrote:
> From 65f9c258658b50aeb67947f5b54e926538560488 Mon Sep 17 00:00:00 2001
> From: Devin Hussey <husseydevin@gmail.com>
> Date: Thu, 15 Nov 2018 08:29:33 -0500
> Subject: [PATCH] Revert 78a00a7 and 3cd5386, they are not ready yet.
> 
> They are not ready, as simply running dash on its own configure
> script will reveal (note: config.cache does not exist):
> 
>    checking for a BSD-compatible install... (cached)
>    checking whether build environment is sane... yes
>    configure: 1: eval: --is-lightweight: not found
>    configure: WARNING: 'missing' script is too old or missing
>    checking for a thread-safe mkdir -p... (cached)  -p
>    checking for gawk... (cached) no
>    checking for mawk... (cached) no
>    checking for nawk... (cached) no
>    checking for awk... (cached) no
>    checking whether make sets $(MAKE)... (cached) configure: 1: test:
> =: unexpected operator
>    no
>    checking whether make supports nested variables... (cached)
>    configure: 2878: test: =: unexpected operator
>    checking for gcc... (cached) no
>    checking for cc... (cached) no
>    checking for cl.exe... (cached) no
>    configure: error: in `/Users/Devin/dash2':
>    configure: error: no acceptable C compiler found in $PATH
>    See `config.log' for more details
> 
> 3cd5386 completely broke expansion.
> 
> I got the configure script to run by changing
> 
>        return p - 1;
> 
> to
> 
>        return p;
> 
> in argstr, but variable expansion is still broken.
> 
> The most broken thing is ${FOO+BAR}:
> 
>    $ bash -c 'unset FOO; echo ${FOO+BAR}'
> 
>    $ ./dash -c 'unset FOO; echo ${FOO+BAR}'
>    BAR
>    $

Please look at the pending patches first.  These issues have
been fixed months ago:

https://patchwork.kernel.org/patch/10596841/

Unfortunately I haven't the time to push out the fix to the
git tree.  I'll do to that now.

Thanks,
diff mbox series

Patch

diff --git a/src/expand.c b/src/expand.c
index 14daa63..7a51766 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -110,13 +110,13 @@  static struct ifsregion *ifslastp;
 /* holds expanded arg list */
 static struct arglist exparg;

-static char *argstr(char *p, int flag);
-static char *exptilde(char *startp, int flag);
-static char *expari(char *start, int flag);
+STATIC void argstr(char *, int);
+STATIC char *exptilde(char *, char *, int);
 STATIC void expbackq(union node *, int);
+STATIC const char *subevalvar(char *, char *, int, int, int, int, int);
 STATIC char *evalvar(char *, int);
 static size_t strtodest(const char *p, int flags);
-static size_t memtodest(const char *p, size_t len, int flags);
+static void memtodest(const char *p, size_t len, int flags);
 STATIC ssize_t varvalue(char *, int, int, int);
 STATIC void expandmeta(struct strlist *, int);
 #ifdef HAVE_GLOB
@@ -133,7 +133,7 @@  STATIC int pmatch(const char *, const char *);
 #else
 #define pmatch(a, b) !fnmatch((a), (b), 0)
 #endif
-static size_t cvtnum(intmax_t num, int flags);
+STATIC int cvtnum(intmax_t);
 STATIC size_t esclen(const char *, const char *);
 STATIC char *scanleft(char *, char *, char *, char *, int, int);
 STATIC char *scanright(char *, char *, char *, char *, int, int);
@@ -192,11 +192,13 @@  expandarg(union node *arg, struct arglist
*arglist, int flag)
     argbackq = arg->narg.backquote;
     STARTSTACKSTR(expdest);
     argstr(arg->narg.text, flag);
+    p = _STPUTC('\0', expdest);
+    expdest = p - 1;
     if (arglist == NULL) {
         /* here document expanded */
         goto out;
     }
-    p = grabstackstr(expdest);
+    p = grabstackstr(p);
     exparg.lastp = &exparg.list;
     /*
      * TODO - EXP_REDIR
@@ -230,7 +232,8 @@  out:
  * $@ like $* since no splitting will be performed.
  */

-static char *argstr(char *p, int flag)
+STATIC void
+argstr(char *p, int flag)
 {
     static const char spclchars[] = {
         '=',
@@ -240,7 +243,6 @@  static char *argstr(char *p, int flag)
         CTLESC,
         CTLVAR,
         CTLBACKQ,
-        CTLARI,
         CTLENDARI,
         0
     };
@@ -251,41 +253,35 @@  static char *argstr(char *p, int flag)
     size_t length;
     int startloc;

-    reject += !!(flag & EXP_VARTILDE2);
-    reject += flag & EXP_VARTILDE ? 0 : 2;
+    if (!(flag & EXP_VARTILDE)) {
+        reject += 2;
+    } else if (flag & EXP_VARTILDE2) {
+        reject++;
+    }
     inquotes = 0;
     length = 0;
     if (flag & EXP_TILDE) {
+        char *q;
+
         flag &= ~EXP_TILDE;
 tilde:
-        if (*p == '~')
-            p = exptilde(p, flag);
+        q = p;
+        if (*q == '~')
+            p = exptilde(p, q, flag);
     }
 start:
     startloc = expdest - (char *)stackblock();
     for (;;) {
-        int end;
-
         length += strcspn(p + length, reject);
-        end = 0;
         c = (signed char)p[length];
-        if (!(c & 0x80) || c == CTLENDARI || c == CTLENDVAR) {
-            /*
-             * c == '=' || c == ':' || c == '\0' ||
-             * c == CTLENDARI || c == CTLENDVAR
-             */
+        if (c && (!(c & 0x80) || c == CTLENDARI)) {
+            /* c == '=' || c == ':' || c == CTLENDARI */
             length++;
-            /* c == '\0' || c == CTLENDARI || c == CTLENDVAR */
-            end = !!((c - 1) & 0x80);
         }
-        if (length > 0 && !(flag & EXP_DISCARD)) {
+        if (length > 0) {
             int newloc;
-            char *q;
-
-            q = stnputs(p, length, expdest);
-            q[-1] &= end - 1;
-            expdest = q - (flag & EXP_WORD ? end : 0);
-            newloc = expdest - (char *)stackblock() - end;
+            expdest = stnputs(p, length, expdest);
+            newloc = expdest - (char *)stackblock();
             if (breakall && !inquotes && newloc > startloc) {
                 recordregion(startloc, newloc, 0);
             }
@@ -294,11 +290,14 @@  start:
         p += length + 1;
         length = 0;

-        if (end)
-            break;
-
         switch (c) {
+        case '\0':
+            goto breakloop;
         case '=':
+            if (flag & EXP_VARTILDE2) {
+                p--;
+                continue;
+            }
             flag |= EXP_VARTILDE2;
             reject++;
             /* fall through */
@@ -311,6 +310,11 @@  start:
                 goto tilde;
             }
             continue;
+        }
+
+        switch (c) {
+        case CTLENDVAR: /* ??? */
+            goto breakloop;
         case CTLQUOTEMARK:
             /* "$@" syntax adherence hack */
             if (!inquotes && !memcmp(p, dolatstr + 1,
@@ -335,23 +339,25 @@  addquote:
             goto start;
         case CTLBACKQ:
             expbackq(argbackq->n, flag | inquotes);
+            argbackq = argbackq->next;
             goto start;
-        case CTLARI:
-            p = expari(p, flag | inquotes);
+        case CTLENDARI:
+            p--;
+            expari(flag | inquotes);
             goto start;
         }
     }
-    return p - 1;
+breakloop:
+    ;
 }

-static char *exptilde(char *startp, int flag)
+STATIC char *
+exptilde(char *startp, char *p, int flag)
 {
     signed char c;
     char *name;
     const char *home;
-    char *p;

-    p = startp;
     name = p + 1;

     while ((c = *++p) != '\0') {
@@ -370,21 +376,19 @@  static char *exptilde(char *startp, int flag)
         }
     }
 done:
-    if (flag & EXP_DISCARD)
-        goto out;
     *p = '\0';
     if (*name == '\0') {
         home = lookupvar(homestr);
     } else {
         home = getpwhome(name);
     }
-    *p = c;
     if (!home)
         goto lose;
+    *p = c;
     strtodest(home, flag | EXP_QUOTED);
-out:
     return (p);
 lose:
+    *p = c;
     return (startp);
 }

@@ -433,43 +437,63 @@  removerecordregions(int endoff)
  * Expand arithmetic expression.  Backup to start of expression,
  * evaluate, place result in (backed up) result, adjust string position.
  */
-static char *expari(char *start, int flag)
+void
+expari(int flag)
 {
     struct stackmark sm;
+    char *p, *start;
     int begoff;
-    int endoff;
     int len;
     intmax_t result;
-    char *p;

-    p = stackblock();
-    begoff = expdest - p;
-    p = argstr(start, flag & EXP_DISCARD);
-
-    if (flag & EXP_DISCARD)
-        goto out;
+    /*    ifsfree(); */

+    /*
+     * This routine is slightly over-complicated for
+     * efficiency.  Next we scan backwards looking for the
+     * start of arithmetic.
+     */
     start = stackblock();
-    endoff = expdest - start;
-    start += begoff;
-    STADJUST(start - expdest, expdest);
+    p = expdest;
+    pushstackmark(&sm, p - start);
+    *--p = '\0';
+    p--;
+    do {
+        int esc;
+
+        while (*p != (char)CTLARI) {
+            p--;
+#ifdef DEBUG
+            if (p < start) {
+                sh_error("missing CTLARI (shouldn't happen)");
+            }
+#endif
+        }
+
+        esc = esclen(start, p);
+        if (!(esc % 2)) {
+            break;
+        }
+
+        p -= esc + 1;
+    } while (1);
+
+    begoff = p - start;

     removerecordregions(begoff);

+    expdest = p;
+
     if (likely(flag & QUOTES_ESC))
-        rmescapes(start);
+        rmescapes(p + 1);

-    pushstackmark(&sm, endoff);
-    result = arith(start);
+    result = arith(p + 1);
     popstackmark(&sm);

-    len = cvtnum(result, flag);
+    len = cvtnum(result);

     if (likely(!(flag & EXP_QUOTED)))
         recordregion(begoff, begoff + len, 0);
-
-out:
-    return p;
 }


@@ -488,9 +512,6 @@  expbackq(union node *cmd, int flag)
     int startloc;
     struct stackmark smark;

-    if (flag & EXP_DISCARD)
-        goto out;
-
     INTOFF;
     startloc = expdest - (char *)stackblock();
     pushstackmark(&smark, startloc);
@@ -535,9 +556,6 @@  read:
         (dest - (char *)stackblock()) - startloc,
         (dest - (char *)stackblock()) - startloc,
         stackblock() + startloc));
-
-out:
-    argbackq = argbackq->next;
 }


@@ -608,35 +626,33 @@  scanright(
     return 0;
 }

-static char *subevalvar(char *start, char *str, int strloc, int startloc,
-            int varflags, int flag)
+STATIC const char *
+subevalvar(char *p, char *str, int strloc, int subtype, int startloc,
int varflags, int flag)
 {
-    int subtype = varflags & VSTYPE;
     int quotes = flag & QUOTES_ESC;
     char *startp;
     char *loc;
-    long amount;
+    struct nodelist *saveargbackq = argbackq;
+    int amount;
     char *rmesc, *rmescend;
     int zero;
     char *(*scan)(char *, char *, char *, char *, int , int);
-    char *p;
-
-    p = argstr(start, (flag & EXP_DISCARD) | EXP_TILDE |
-              (str ?  0 : EXP_CASE));
-    if (flag & EXP_DISCARD)
-        return p;

+    argstr(p, EXP_TILDE | (subtype != VSASSIGN && subtype != VSQUESTION ?
+                   EXP_CASE : 0));
+    STPUTC('\0', expdest);
+    argbackq = saveargbackq;
     startp = stackblock() + startloc;

     switch (subtype) {
     case VSASSIGN:
         setvar(str, startp, 0);
-
-        loc = startp;
-        goto out;
+        amount = startp - expdest;
+        STADJUST(amount, expdest);
+        return startp;

     case VSQUESTION:
-        varunset(start, str, startp, varflags);
+        varunset(p, str, startp, varflags);
         /* NOTREACHED */
     }

@@ -671,17 +687,10 @@  static char *subevalvar(char *start, char *str,
int strloc, int startloc,
             loc = startp + (str - loc) - 1;
         }
         *loc = '\0';
-    } else
-        loc = str - 1;
-
-out:
-    amount = loc - expdest;
-    STADJUST(amount, expdest);
-
-    /* Remove any recorded regions beyond start of variable */
-    removerecordregions(startloc);
-
-    return p;
+        amount = loc - expdest;
+        STADJUST(amount, expdest);
+    }
+    return loc;
 }


@@ -696,6 +705,7 @@  evalvar(char *p, int flag)
     int varflags;
     char *var;
     int patloc;
+    int c;
     int startloc;
     ssize_t varlen;
     int quoted;
@@ -703,6 +713,9 @@  evalvar(char *p, int flag)
     varflags = *p++;
     subtype = varflags & VSTYPE;

+    if (!subtype)
+        sh_error("Bad substitution");
+
     quoted = flag & EXP_QUOTED;
     var = p;
     startloc = expdest - (char *)stackblock();
@@ -713,30 +726,32 @@  again:
     if (varflags & VSNUL)
         varlen--;

-    switch (subtype) {
-    case VSPLUS:
+    if (subtype == VSPLUS) {
         varlen = -1 - varlen;
-        /* fall through */
+        goto vsplus;
+    }

-    case 0:
-    case VSMINUS:
-        p = argstr(p, flag | EXP_TILDE | EXP_WORD);
-        if (varlen < 0)
-            return p;
+    if (subtype == VSMINUS) {
+vsplus:
+        if (varlen < 0) {
+            argstr(p, flag | EXP_TILDE | EXP_WORD);
+            goto end;
+        }
         goto record;
+    }

-    case VSASSIGN:
-    case VSQUESTION:
+    if (subtype == VSASSIGN || subtype == VSQUESTION) {
         if (varlen >= 0)
             goto record;

-        p = subevalvar(p, var, 0, startloc, varflags,
-                   flag & ~QUOTES_ESC);
-
-        if (flag & EXP_DISCARD)
-            return p;
-
+        subevalvar(p, var, 0, subtype, startloc, varflags,
+               flag & ~QUOTES_ESC);
         varflags &= ~VSNUL;
+        /*
+         * Remove any recorded regions beyond
+         * start of variable
+         */
+        removerecordregions(startloc);
         goto again;
     }

@@ -744,14 +759,20 @@  again:
         varunset(p, var, 0, 0);

     if (subtype == VSLENGTH) {
-        if (flag & EXP_DISCARD)
-            return p;
-        cvtnum(varlen > 0 ? varlen : 0, flag);
+        cvtnum(varlen > 0 ? varlen : 0);
         goto record;
     }

-    if (subtype == VSNORMAL)
-        goto record;
+    if (subtype == VSNORMAL) {
+record:
+        if (quoted) {
+            quoted = *var == '@' && shellparam.nparam;
+            if (!quoted)
+                goto end;
+        }
+        recordregion(startloc, expdest - (char *)stackblock(), quoted);
+        goto end;
+    }

 #ifdef DEBUG
     switch (subtype) {
@@ -765,28 +786,45 @@  again:
     }
 #endif

-    flag |= varlen < 0 ? EXP_DISCARD : 0;
-    if (!(flag & EXP_DISCARD)) {
+    if (varlen >= 0) {
         /*
          * Terminate the string and start recording the pattern
          * right after it
          */
         STPUTC('\0', expdest);
+        patloc = expdest - (char *)stackblock();
+        if (subevalvar(p, NULL, patloc, subtype,
+                   startloc, varflags, flag) == 0) {
+            int amount = expdest - (
+                (char *)stackblock() + patloc - 1
+            );
+            STADJUST(-amount, expdest);
+        }
+        /* Remove any recorded regions beyond start of variable */
+        removerecordregions(startloc);
+        goto record;
     }

-    patloc = expdest - (char *)stackblock();
-    p = subevalvar(p, NULL, patloc, startloc, varflags, flag);
-
-record:
-    if (flag & EXP_DISCARD)
-        return p;
+    varlen = 0;

-    if (quoted) {
-        quoted = *var == '@' && shellparam.nparam;
-        if (!quoted)
-            return p;
+end:
+    if (subtype != VSNORMAL) {    /* skip to end of alternative */
+        int nesting = 1;
+        for (;;) {
+            if ((c = (signed char)*p++) == CTLESC)
+                p++;
+            else if (c == CTLBACKQ) {
+                if (varlen >= 0)
+                    argbackq = argbackq->next;
+            } else if (c == CTLVAR) {
+                if ((*p++ & VSTYPE) != VSNORMAL)
+                    nesting++;
+            } else if (c == CTLENDVAR) {
+                if (--nesting == 0)
+                    break;
+            }
+        }
     }
-    recordregion(startloc, expdest - (char *)stackblock(), quoted);
     return p;
 }

@@ -795,17 +833,15 @@  record:
  * Put a string on the stack.
  */

-static size_t memtodest(const char *p, size_t len, int flags)
+static void memtodest(const char *p, size_t len, int flags)
 {
     const char *syntax = flags & EXP_QUOTED ? DQSYNTAX : BASESYNTAX;
     char *q;
-    char *s;

     if (unlikely(!len))
-        return 0;
+        return;

     q = makestrspace(len * 2, expdest);
-    s = q;

     do {
         int c = (signed char)*p++;
@@ -820,7 +856,6 @@  static size_t memtodest(const char *p, size_t len,
int flags)
     } while (--len);

     expdest = q;
-    return q - s;
 }


@@ -847,18 +882,10 @@  varvalue(char *name, int varflags, int flags, int quoted)
     char sepc;
     char **ap;
     int subtype = varflags & VSTYPE;
-    int discard = (subtype == VSPLUS || subtype == VSLENGTH) |
-              (flags & EXP_DISCARD);
+    int discard = subtype == VSPLUS || subtype == VSLENGTH;
     ssize_t len = 0;
     char c;

-    if (!subtype) {
-        if (discard)
-            return -1;
-
-        sh_error("Bad substitution");
-    }
-
     flags |= EXP_KEEPNUL;
     flags &= discard ? ~QUOTES_ESC : ~0;
     sep = (flags & EXP_FULL) << CHAR_BIT;
@@ -878,7 +905,7 @@  varvalue(char *name, int varflags, int flags, int quoted)
         if (num == 0)
             return -1;
 numvar:
-        len = cvtnum(num, flags);
+        len = cvtnum(num);
         break;
     case '-':
         p = makestrspace(NOPTS, expdest);
@@ -952,7 +979,6 @@  value:

     if (discard)
         STADJUST(-len, expdest);
-
     return len;
 }

@@ -1704,6 +1730,7 @@  casematch(union node *pattern, char *val)
     argbackq = pattern->narg.backquote;
     STARTSTACKSTR(expdest);
     argstr(pattern->narg.text, EXP_TILDE | EXP_CASE);
+    STACKSTRNUL(expdest);
     ifsfree();
     result = patmatch(stackblock(), val);
     popstackmark(&smark);
@@ -1714,13 +1741,15 @@  casematch(union node *pattern, char *val)
  * Our own itoa().
  */

-static size_t cvtnum(intmax_t num, int flags)
+STATIC int
+cvtnum(intmax_t num)
 {
     int len = max_int_length(sizeof(num));
-    char buf[len];

-    len = fmtstr(buf, len, "%" PRIdMAX, num);
-    return memtodest(buf, len, flags);
+    expdest = makestrspace(len, expdest);
+    len = fmtstr(expdest, len, "%" PRIdMAX, num);
+    STADJUST(len, expdest);
+    return len;
 }

 STATIC void
diff --git a/src/expand.h b/src/expand.h
index c44b848..617b851 100644
--- a/src/expand.h
+++ b/src/expand.h
@@ -59,11 +59,11 @@  struct arglist {
 #define EXP_WORD    0x80    /* expand word in parameter expansion */
 #define EXP_QUOTED    0x100    /* expand word in double quotes */
 #define EXP_KEEPNUL    0x200    /* do not skip NUL characters */
-#define EXP_DISCARD    0x400    /* discard result of expansion */


 union node;
 void expandarg(union node *, struct arglist *, int);
+void expari(int);
 #define rmescapes(p) _rmescapes((p), 0)
 char *_rmescapes(char *, int);
 int casematch(union node *, char *);