On Thu, Oct 21, 2010 at 10:06:23AM +0200, Otto Moerbeek wrote: > On Wed, Oct 20, 2010 at 07:59:27PM +0100, Nicholas Marriott wrote: > > > I like it a lot. > > > > Although I don't see much point in NBBY :-). > > > > Couldn't you print the failing pointer on the other realloc messages? I > > guess it may not be much use. > > I'm sorry, but I don't get you here.
I meant some of the other messages in orealloc eg "guard size", but having the pointer wouldn't tell the user much I guess. > > > > > Otherwise reads fine and works for me. > > Good, I'm committing a different version. > > -Otto > > > > > > On Wed, Oct 20, 2010 at 09:27:50AM +0200, Otto Moerbeek wrote: > > > Hi, > > > > > > when feeding free(3) or realloc(3) a bogus or already free pointer, it > > > helps to see the actual pointer. So here's a diff to do that. > > > > > > Typical output: > > > > > > a.out in free(): error: bogus pointer (double free?) 0x1234567890abcdef > > > > > > Please test & review, > > > > > > -Otto > > > > > > Index: stdlib/malloc.c > > > =================================================================== > > > RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v > > > retrieving revision 1.125 > > > diff -u -p -r1.125 malloc.c > > > --- stdlib/malloc.c 18 May 2010 22:24:55 -0000 1.125 > > > +++ stdlib/malloc.c 20 Oct 2010 07:13:16 -0000 > > > @@ -352,12 +352,30 @@ malloc_exit(void) > > > #endif /* MALLOC_STATS */ > > > > > > > > > +static void > > > +ptr2str(struct iovec *io, void *p, char *buf) > > > +{ > > > + int i, j; > > > + uintptr_t n = (uintptr_t)p; > > > + > > > + io->iov_base = buf; > > > + io->iov_len = 0; > > > + if (p == NULL) > > > + return; > > > + buf[0] = ' '; > > > + buf[1] = '0'; > > > + buf[2] = 'x'; > > > + for (j = 3, i = sizeof(n) * NBBY - NBBY/2; i >= 0; j++, i -= NBBY/2) > > > + buf[j] = "0123456789abcdef"[(n >> i) & 0xf]; > > > + io->iov_len = j; > > > +} > > > > > > static void > > > -wrterror(char *p) > > > +wrterror(char *msg, void *p) > > > { > > > char *q = " error: "; > > > - struct iovec iov[5]; > > > + struct iovec iov[6]; > > > + char buf[20]; > > > > > > iov[0].iov_base = __progname; > > > iov[0].iov_len = strlen(__progname); > > > @@ -365,11 +383,12 @@ wrterror(char *p) > > > iov[1].iov_len = strlen(malloc_func); > > > iov[2].iov_base = q; > > > iov[2].iov_len = strlen(q); > > > - iov[3].iov_base = p; > > > - iov[3].iov_len = strlen(p); > > > - iov[4].iov_base = "\n"; > > > - iov[4].iov_len = 1; > > > - writev(STDERR_FILENO, iov, 5); > > > + iov[3].iov_base = msg; > > > + iov[3].iov_len = strlen(msg); > > > + ptr2str(&iov[4], p, buf); > > > + iov[5].iov_base = "\n"; > > > + iov[5].iov_len = 1; > > > + writev(STDERR_FILENO, iov, 6); > > > > > > #ifdef MALLOC_STATS > > > if (mopts.malloc_stats) > > > @@ -414,13 +433,13 @@ unmap(struct dir_info *d, void *p, size_ > > > u_int i, offset; > > > > > > if (sz != PAGEROUND(sz)) { > > > - wrterror("munmap round"); > > > + wrterror("munmap round", NULL); > > > return; > > > } > > > > > > if (psz > mopts.malloc_cache) { > > > if (munmap(p, sz)) > > > - wrterror("munmap"); > > > + wrterror("munmap", p); > > > malloc_used -= sz; > > > return; > > > } > > > @@ -434,7 +453,7 @@ unmap(struct dir_info *d, void *p, size_ > > > if (r->p != NULL) { > > > rsz = r->size << MALLOC_PAGESHIFT; > > > if (munmap(r->p, rsz)) > > > - wrterror("munmap"); > > > + wrterror("munmap", r->p); > > > r->p = NULL; > > > if (tounmap > r->size) > > > tounmap -= r->size; > > > @@ -446,7 +465,7 @@ unmap(struct dir_info *d, void *p, size_ > > > } > > > } > > > if (tounmap > 0) > > > - wrterror("malloc cache underflow"); > > > + wrterror("malloc cache underflow", NULL); > > > for (i = 0; i < mopts.malloc_cache; i++) { > > > r = &d->free_regions[i]; > > > if (r->p == NULL) { > > > @@ -461,9 +480,9 @@ unmap(struct dir_info *d, void *p, size_ > > > } > > > } > > > if (i == mopts.malloc_cache) > > > - wrterror("malloc free slot lost"); > > > + wrterror("malloc free slot lost", NULL); > > > if (d->free_regions_size > mopts.malloc_cache) > > > - wrterror("malloc cache overflow"); > > > + wrterror("malloc cache overflow", NULL); > > > } > > > > > > static void > > > @@ -478,7 +497,7 @@ zapcacheregion(struct dir_info *d, void > > > if (r->p == p) { > > > rsz = r->size << MALLOC_PAGESHIFT; > > > if (munmap(r->p, rsz)) > > > - wrterror("munmap"); > > > + wrterror("munmap", r->p); > > > r->p = NULL; > > > d->free_regions_size -= r->size; > > > r->size = 0; > > > @@ -497,9 +516,9 @@ map(struct dir_info *d, size_t sz, int z > > > > > > if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) || > > > d->canary1 != ~d->canary2) > > > - wrterror("internal struct corrupt"); > > > + wrterror("internal struct corrupt", NULL); > > > if (sz != PAGEROUND(sz)) { > > > - wrterror("map round"); > > > + wrterror("map round", NULL); > > > return NULL; > > > } > > > if (psz > d->free_regions_size) { > > > @@ -551,7 +570,7 @@ map(struct dir_info *d, size_t sz, int z > > > if (p != MAP_FAILED) > > > malloc_used += sz; > > > if (d->free_regions_size > mopts.malloc_cache) > > > - wrterror("malloc cache"); > > > + wrterror("malloc cache", NULL); > > > /* zero fill not needed */ > > > return p; > > > } > > > @@ -724,7 +743,7 @@ omalloc_init(struct dir_info **dp) > > > regioninfo_size = d->regions_total * sizeof(struct region_info); > > > d->r = MMAP(regioninfo_size); > > > if (d->r == MAP_FAILED) { > > > - wrterror("malloc init mmap failed"); > > > + wrterror("malloc init mmap failed", NULL); > > > d->regions_total = 0; > > > return 1; > > > } > > > @@ -787,7 +806,7 @@ omalloc_grow(struct dir_info *d) > > > } > > > /* avoid pages containing meta info to end up in cache */ > > > if (munmap(d->r, d->regions_total * sizeof(struct region_info))) > > > - wrterror("munmap"); > > > + wrterror("munmap", d->r); > > > else > > > malloc_used -= d->regions_total * sizeof(struct region_info); > > > d->regions_free = d->regions_free + d->regions_total; > > > @@ -853,7 +872,7 @@ find(struct dir_info *d, void *p) > > > > > > if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) || > > > d->canary1 != ~d->canary2) > > > - wrterror("internal struct corrupt"); > > > + wrterror("internal struct corrupt", NULL); > > > p = MASK_POINTER(p); > > > index = hash(p) & mask; > > > r = d->r[index].p; > > > @@ -876,7 +895,7 @@ delete(struct dir_info *d, struct region > > > size_t i, j, r; > > > > > > if (d->regions_total & (d->regions_total - 1)) > > > - wrterror("regions_total not 2^x"); > > > + wrterror("regions_total not 2^x", NULL); > > > d->regions_free++; > > > STATS_INC(g_pool->deletes); > > > > > > @@ -960,7 +979,7 @@ omalloc_make_chunks(struct dir_info *d, > > > > > > bits++; > > > if ((uintptr_t)pp & bits) > > > - wrterror("pp & bits"); > > > + wrterror("pp & bits", pp); > > > > > > insert(d, (void *)((uintptr_t)pp | bits), (uintptr_t)bp); > > > return bp; > > > @@ -980,7 +999,7 @@ malloc_bytes(struct dir_info *d, size_t > > > > > > if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) || > > > d->canary1 != ~d->canary2) > > > - wrterror("internal struct corrupt"); > > > + wrterror("internal struct corrupt", NULL); > > > /* Don't bother with anything less than this */ > > > /* unless we have a malloc(0) requests */ > > > if (size != 0 && size < MALLOC_MINSIZE) > > > @@ -1005,7 +1024,7 @@ malloc_bytes(struct dir_info *d, size_t > > > bp = LIST_FIRST(&d->chunk_dir[j]); > > > > > > if (bp->canary != d->canary1) > > > - wrterror("chunk info corrupted"); > > > + wrterror("chunk info corrupted", NULL); > > > /* Find first word of bitmap which isn't empty */ > > > for (lp = bp->bits; !*lp; lp++) > > > /* EMPTY */; > > > @@ -1029,7 +1048,7 @@ malloc_bytes(struct dir_info *d, size_t > > > k = 0; > > > } > > > if (lp - bp->bits > (bp->total - 1) / MALLOC_BITS) { > > > - wrterror("chunk overflow"); > > > + wrterror("chunk overflow", NULL); > > > errno = EFAULT; > > > return (NULL); > > > } > > > @@ -1065,17 +1084,17 @@ free_bytes(struct dir_info *d, struct re > > > > > > info = (struct chunk_info *)r->size; > > > if (info->canary != d->canary1) > > > - wrterror("chunk info corrupted"); > > > + wrterror("chunk info corrupted", NULL); > > > > > > /* Find the chunk number on the page */ > > > i = ((uintptr_t)ptr & MALLOC_PAGEMASK) >> info->shift; > > > > > > if ((uintptr_t)ptr & ((1UL << (info->shift)) - 1)) { > > > - wrterror("modified chunk-pointer"); > > > + wrterror("modified chunk-pointer", ptr); > > > return; > > > } > > > if (info->bits[i / MALLOC_BITS] & (1UL << (i % MALLOC_BITS))) { > > > - wrterror("chunk is already free"); > > > + wrterror("chunk is already free", ptr); > > > return; > > > } > > > > > > @@ -1133,7 +1152,7 @@ omalloc(size_t sz, int zero_fill) > > > if (mopts.malloc_guard) { > > > if (mprotect((char *)p + psz - mopts.malloc_guard, > > > mopts.malloc_guard, PROT_NONE)) > > > - wrterror("mprotect"); > > > + wrterror("mprotect", NULL); > > > malloc_guarded += mopts.malloc_guard; > > > } > > > > > > @@ -1182,7 +1201,7 @@ malloc_recurse(void) > > > > > > if (noprint == 0) { > > > noprint = 1; > > > - wrterror("recursive call"); > > > + wrterror("recursive call", NULL); > > > } > > > malloc_active--; > > > _MALLOC_UNLOCK(); > > > @@ -1195,7 +1214,7 @@ malloc_init(void) > > > if (omalloc_init(&g_pool)) { > > > _MALLOC_UNLOCK(); > > > if (mopts.malloc_xmalloc) > > > - wrterror("out of memory"); > > > + wrterror("out of memory", NULL); > > > errno = ENOMEM; > > > return -1; > > > } > > > @@ -1222,7 +1241,7 @@ malloc(size_t size) > > > malloc_active--; > > > _MALLOC_UNLOCK(); > > > if (r == NULL && mopts.malloc_xmalloc) { > > > - wrterror("out of memory"); > > > + wrterror("out of memory", NULL); > > > errno = ENOMEM; > > > } > > > if (r != NULL) > > > @@ -1238,7 +1257,7 @@ ofree(void *p) > > > > > > r = find(g_pool, p); > > > if (r == NULL) { > > > - wrterror("bogus pointer (double free?)"); > > > + wrterror("bogus pointer (double free?)", p); > > > return; > > > } > > > REALSIZE(sz, r); > > > @@ -1246,7 +1265,7 @@ ofree(void *p) > > > if (sz - mopts.malloc_guard >= MALLOC_PAGESIZE - > > > MALLOC_LEEWAY) { > > > if (r->p != p) { > > > - wrterror("bogus pointer"); > > > + wrterror("bogus pointer", p); > > > return; > > > } > > > } else { > > > @@ -1261,12 +1280,12 @@ ofree(void *p) > > > } > > > if (mopts.malloc_guard) { > > > if (sz < mopts.malloc_guard) > > > - wrterror("guard size"); > > > + wrterror("guard size", NULL); > > > if (!mopts.malloc_freeprot) { > > > if (mprotect((char *)p + PAGEROUND(sz) - > > > mopts.malloc_guard, mopts.malloc_guard, > > > PROT_READ | PROT_WRITE)) > > > - wrterror("mprotect"); > > > + wrterror("mprotect", NULL); > > > } > > > malloc_guarded -= mopts.malloc_guard; > > > } > > > @@ -1290,7 +1309,7 @@ ofree(void *p) > > > if (p != NULL) { > > > r = find(g_pool, p); > > > if (r == NULL) { > > > - wrterror("bogus pointer (double free?)"); > > > + wrterror("bogus pointer (double free?)", p); > > > return; > > > } > > > free_bytes(g_pool, r, p); > > > @@ -1311,7 +1330,7 @@ free(void *ptr) > > > malloc_func = " in free():"; > > > if (g_pool == NULL) { > > > _MALLOC_UNLOCK(); > > > - wrterror("free() called before allocation"); > > > + wrterror("free() called before allocation", NULL); > > > return; > > > } > > > if (malloc_active++) { > > > @@ -1337,7 +1356,7 @@ orealloc(void *p, size_t newsz) > > > > > > r = find(g_pool, p); > > > if (r == NULL) { > > > - wrterror("bogus pointer (double free?)"); > > > + wrterror("bogus pointer (double free?)", p); > > > return NULL; > > > } > > > if (newsz >= SIZE_MAX - mopts.malloc_guard - MALLOC_PAGESIZE) { > > > @@ -1349,7 +1368,7 @@ orealloc(void *p, size_t newsz) > > > goldsz = oldsz; > > > if (oldsz > MALLOC_MAXCHUNK) { > > > if (oldsz < mopts.malloc_guard) > > > - wrterror("guard size"); > > > + wrterror("guard size", NULL); > > > oldsz -= mopts.malloc_guard; > > > } > > > > > > @@ -1383,11 +1402,11 @@ orealloc(void *p, size_t newsz) > > > if (mprotect((char *)p + roldsz - > > > mopts.malloc_guard, mopts.malloc_guard, > > > PROT_READ | PROT_WRITE)) > > > - wrterror("mprotect"); > > > + wrterror("mprotect", NULL); > > > if (mprotect((char *)p + rnewsz - > > > mopts.malloc_guard, mopts.malloc_guard, > > > PROT_NONE)) > > > - wrterror("mprotect"); > > > + wrterror("mprotect", NULL); > > > } > > > unmap(g_pool, (char *)p + rnewsz, roldsz - rnewsz); > > > r->size = gnewsz; > > > @@ -1437,7 +1456,7 @@ realloc(void *ptr, size_t size) > > > malloc_active--; > > > _MALLOC_UNLOCK(); > > > if (r == NULL && mopts.malloc_xmalloc) { > > > - wrterror("out of memory"); > > > + wrterror("out of memory", NULL); > > > errno = ENOMEM; > > > } > > > if (r != NULL) > > > @@ -1464,7 +1483,7 @@ calloc(size_t nmemb, size_t size) > > > nmemb > 0 && SIZE_MAX / nmemb < size) { > > > _MALLOC_UNLOCK(); > > > if (mopts.malloc_xmalloc) > > > - wrterror("out of memory"); > > > + wrterror("out of memory", NULL); > > > errno = ENOMEM; > > > return NULL; > > > } > > > @@ -1480,7 +1499,7 @@ calloc(size_t nmemb, size_t size) > > > malloc_active--; > > > _MALLOC_UNLOCK(); > > > if (r == NULL && mopts.malloc_xmalloc) { > > > - wrterror("out of memory"); > > > + wrterror("out of memory", NULL); > > > errno = ENOMEM; > > > } > > > if (r != NULL)