* aliases+=(foo 'echo bar') crash @ 2014-07-23 16:09 Stephane Chazelas 2014-07-23 16:52 ` Peter Stephenson 0 siblings, 1 reply; 7+ messages in thread From: Stephane Chazelas @ 2014-07-23 16:09 UTC (permalink / raw) To: zsh-workers $ zsh --version zsh 5.0.5 (x86_64-pc-linux-gnu) $ zsh -c 'aliases+=(a b)' *** Error in `zsh': double free or corruption (out): 0x00007f55e838f7e0 *** zsh: abort zsh -c 'aliases+=(a b)' (gdb) bt #0 0x00007ffff7135407 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007ffff71367e8 in __GI_abort () at abort.c:89 #2 0x00007ffff7173394 in __libc_message (do_abort=do_abort@entry=1, fmt=fmt@entry=0x7ffff7266010 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175 #3 0x00007ffff7178b6e in malloc_printerr (action=1, str=0x7ffff7266160 "double free or corruption (out)", ptr=<optimized out>) at malloc.c:4996 #4 0x00007ffff7179876 in _int_free (av=<optimized out>, p=<optimized out>, have_lock=0) at malloc.c:3840 #5 0x0000000000455305 in strsetfn () #6 0x000000000045a2f2 in ?? () #7 0x000000000045a7cc in arrhashsetfn () #8 0x000000000045b263 in assignaparam () #9 0x00000000004226cc in ?? () #10 0x00000000004272a5 in ?? () #11 0x0000000000428896 in ?? () #12 0x0000000000428c0e in ?? () #13 0x0000000000429db1 in execlist () #14 0x000000000042a030 in execode () #15 0x000000000042a102 in execstring () #16 0x000000000043bde2 in init_misc () #17 0x000000000043d29e in zsh_main () #18 0x00007ffff7121b45 in __libc_start_main (main=0x40f010 <main>, argc=3, argv=0x7fffffffdd98, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdd88) at libc-start.c:287 #19 0x000000000040f03e in _start () ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: aliases+=(foo 'echo bar') crash 2014-07-23 16:09 aliases+=(foo 'echo bar') crash Stephane Chazelas @ 2014-07-23 16:52 ` Peter Stephenson 2014-07-24 1:37 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2014-07-23 16:52 UTC (permalink / raw) To: zsh-workers On Wed, 23 Jul 2014 17:09:35 +0100 Stephane Chazelas <stephane.chazelas@gmail.com> wrote: > $ zsh -c 'aliases+=(a b)' > *** Error in `zsh': double free or corruption (out): 0x00007f55e838f7e0 *** > zsh: abort zsh -c 'aliases+=(a b)' 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++); The crash is down there (it comes up as just a debug warning for me, I assume it's the same thing). pws ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: aliases+=(foo 'echo bar') crash 2014-07-23 16:52 ` Peter Stephenson @ 2014-07-24 1:37 ` Bart Schaefer 2014-07-24 3:04 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2014-07-24 1:37 UTC (permalink / raw) To: zsh-workers 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: aliases+=(foo 'echo bar') crash 2014-07-24 1:37 ` Bart Schaefer @ 2014-07-24 3:04 ` Bart Schaefer 2014-07-24 5:45 ` [PATCH] " Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2014-07-24 3:04 UTC (permalink / raw) To: zsh-workers On Jul 23, 6:37pm, Bart Schaefer wrote: } } 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. So ->getnode() there is getpmralias() which calls getalias() which allocates everything on the heap. When arrhashsetfn() tries to replace the value returned from getalias(), kaboom. The problem may be that the aliases hash has been autoloaded from the zsh/parameter module AFTER the flow of control has already passed the point of deciding what function to call to perform the assignment, and so is using the generic hash function instead of the special functions for setting aliases. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Re: aliases+=(foo 'echo bar') crash 2014-07-24 3:04 ` Bart Schaefer @ 2014-07-24 5:45 ` Bart Schaefer 2014-07-24 9:47 ` Peter Stephenson 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2014-07-24 5:45 UTC (permalink / raw) To: zsh-workers On Jul 23, 8:04pm, Bart Schaefer wrote: } } The problem may be that the aliases hash has been autoloaded from the } zsh/parameter module AFTER the flow of control has already passed the } point of deciding what function to call to perform the assignment, and } so is using the generic hash function instead of the special functions } for setting aliases. It's both better and worse than that. When createparam() is called for a value that doesn't already exist in the aliases hash table, a new unset value is correctly created, but then assigngetset() is called on it, which clobbers the special gsu structure with the default setfn. I believe the following fixes it for the zsh/parameter module, but there may be other modules where the same problem arises. diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c index 22148f9..0385a70 100644 --- a/Src/Modules/parameter.c +++ b/Src/Modules/parameter.c @@ -106,7 +106,7 @@ getpmparameter(UNUSED(HashTable ht), const char *name) pm->u.str = paramtypestr(rpm); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -224,7 +224,7 @@ getpmcommand(UNUSED(HashTable ht), const char *name) } } else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -410,7 +410,7 @@ getfunction(UNUSED(HashTable ht), const char *name, int dis) } } else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -661,7 +661,7 @@ getbuiltin(UNUSED(HashTable ht), const char *name, int dis) pm->u.str = dupstring(t); } else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -876,7 +876,7 @@ getpmoption(UNUSED(HashTable ht), const char *name) } else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -934,7 +934,7 @@ getpmmodule(UNUSED(HashTable ht), const char *name) pm->u.str = dupstring(type); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1048,7 +1048,7 @@ getpmhistory(UNUSED(HashTable ht), const char *name) pm->u.str = dupstring(he->node.nam); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1158,7 +1158,7 @@ getpmjobtext(UNUSED(HashTable ht), const char *name) pm->u.str = pmjobtext(job); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1259,7 +1259,7 @@ getpmjobstate(UNUSED(HashTable ht), const char *name) pm->u.str = pmjobstate(job); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1325,7 +1325,7 @@ getpmjobdir(UNUSED(HashTable ht), const char *name) pm->u.str = pmjobdir(job); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1451,7 +1451,7 @@ getpmnameddir(UNUSED(HashTable ht), const char *name) pm->u.str = dupstring(nd->dir); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1502,7 +1502,7 @@ getpmuserdir(UNUSED(HashTable ht), const char *name) pm->u.str = dupstring(nd->dir); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1754,7 +1754,7 @@ getalias(HashTable alht, UNUSED(HashTable ht), const char *name, int flags) pm->u.str = dupstring(al->text); else { pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } return &pm->node; } @@ -1950,7 +1950,7 @@ getpmusergroups(UNUSED(HashTable ht), const char *name) if (!gs) { zerr("failed to retrieve groups for user: %e", errno); pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); return &pm->node; } @@ -1965,7 +1965,7 @@ getpmusergroups(UNUSED(HashTable ht), const char *name) } pm->u.str = dupstring(""); - pm->node.flags |= PM_UNSET; + pm->node.flags |= (PM_UNSET|PM_SPECIAL); return &pm->node; } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Re: aliases+=(foo 'echo bar') crash 2014-07-24 5:45 ` [PATCH] " Bart Schaefer @ 2014-07-24 9:47 ` Peter Stephenson 2014-07-24 15:29 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Peter Stephenson @ 2014-07-24 9:47 UTC (permalink / raw) To: zsh-workers On Wed, 23 Jul 2014 22:45:35 -0700 Bart Schaefer <schaefer@brasslantern.com> wrote: > - pm->node.flags |= PM_UNSET; > + pm->node.flags |= (PM_UNSET|PM_SPECIAL); It's a bit surprising this hasn't caused mayhem before now, but I suppose the damage is limited or non-existent until you try to set the parameters, and mostly that just gives you a non-standard interface to something that's usually done another way. pws ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Re: aliases+=(foo 'echo bar') crash 2014-07-24 9:47 ` Peter Stephenson @ 2014-07-24 15:29 ` Bart Schaefer 0 siblings, 0 replies; 7+ messages in thread From: Bart Schaefer @ 2014-07-24 15:29 UTC (permalink / raw) To: zsh-workers On Jul 24, 10:47am, Peter Stephenson wrote: } Subject: Re: [PATCH] Re: aliases+=(foo 'echo bar') crash } } On Wed, 23 Jul 2014 22:45:35 -0700 } Bart Schaefer <schaefer@brasslantern.com> wrote: } > - pm->node.flags |= PM_UNSET; } > + pm->node.flags |= (PM_UNSET|PM_SPECIAL); } } It's a bit surprising this hasn't caused mayhem before now, but I } suppose the damage is limited or non-existent until you try to set } the parameters Simple assignment via either aliases=(a b) or aliases[a]=b doesn't go through the same code path, so there was no issue until the += syntax was added ... and it's pretty unusual to append something to a hash table that way, so I'm not that surprised that no one encountered it. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-07-24 15:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-23 16:09 aliases+=(foo 'echo bar') crash Stephane Chazelas 2014-07-23 16:52 ` Peter Stephenson 2014-07-24 1:37 ` Bart Schaefer 2014-07-24 3:04 ` Bart Schaefer 2014-07-24 5:45 ` [PATCH] " Bart Schaefer 2014-07-24 9:47 ` Peter Stephenson 2014-07-24 15:29 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).