From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9128 invoked from network); 4 Mar 2006 08:58:17 -0000 X-Spam-Checker-Version: SpamAssassin 3.1.0 (2005-09-13) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00, FORGED_RCVD_HELO autolearn=ham version=3.1.0 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 4 Mar 2006 08:58:17 -0000 Received: (qmail 62253 invoked from network); 4 Mar 2006 08:58:10 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 4 Mar 2006 08:58:10 -0000 Received: (qmail 9181 invoked by alias); 4 Mar 2006 08:58:07 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 22319 Received: (qmail 9171 invoked from network); 4 Mar 2006 08:58:07 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by sunsite.dk with SMTP; 4 Mar 2006 08:58:07 -0000 Received: (qmail 61799 invoked from network); 4 Mar 2006 08:58:07 -0000 Received: from flock1.newmail.ru (80.68.241.157) by a.mx.sunsite.dk with SMTP; 4 Mar 2006 08:58:05 -0000 Received: (qmail 30296 invoked from network); 4 Mar 2006 08:58:03 -0000 Received: from unknown (HELO cooker.local) (arvidjaar@newmail.ru@83.237.228.20) by smtpd.newmail.ru with SMTP; 4 Mar 2006 08:58:03 -0000 From: Andrey Borzenkov To: "Zsh-workers" Subject: [PATCH][RFC] check for heap memory in zfree() Date: Sat, 4 Mar 2006 11:57:59 +0300 User-Agent: KMail/1.9.1 References: <20060302175252.GA31734@let.rug.nl> <200603041104.48265.arvidjaar@mail.ru> In-Reply-To: <200603041104.48265.arvidjaar@mail.ru> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart2090029.ZZ5cRtOz42"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <200603041158.02573.arvidjaar@newmail.ru> --nextPart2090029.ZZ5cRtOz42 Content-Type: multipart/mixed; boundary="Boundary-01=_aaVCEjNXDW3EnhX" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_aaVCEjNXDW3EnhX Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline I did not receive my previous mail so I am not sure if it was ever=20 delivered ... On Saturday 04 March 2006 11:04, Andrey Borzenkov wrote: > [moved to workers] > > On Thursday 02 March 2006 20:52, Francisco Borges wrote: > > % typeset -U dirstack > > > > and the shell crashed. > > The problem is rather non-trivial. dirsgetfn returns array built on-the-f= ly > in heap, while typeset -U calls uniqarray() that tries to zfree array > elements. There are at least two problems here: > > - typeset -U is not prepared to deal with "pseudo" parameters at all. It > assumes a->getfn() returns pointer to real parameter value. So it would > have not worked for dirstack anyway > > - I was about to change typeset -U to pm->gsu.a->setfn(pm, > pm->gsu.a->getfn(pm)) (basically doing foo=3D($foo)) and adding uniqarray > call to dirssetfn() when I realized that it would not help at all in this > case as dirssetfn() tries to free passed value too; so it would have > crashed just the same. > > Apparently to solve it in general we need one of > > - per-parameter type ->uniq function (is it an overkill?) Possibly > generalized to per-parameter ->setflags function. > > - some way to know if passed pointer was allocated from heap or not. I > guess it should be possible; something like isheap(p)? > OK attached is patch that checks if memory has been allocated from heap.=20 Comments on whether it makes sense? I am rather concerned that it may hide= =20 real problem sometimes (i.e. instead of crashing right away memory that was= =20 _supposed_ to be permanently allocated may end up in heap and be silently=20 removed later; it is apparently harder to debug). The patch does fix dirstack crash BTW. =2Dandrey --Boundary-01=_aaVCEjNXDW3EnhX Content-Type: text/x-diff; charset="iso-8859-1"; name="do_not_free_heap.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="do_not_free_heap.patch" Index: Src/mem.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsroot/zsh/zsh/Src/mem.c,v retrieving revision 1.12 diff -u -p -r1.12 mem.c =2D-- Src/mem.c 17 Jul 2004 19:25:14 -0000 1.12 +++ Src/mem.c 4 Mar 2006 08:51:40 -0000 @@ -1241,17 +1241,6 @@ free(FREE_ARG_T p) #endif } =20 =2D/* this one is for strings (and only strings, real strings, real C strin= gs, =2D those that have a zero byte at the end) */ =2D =2D/**/ =2Dmod_export void =2Dzsfree(char *p) =2D{ =2D if (p) =2D zfree(p, strlen(p) + 1); =2D} =2D MALLOC_RET_T realloc(MALLOC_RET_T p, MALLOC_ARG_T size) { @@ -1463,19 +1452,34 @@ bin_mem(char *name, char **argv, Options =20 /**/ mod_export void =2Dzfree(void *p, UNUSED(int sz)) +zfree(void *p, int sz) { =2D if (p) + Heap h; + + if (!p) + return; + + queue_signals(); + for (h =3D heaps; h; h =3D h->next) + if ((char *)p >=3D arena(h) && (char *)p + sz < arena(h) + ARENA_SIZEOF(h= )) + break; + unqueue_signals(); + + /* Do not free memory allocated in heap */ + if (!h) free(p); } =20 /**/ +#endif + +/* this one is for strings (and only strings, real strings, real C strings, + those that have a zero byte at the end) */ + +/**/ mod_export void zsfree(char *p) { if (p) =2D free(p); + zfree(p, strlen(p) + 1); } =2D =2D/**/ =2D#endif --Boundary-01=_aaVCEjNXDW3EnhX-- --nextPart2090029.ZZ5cRtOz42 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2.1 (GNU/Linux) iD8DBQBECVaaR6LMutpd94wRAgUQAJ4n0GRuJaTmLEInYpNMbw+w21pIbwCeLmNj 7UBvz7a3FqueYyJDyghzvbU= =S80w -----END PGP SIGNATURE----- --nextPart2090029.ZZ5cRtOz42--