Re: [ECOS] Re: dlmalloc-2.6.4.c MALLOC_COPY broken?

[Andrew Lunn <andrew at lunn dot ch>]

> Try putting some asserts in memcpy to trap any occurances of this... you 
> will find the realloc breaks the rules.  It should use memmove instead.

Could you explain this please....

    // otherwise try to resize allocation
    newptr = POOL.resize_alloc( (cyg_uint8 *)ptr, size, &oldsize );

    if ( NULL == newptr ) {
        // if resize_alloc doesn't return a pointer, it failed, so we
        // just have to allocate new space instead, and later copy it
        
        CYG_ASSERT( oldsize != 0,
                    "resize_alloc() couldn't determine allocation size!" );

        newptr = malloc( size );
        
        if ( NULL != newptr ) {
            memcpy( newptr, ptr, size < (size_t) oldsize ? size
                    : (size_t) oldsize );
            free( ptr );
        }
    }
    
    CYG_REPORT_RETVAL( newptr );
    return newptr;
} // realloc()

I don't see how this can result in overlapping memory. newptr is a new
block of memory, so it is not possible that it overlaps with ptr.

Do you actually mean POOL.resize_alloc has the problem?

dlmalloc's MALLOC_COPY() does use memcpy if it is not using Duff's
device. The attached patch fixes this. What i'm not sure about is if
to rename CYGIMP_MEMALLOC_ALLOCATOR_DLMALLOC_USE_MEMCPY or
CYGIMP_MEMALLOC_ALLOCATOR_DLMALLOC_USE_MEMMOVE.

        Andrew

Index: services/memalloc/common/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/memalloc/common/current/ChangeLog,v
retrieving revision 1.39
diff -u -r1.39 ChangeLog
--- services/memalloc/common/current/ChangeLog	17 May 2006 16:14:02 -0000	1.39
+++ services/memalloc/common/current/ChangeLog	6 Jan 2008 12:07:05 -0000
@@ -1,3 +1,14 @@
+2008-01-06  Andrew Lunn  <andrew.lunn@ascom.ch>
+
+	* src/dlmalloc.cxx (MALLOC_COPY): 
+
+	* cdl/memalloc.cdl: Use memmove instead of memcpy which can go
+	wrong in realloc() when the new and old block overlaps.
+	CYGIMP_MEMALLOC_ALLOCATOR_DLMALLOC_USE_MEMCPY has been retained
+	instead of being renamed to ..._MEMMOVE to help backward
+	compatibility with older configurations. Thanks to Oyvind Harboe
+	and Dave Lawrence.
+
 2006-05-17  Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* doc/memalloc.sgml: Fix parameters for calloc. Reported by
Index: services/memalloc/common/current/cdl/memalloc.cdl
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/memalloc/common/current/cdl/memalloc.cdl,v
retrieving revision 1.15
diff -u -r1.15 memalloc.cdl
--- services/memalloc/common/current/cdl/memalloc.cdl	30 Jul 2005 11:42:55 -0000	1.15
+++ services/memalloc/common/current/cdl/memalloc.cdl	6 Jan 2008 12:07:05 -0000
@@ -176,11 +176,11 @@
             }
 
            cdl_option CYGIMP_MEMALLOC_ALLOCATOR_DLMALLOC_USE_MEMCPY {
-                display       "Use system memcpy() and memset()"
+                display       "Use system memmove() and memset()"
                 requires      CYGPKG_ISOINFRA
                 default_value { 0 != CYGPKG_ISOINFRA }
                 description   "
-                    This may be used to control whether memset() and memcpy()
+                    This may be used to control whether memset() and memmove()
                     are used within the implementation. The alternative is
                     to use some macro equivalents, which some people report
                     are faster in some circumstances."
Index: services/memalloc/common/current/src/dlmalloc.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/services/memalloc/common/current/src/dlmalloc.cxx,v
retrieving revision 1.10
diff -u -r1.10 dlmalloc.cxx
--- services/memalloc/common/current/src/dlmalloc.cxx	4 Oct 2004 12:02:57 -0000	1.10
+++ services/memalloc/common/current/src/dlmalloc.cxx	6 Jan 2008 12:07:06 -0000
@@ -295,7 +295,7 @@
 
 #ifdef CYGIMP_MEMALLOC_ALLOCATOR_DLMALLOC_USE_MEMCPY
 
-#include <string.h>                    // memcpy, memset
+#include <string.h>                    // memmove, memset
 
 /* The following macros are only invoked with (2n+1)-multiples of
    INTERNAL_SIZE_T units, with a positive integer n. This is exploited
@@ -333,7 +333,7 @@
                                      *mcdst++ = *mcsrc++;                     \
                                      *mcdst++ = *mcsrc++;                     \
                                      *mcdst   = *mcsrc  ;                     \
-  } else memcpy(dest, src, mcsz);                                             \
+  } else memmove(dest, src, mcsz);                                             \
 } while(0)
 
 #else /* !CYGIMP_MEMALLOC_ALLOCATOR_DLMALLOC_USE_MEMCPY */