This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.
Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Hello,
Jakub wrote:
> > realloc was using memcpy even for copies where source and destination might
> > overlap. Found with new alphaev6 memcpy during make check.
Argh! You're right. This _must_ have been hard to find. Excellent
work.
> Thanks. Looks safe to me, Wolfram should comment as well. I've
> applied the patch after removing most of the MALLOC_MEMMOVE
> definition. It cannot be correct since it didn't change the copy
> direction. The macro for now simply calls memmove. If you want to
> provide a more complicated, faster version please go ahead.
Jakub's patch was actually correct, IMHO, although I wouldn't use the
name `MEMMOVE' in the macro. The copy direction does _not_ have to be
changed because dest is always below source in the cases where overlap
can occur. So my suggestion would be to just adapt the MALLOC_COPY()
macro and document those restrictions, which I've done in the patch
below (against current CVS head, where MALLOC_MEMMOVE was defined but
not used).
Regards,
Wolfram.
2000-12-28 Wolfram Gloger <wg@malloc.de>
* malloc/malloc.c (MALLOC_COPY): Handle case if source and
destination overlap. Assume dest is always below source if
overlapping.
Index: libc/malloc/malloc.c
===================================================================
RCS file: /cvs/glibc/libc/malloc/malloc.c,v
retrieving revision 1.76
diff -u -r1.76 malloc.c
--- malloc.c 2000/12/27 23:27:22 1.76
+++ malloc.c 2000/12/28 10:36:15
@@ -423,11 +423,12 @@
#endif
#endif
-#if USE_MEMCPY
-
/* The following macros are only invoked with (2n+1)-multiples of
INTERNAL_SIZE_T units, with a positive integer n. This is exploited
- for fast inline execution when n is small. */
+ for fast inline execution when n is small. If the regions to be
+ copied do overlap, the destination lies always _below_ the source. */
+
+#if USE_MEMCPY
#define MALLOC_ZERO(charp, nbytes) \
do { \
@@ -446,7 +447,9 @@
} else memset((charp), 0, mzsz); \
} while(0)
-#define MALLOC_COPY(dest,src,nbytes) \
+/* If the regions overlap, dest is always _below_ src. */
+
+#define MALLOC_COPY(dest,src,nbytes,overlap) \
do { \
INTERNAL_SIZE_T mcsz = (nbytes); \
if(mcsz <= 9*sizeof(mcsz)) { \
@@ -461,12 +464,12 @@
*mcdst++ = *mcsrc++; \
*mcdst++ = *mcsrc++; \
*mcdst = *mcsrc ; \
- } else memcpy(dest, src, mcsz); \
+ } else if(overlap) \
+ memmove(dest, src, mcsz); \
+ else \
+ memcpy(dest, src, mcsz); \
} while(0)
-#define MALLOC_MEMMOVE(dest,src,nbytes) \
- memmove(dest, src, mcsz)
-
#else /* !USE_MEMCPY */
/* Use Duff's device for good zeroing/copying performance. */
@@ -487,8 +490,10 @@
case 1: *mzp++ = 0; if(mcn <= 0) break; mcn--; } \
} \
} while(0)
+
+/* If the regions overlap, dest is always _below_ src. */
-#define MALLOC_COPY(dest,src,nbytes) \
+#define MALLOC_COPY(dest,src,nbytes,overlap) \
do { \
INTERNAL_SIZE_T* mcsrc = (INTERNAL_SIZE_T*) src; \
INTERNAL_SIZE_T* mcdst = (INTERNAL_SIZE_T*) dest; \
@@ -3255,7 +3260,7 @@
/* Must alloc, copy, free. */
newmem = mALLOc(bytes);
if (newmem == 0) return 0; /* propagate failure */
- MALLOC_COPY(newmem, oldmem, oldsize - 2*SIZE_SZ);
+ MALLOC_COPY(newmem, oldmem, oldsize - 2*SIZE_SZ, 0);
munmap_chunk(oldp);
return newmem;
}
@@ -3370,7 +3375,8 @@
unlink(prev, bck, fwd);
newp = prev;
newsize += prevsize + nextsize;
- MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize);
+ MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize,
+ 1);
top(ar_ptr) = chunk_at_offset(newp, nb);
set_head(top(ar_ptr), (newsize - nb) | PREV_INUSE);
set_head_size(newp, nb);
@@ -3385,7 +3391,7 @@
unlink(prev, bck, fwd);
newp = prev;
newsize += nextsize + prevsize;
- MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize);
+ MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize, 1);
goto split;
}
}
@@ -3396,7 +3402,7 @@
unlink(prev, bck, fwd);
newp = prev;
newsize += prevsize;
- MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize);
+ MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize, 1);
goto split;
}
}
@@ -3436,7 +3442,7 @@
}
/* Otherwise copy, free, and exit */
- MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize);
+ MALLOC_COPY(BOUNDED_N(chunk2mem(newp), oldsize), oldmem, oldsize, 0);
chunk_free(ar_ptr, oldp);
return newp;
}
@@ -4605,7 +4611,7 @@
newp = (top_check() >= 0) ? chunk_alloc(&main_arena, nb) : NULL;
if (newp) {
MALLOC_COPY(BOUNDED_N(chunk2mem(newp), nb),
- oldmem, oldsize - 2*SIZE_SZ);
+ oldmem, oldsize - 2*SIZE_SZ, 0);
munmap_chunk(oldp);
}
}
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |