From: Andrey Borzenkov <arvidjaar@newmail.ru>
To: "Zsh-workers" <zsh-workers@sunsite.dk>
Subject: [PATCH][RFC] check for heap memory in zfree()
Date: Sat, 4 Mar 2006 11:57:59 +0300 [thread overview]
Message-ID: <200603041158.02573.arvidjaar@newmail.ru> (raw)
In-Reply-To: <200603041104.48265.arvidjaar@mail.ru>
[-- Attachment #1.1: Type: text/plain, Size: 1734 bytes --]
I did not receive my previous mail so I am not sure if it was ever
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-fly
> 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=($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.
Comments on whether it makes sense? I am rather concerned that it may hide
real problem sometimes (i.e. instead of crashing right away memory that was
_supposed_ to be permanently allocated may end up in heap and be silently
removed later; it is apparently harder to debug).
The patch does fix dirstack crash BTW.
-andrey
[-- Attachment #1.2: do_not_free_heap.patch --]
[-- Type: text/x-diff, Size: 1360 bytes --]
Index: Src/mem.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/mem.c,v
retrieving revision 1.12
diff -u -p -r1.12 mem.c
--- 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
}
-/* 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)
- zfree(p, strlen(p) + 1);
-}
-
MALLOC_RET_T
realloc(MALLOC_RET_T p, MALLOC_ARG_T size)
{
@@ -1463,19 +1452,34 @@ bin_mem(char *name, char **argv, Options
/**/
mod_export void
-zfree(void *p, UNUSED(int sz))
+zfree(void *p, int sz)
{
- if (p)
+ Heap h;
+
+ if (!p)
+ return;
+
+ queue_signals();
+ for (h = heaps; h; h = h->next)
+ if ((char *)p >= arena(h) && (char *)p + sz < arena(h) + ARENA_SIZEOF(h))
+ break;
+ unqueue_signals();
+
+ /* Do not free memory allocated in heap */
+ if (!h)
free(p);
}
/**/
+#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)
- free(p);
+ zfree(p, strlen(p) + 1);
}
-
-/**/
-#endif
[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]
next parent reply other threads:[~2006-03-04 8:58 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20060302175252.GA31734@let.rug.nl>
[not found] ` <200603041104.48265.arvidjaar@mail.ru>
2006-03-04 8:57 ` Andrey Borzenkov [this message]
2006-03-05 9:13 ` Bart Schaefer
2006-03-05 17:23 ` Peter Stephenson
2006-03-05 20:43 ` Bart Schaefer
2006-03-06 10:32 ` Peter Stephenson
2006-03-06 16:25 ` Bart Schaefer
2006-03-04 9:28 ` [PATCH] Re: dirstack history: loving zsh, crashing zsh Andrey Borzenkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200603041158.02573.arvidjaar@newmail.ru \
--to=arvidjaar@newmail.ru \
--cc=zsh-workers@sunsite.dk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).