Re: improve realloc(3)
On Mon, Jan 09, 2017 at 07:49:24PM +0100, Otto Moerbeek wrote:
> Hi,
>
> this diff implements some improvements to realloc and some cleanup of
> the MALLOC_MOVE code.
>
> 1. When shrinking a chunk allocation, compare the size of the current
> allocation to the size of the new allocation (instead of the requested size).
> Current code does a malloc-free-copy dance in too many cases.
>
> 2. Current code takes the easy way and always reallocates if C is
> active. This diff fixes by carefully updating the recorded requested
> size in all cases, and writing the canary bytes in the proper location
> after reallocating.
>
> 3. Introduce defines to test if MALLOC_MOVE should be done and to
> compute the new value.
>
> Please review and test with you favorite (and other) combination of
> malloc flags.
Anybody tested?
No feedback no progress, Jimi Hendrix could have said
-Otto
>
> Index: malloc.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.211
> diff -u -p -r1.211 malloc.c
> --- malloc.c 4 Nov 2016 09:11:20 - 1.211
> +++ malloc.c 9 Jan 2017 13:35:29 -
> @@ -73,6 +73,12 @@
> * Set to zero to be the most strict.
> */
> #define MALLOC_LEEWAY0
> +#define MALLOC_MOVE_COND(sz) ((sz) - mopts.malloc_guard <\
> + MALLOC_PAGESIZE - MALLOC_LEEWAY)
> +#define MALLOC_MOVE(p, sz) (((char *)(p)) +\
> + ((MALLOC_PAGESIZE - MALLOC_LEEWAY - \
> + ((sz) - mopts.malloc_guard)) & \
> + ~(MALLOC_MINSIZE - 1)))
>
> #define PAGEROUND(x) (((x) + (MALLOC_PAGEMASK)) & ~MALLOC_PAGEMASK)
>
> @@ -199,6 +205,7 @@ char *malloc_options;/* compile-time
> o
> static u_char getrbyte(struct dir_info *d);
> static __dead void wrterror(struct dir_info *d, char *msg, ...)
> __attribute__((__format__ (printf, 2, 3)));
> +static void fill_canary(char *ptr, size_t sz, size_t allocated);
>
> #ifdef MALLOC_STATS
> void malloc_dump(int, int, struct dir_info *);
> @@ -209,8 +216,8 @@ static void malloc_exit(void);
> #define CALLER NULL
> #endif
>
> -/* low bits of r->p determine size: 0 means >= page size and p->size holding
> - * real size, otherwise r->size is a shift count, or 1 for malloc(0)
> +/* low bits of r->p determine size: 0 means >= page size and r->size holding
> + * real size, otherwise low bits are a shift count, or 1 for malloc(0)
> */
> #define REALSIZE(sz, r) \
> (sz) = (uintptr_t)(r)->p & MALLOC_PAGEMASK, \
> @@ -905,23 +912,10 @@ omalloc_make_chunks(struct dir_info *d,
> return bp;
> }
>
> -
> -/*
> - * Allocate a chunk
> - */
> -static void *
> -malloc_bytes(struct dir_info *d, size_t argsize, void *f)
> +static int
> +find_chunksize(size_t size)
> {
> - int i, j, listnum;
> - size_t k, size;
> - u_short u, *lp;
> - struct chunk_info *bp;
> -
> - if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) ||
> - d->canary1 != ~d->canary2)
> - wrterror(d, "internal struct corrupt");
> -
> - size = argsize;
> + int i, j;
>
> /* Don't bother with anything less than this */
> /* unless we have a malloc(0) requests */
> @@ -937,6 +931,25 @@ malloc_bytes(struct dir_info *d, size_t
> while (i >>= 1)
> j++;
> }
> + return j;
> +}
> +
> +/*
> + * Allocate a chunk
> + */
> +static void *
> +malloc_bytes(struct dir_info *d, size_t size, void *f)
> +{
> + int i, j, listnum;
> + size_t k;
> + u_short u, *lp;
> + struct chunk_info *bp;
> +
> + if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) ||
> + d->canary1 != ~d->canary2)
> + wrterror(d, "internal struct corrupt");
> +
> + j = find_chunksize(size);
>
> listnum = getrbyte(d) % MALLOC_CHUNK_LISTS;
> /* If it's empty, make a page more of that size chunks */
> @@ -990,25 +1003,30 @@ malloc_bytes(struct dir_info *d, size_t
> k += (lp - bp->bits) * MALLOC_BITS;
>
> if (mopts.chunk_canaries)
> - bp->bits[bp->offset + k] = argsize;
> + bp->bits[bp->offset + k] = size;
>
> k <<= bp->shift;
>
> if (bp->size > 0) {
> if (mopts.malloc_junk == 2)
> memset((char *)bp->page + k, SOME_JUNK, bp->size);
> - else if (mopts.chunk_canaries) {
> - size_t sz = bp->size - argsize;
> -
> - if (sz > CHUNK_CHECK_LENGTH)
> - sz = CHUNK_CHECK_LENGTH;
> - memset((char *)bp->page + k + argsize, SOME_JUNK, sz);
> - }
> +
improve realloc(3)
Hi,
this diff implements some improvements to realloc and some cleanup of
the MALLOC_MOVE code.
1. When shrinking a chunk allocation, compare the size of the current
allocation to the size of the new allocation (instead of the requested size).
Current code does a malloc-free-copy dance in too many cases.
2. Current code takes the easy way and always reallocates if C is
active. This diff fixes by carefully updating the recorded requested
size in all cases, and writing the canary bytes in the proper location
after reallocating.
3. Introduce defines to test if MALLOC_MOVE should be done and to
compute the new value.
Please review and test with you favorite (and other) combination of
malloc flags.
-Otto
Index: malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.211
diff -u -p -r1.211 malloc.c
--- malloc.c4 Nov 2016 09:11:20 - 1.211
+++ malloc.c9 Jan 2017 13:35:29 -
@@ -73,6 +73,12 @@
* Set to zero to be the most strict.
*/
#define MALLOC_LEEWAY 0
+#define MALLOC_MOVE_COND(sz) ((sz) - mopts.malloc_guard <\
+ MALLOC_PAGESIZE - MALLOC_LEEWAY)
+#define MALLOC_MOVE(p, sz) (((char *)(p)) +\
+ ((MALLOC_PAGESIZE - MALLOC_LEEWAY - \
+ ((sz) - mopts.malloc_guard)) & \
+ ~(MALLOC_MINSIZE - 1)))
#define PAGEROUND(x) (((x) + (MALLOC_PAGEMASK)) & ~MALLOC_PAGEMASK)
@@ -199,6 +205,7 @@ char*malloc_options;/* compile-time
o
static u_char getrbyte(struct dir_info *d);
static __dead void wrterror(struct dir_info *d, char *msg, ...)
__attribute__((__format__ (printf, 2, 3)));
+static void fill_canary(char *ptr, size_t sz, size_t allocated);
#ifdef MALLOC_STATS
void malloc_dump(int, int, struct dir_info *);
@@ -209,8 +216,8 @@ static void malloc_exit(void);
#define CALLER NULL
#endif
-/* low bits of r->p determine size: 0 means >= page size and p->size holding
- * real size, otherwise r->size is a shift count, or 1 for malloc(0)
+/* low bits of r->p determine size: 0 means >= page size and r->size holding
+ * real size, otherwise low bits are a shift count, or 1 for malloc(0)
*/
#define REALSIZE(sz, r)\
(sz) = (uintptr_t)(r)->p & MALLOC_PAGEMASK, \
@@ -905,23 +912,10 @@ omalloc_make_chunks(struct dir_info *d,
return bp;
}
-
-/*
- * Allocate a chunk
- */
-static void *
-malloc_bytes(struct dir_info *d, size_t argsize, void *f)
+static int
+find_chunksize(size_t size)
{
- int i, j, listnum;
- size_t k, size;
- u_short u, *lp;
- struct chunk_info *bp;
-
- if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) ||
- d->canary1 != ~d->canary2)
- wrterror(d, "internal struct corrupt");
-
- size = argsize;
+ int i, j;
/* Don't bother with anything less than this */
/* unless we have a malloc(0) requests */
@@ -937,6 +931,25 @@ malloc_bytes(struct dir_info *d, size_t
while (i >>= 1)
j++;
}
+ return j;
+}
+
+/*
+ * Allocate a chunk
+ */
+static void *
+malloc_bytes(struct dir_info *d, size_t size, void *f)
+{
+ int i, j, listnum;
+ size_t k;
+ u_short u, *lp;
+ struct chunk_info *bp;
+
+ if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) ||
+ d->canary1 != ~d->canary2)
+ wrterror(d, "internal struct corrupt");
+
+ j = find_chunksize(size);
listnum = getrbyte(d) % MALLOC_CHUNK_LISTS;
/* If it's empty, make a page more of that size chunks */
@@ -990,25 +1003,30 @@ malloc_bytes(struct dir_info *d, size_t
k += (lp - bp->bits) * MALLOC_BITS;
if (mopts.chunk_canaries)
- bp->bits[bp->offset + k] = argsize;
+ bp->bits[bp->offset + k] = size;
k <<= bp->shift;
if (bp->size > 0) {
if (mopts.malloc_junk == 2)
memset((char *)bp->page + k, SOME_JUNK, bp->size);
- else if (mopts.chunk_canaries) {
- size_t sz = bp->size - argsize;
-
- if (sz > CHUNK_CHECK_LENGTH)
- sz = CHUNK_CHECK_LENGTH;
- memset((char *)bp->page + k + argsize, SOME_JUNK, sz);
- }
+ else if (mopts.chunk_canaries)
+ fill_canary((char *)bp->page + k, size, bp->size);
}
return ((char *)bp->page + k);
}
static void
+fill_canary(char *ptr, size_t sz, size_t allocated)
+{
+ size_t check_sz = allocated - sz;
+
+ if (check_sz > CHUNK_CHECK
