Re: aliases+=(foo 'echo bar') crash

[Bart Schaefer <schaefer@brasslantern.com>]

On Jul 23,  5:52pm, Peter Stephenson wrote:
}
} I haven't got very far with this, but I'm suspicious of this blithe
} assumption in arrhashsetfn()...
} 
} 	/* ...but we can use the value without copying. */
} 	setstrvalue(v, *aptr++);

That does appear to be related; valgrind complains about it:

==5082== Invalid free() / delete / delete[]
==5082==    at 0x4004EFA: free (vg_replace_malloc.c:235)
==5082==    by 0x8091F38: zsfree (mem.c:1727)
==5082==    by 0x80A0572: strsetfn (params.c:3148)
==5082==    by 0x809DA30: setstrvalue (params.c:2297)
==5082==    by 0x80A07FE: arrhashsetfn (params.c:3247)
==5082==    by 0x809E234: setarrvalue (params.c:2472)
==5082==    by 0x809F724: assignaparam (params.c:2829)
==5082==    by 0x80650B7: addvars (exec.c:2304)
==5082==    by 0x8066030: execcmd (exec.c:2677)
==5082==    by 0x8063A59: execpline2 (exec.c:1691)
==5082==    by 0x8062DFE: execpline (exec.c:1478)
==5082==    by 0x80626D6: execlist (exec.c:1261)
==5082==  Address 0x43C16A8 is not stack'd, malloc'd or (recently) free'd

However, if we look at addvars (exec.c:2304):

   2286         if (vl) {
   2287             ptr = arr = (char **) zalloc(sizeof(char **) *
   2288                                          (countlinknodes(vl) + 1));
   2289 
   2290             while (nonempty(vl))
   2291                 *ptr++ = ztrdup((char *) ugetnode(vl));
   2292         } else
   2293             ptr = arr = (char **) zalloc(sizeof(char **));
   2294 
   2295         *ptr = NULL;
   2296         if (xtr) {
   2297             fprintf(xtrerr, "( ");
   2298             for (ptr = arr; *ptr; ptr++) {
   2299                 quotedzputs(*ptr, xtrerr);
   2300                 fputc(' ', xtrerr);
   2301             }
   2302             fprintf(xtrerr, ") ");
   2303         }
   2304         assignaparam(name, arr, myflags);

The "arr" pointer is zalloc'd and every value in it is ztrdup'd, so the
basic assumption seems to be good.

The real problem seems to be here:

   3224     /* Best not to shortcut this by using the existing hash table,   *
   3225      * since that could cause trouble for special hashes.  This way, *
   3226      * it's up to pm->gsu.h->setfn() what to do.                     */
   3227     int alen = arrlen(val);
   3228     HashTable opmtab = paramtab, ht = 0;
   3229     char **aptr = val;
   3230     Value v = (Value) hcalloc(sizeof *v);
   3231     v->end = -1;
...
   3242         /* The parameter name is ztrdup'd... */
   3243         v->pm = createparam(*aptr, PM_SCALAR|PM_UNSET);

The bad free that's being complained about is v->pm->u.str, which either
came from hcalloc() for v or from somewhere in createparam().  The crash
is actually here at ->setfn():

   2298     switch (PM_TYPE(v->pm->node.flags)) {
   2299     case PM_SCALAR:
   2300         if (v->start == 0 && v->end == -1) {
   2301             v->pm->gsu.s->setfn(v->pm, val);

The bad values in *pm come from here:

    857         oldpm = (Param) (paramtab == realparamtab ?
    858                          gethashnode2(paramtab, name) :
    859                          paramtab->getnode(paramtab, name));

(where paramtab != realparamtab).  That's as far as I've gotten.