Re: malloc error messages
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.
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 - 1.125
+++ stdlib/malloc.c 20 Oct 2010 07:13:16 -
@@ -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 ioveciov[5];
+ struct ioveciov[6];
+ charbuf[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
Re: malloc error messages
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. Otherwise reads fine and works for me. Good, I'm committing a different version. Add *slightly* here
Re: malloc error messages
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 - 1.125
+++ stdlib/malloc.c 20 Oct 2010 07:13:16 -
@@ -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 ioveciov[5];
+ struct ioveciov[6];
+ charbuf[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;
Re: malloc error messages
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,
Theo reminded me snprintf(3) can be used safely in this case.
-Otto
Index: malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.125
diff -u -p -r1.125 malloc.c
--- malloc.c18 May 2010 22:24:55 - 1.125
+++ malloc.c20 Oct 2010 18:45:59 -
@@ -352,12 +352,12 @@ malloc_exit(void)
#endif /* MALLOC_STATS */
-
static void
-wrterror(char *p)
+wrterror(char *msg, void *p)
{
char*q = error: ;
- struct ioveciov[5];
+ struct ioveciov[6];
+ charbuf[20];
iov[0].iov_base = __progname;
iov[0].iov_len = strlen(__progname);
@@ -365,11 +365,13 @@ 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);
+ iov[4].iov_base = buf;
+ iov[4].iov_len = p == NULL ? 0 : snprintf(buf, sizeof(buf), %p, p);
+ iov[5].iov_base = \n;
+ iov[5].iov_len = 1;
+ writev(STDERR_FILENO, iov, 6);
#ifdef MALLOC_STATS
if (mopts.malloc_stats)
@@ -414,13 +416,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 +436,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 +448,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 +463,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 +480,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 +499,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 +553,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 +726,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 +789,7 @@ omalloc_grow(struct dir_info
Re: malloc error messages
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.
Otherwise reads fine and works for me.
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 - 1.125
+++ stdlib/malloc.c 20 Oct 2010 07:13:16 -
@@ -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 ioveciov[5];
+ struct ioveciov[6];
+ charbuf[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);
