This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: [ECOS] Re: dlmalloc-2.6.4.c MALLOC_COPY broken?
- From: Andrew Lunn <andrew at lunn dot ch>
- To: Dave Lawrence <dlawrence at ad-holdings dot co dot uk>
- Cc: eCos Patches <ecos-patches at ecos dot sourceware dot org>
- Date: Sun, 6 Jan 2008 13:07:32 +0100
- Subject: Re: [ECOS] Re: dlmalloc-2.6.4.c MALLOC_COPY broken?
- References: <c09652430712310515qe60f9den48383d2d6a18a3a7@mail.gmail.com> <fli8qp$p7n$1@ger.gmane.org>
> 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 */