PATCH: and query for malloc(0).

PATCH: and query for malloc(0).

[Peter Stephenson]

Playing with POSIX regexes, I found some errors coming from zsh's memory
allocation with debugging on.  These turned out to resolve to some
malloc()s and free()s for zero length and the following oddity in
malloc():

    /* some systems want malloc to return the highest valid address plus one
       if it is called with an argument of zero */

    if (!size)
	return (MALLOC_RET_T) m_high;

Wuh...err...wuh...err...?   As I've commented in the patch below,

       TODO: really?  Suppose we allocate more memory, so
       that this is now in bounds, then a more rational application
       that thinks it can free() anything it malloc'ed, even
       of zero length, calls free for it?  Aren't we in big
       trouble?  Wouldn't it be safer just to allocate some
       memory anyway?

Actually, what was causing the immediate problem was a free() on
the address when it *was* still m_high, which was (correctly) being
flagged as an error by memory debugging (valgrind agreed something
was rotten).  But actually I think that's a side effect of the odd
code above.

This code already seemed to be ancient at the first mention of mem.c in
the mailing list archive in 1995.  If anyone knows anything, please
speak.  Otherwise I will commit the following patch.  valgrind and
the shell's own routines are much happier with it.

Index: Src/mem.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/mem.c,v
retrieving revision 1.14
diff -u -r1.14 mem.c
--- Src/mem.c	30 May 2006 22:35:03 -0000	1.14
+++ Src/mem.c	27 Apr 2007 21:24:56 -0000
@@ -830,10 +830,26 @@
 #endif
 
     /* some systems want malloc to return the highest valid address plus one
-       if it is called with an argument of zero */
+       if it is called with an argument of zero.
+    
+       TODO: really?  Suppose we allocate more memory, so
+       that this is now in bounds, then a more rational application
+       that thinks it can free() anything it malloc'ed, even
+       of zero length, calls free for it?  Aren't we in big
+       trouble?  Wouldn't it be safer just to allocate some
+       memory anyway?
+
+       If the above comment is really correct, then at least
+       we need to check in free() if we're freeing memory
+       at m_high.
+    */
 
     if (!size)
+#if 1
+	size = 1;
+#else
 	return (MALLOC_RET_T) m_high;
+#endif
 
     queue_signals();  /* just queue signals rather than handling them */
 
-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PATCH: and query for malloc(0).

[Geoff Wing]

On Saturday 2007-04-28 07:36 +1000, Peter Stephenson output:
:Playing with POSIX regexes, I found some errors coming from zsh's memory
:allocation with debugging on.  These turned out to resolve to some
:malloc()s and free()s for zero length and the following oddity in
:malloc():
:    /* some systems want malloc to return the highest valid address plus one
:       if it is called with an argument of zero */
:    if (!size)
:	return (MALLOC_RET_T) m_high;

This appeared in zsh 2.5.  I don't know the origin.

It's a bit weird as different systems will do conflicting things with it.

NetBSD returns NULL

An older Solaris man page says for malloc(): "a unique pointer to the arena
 is returned"; and for bsdmalloc(): "a non-NULL pointer. ... These pointers
 should not be dereferenced."

SUS says: "If size is 0, either a null pointer or a unique pointer that can
 be successfully passed to free() shall be returned."

Given the above, I guess the patch is a reasonable fix.

We are not actually calling malloc(0) in-house, are we?

Regards,
Geoff

Re: PATCH: and query for malloc(0).

[Peter Stephenson]

Geoff Wing wrote:
> On Saturday 2007-04-28 07:36 +1000, Peter Stephenson output:
> :Playing with POSIX regexes, I found some errors coming from zsh's memory
> :allocation with debugging on.  These turned out to resolve to some
> :malloc()s and free()s for zero length and the following oddity in
> :malloc():
> :    /* some systems want malloc to return the highest valid address plus one
> :       if it is called with an argument of zero */
> :    if (!size)
> :	return (MALLOC_RET_T) m_high;
>...
> Given the above, I guess the patch is a reasonable fix.

Thanks, I think I'll commit it and see what happens; under most
circumstances that will be very little.

> We are not actually calling malloc(0) in-house, are we?

No, that should be trapped up above in zalloc(), etc.  I've only seen
the problem when linked with a C library function that's calling
malloc(0).

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php

To get further information regarding CSR, please visit our Investor Relations page at http://ir.csr.com/csr/about/overview