This annotates malloc and friends so that valgrind can track the heap. To do this, we need to follow a few rules:
* Call VALGRIND_MALLOCLIKE_BLOCK whenever we malloc something * Call VALGRIND_FREELIKE_BLOCK whenever we free something (generally after we have done our bookkeeping) * Call VALGRIND_RESIZEINPLACE_BLOCK whenever we change the size of an allocation. Generally this just needs to happen in realloc, but only if the address stays the same.
In addition to the above, dlmalloc itself tends to make a lot of accesses which we know are safe, but which would be unsafe outside of dlmalloc. For this reason, we provide a suppression file which ignores errors ocurring in dlmalloc.c
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Fix one branch of rEALLOc missing a VALGRING_*_BLOCK call - Add some additional suppressions for cALLOc and rEALLOc - Simplify calloc clearing logic
common/dlmalloc.c | 35 +++++++++++++++++++++++++++- common/malloc_simple.c | 10 ++++++++ include/malloc.h | 4 ++++ scripts/u-boot.supp | 53 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 scripts/u-boot.supp
diff --git a/common/dlmalloc.c b/common/dlmalloc.c index cf0270a9c1..cb311b5846 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -8,6 +8,7 @@
#include <malloc.h> #include <asm/io.h> +#include <valgrind/memcheck.h>
#ifdef DEBUG #if __STD_C @@ -1329,6 +1330,7 @@ Void_t* mALLOc(bytes) size_t bytes; unlink(victim, bck, fwd); set_inuse_bit_at_offset(victim, victim_size); check_malloced_chunk(victim, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim); }
@@ -1356,6 +1358,7 @@ Void_t* mALLOc(bytes) size_t bytes; unlink(victim, bck, fwd); set_inuse_bit_at_offset(victim, victim_size); check_malloced_chunk(victim, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim); } } @@ -1379,6 +1382,7 @@ Void_t* mALLOc(bytes) size_t bytes; set_head(remainder, remainder_size | PREV_INUSE); set_foot(remainder, remainder_size); check_malloced_chunk(victim, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim); }
@@ -1388,6 +1392,7 @@ Void_t* mALLOc(bytes) size_t bytes; { set_inuse_bit_at_offset(victim, victim_size); check_malloced_chunk(victim, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim); }
@@ -1443,6 +1448,7 @@ Void_t* mALLOc(bytes) size_t bytes; set_head(remainder, remainder_size | PREV_INUSE); set_foot(remainder, remainder_size); check_malloced_chunk(victim, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim); }
@@ -1451,6 +1457,7 @@ Void_t* mALLOc(bytes) size_t bytes; set_inuse_bit_at_offset(victim, victim_size); unlink(victim, bck, fwd); check_malloced_chunk(victim, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim); }
@@ -1499,6 +1506,7 @@ Void_t* mALLOc(bytes) size_t bytes; /* If big and would otherwise need to extend, try to use mmap instead */ if ((unsigned long)nb >= (unsigned long)mmap_threshold && (victim = mmap_chunk(nb))) + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim); #endif
@@ -1513,6 +1521,7 @@ Void_t* mALLOc(bytes) size_t bytes; top = chunk_at_offset(victim, nb); set_head(top, remainder_size | PREV_INUSE); check_malloced_chunk(victim, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(victim), bytes, SIZE_SZ, false); return chunk2mem(victim);
} @@ -1561,8 +1570,10 @@ void fREe(mem) Void_t* mem;
#if CONFIG_VAL(SYS_MALLOC_F_LEN) /* free() is a no-op - all the memory will be freed on relocation */ - if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) + if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { + VALGRIND_FREELIKE_BLOCK(mem, SIZE_SZ); return; + } #endif
if (mem == NULL) /* free(0) has no effect */ @@ -1584,6 +1595,7 @@ void fREe(mem) Void_t* mem; sz = hd & ~PREV_INUSE; next = chunk_at_offset(p, sz); nextsz = chunksize(next); + VALGRIND_FREELIKE_BLOCK(mem, SIZE_SZ);
if (next == top) /* merge with top */ { @@ -1772,6 +1784,8 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; top = chunk_at_offset(oldp, nb); set_head(top, (newsize - nb) | PREV_INUSE); set_head_size(oldp, nb); + VALGRIND_RESIZEINPLACE_BLOCK(chunk2mem(oldp), oldsize, bytes, + SIZE_SZ); return chunk2mem(oldp); } } @@ -1810,10 +1824,12 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; newp = prev; newsize += prevsize + nextsize; newmem = chunk2mem(newp); + VALGRIND_MALLOCLIKE_BLOCK(newmem, bytes, SIZE_SZ, false); MALLOC_COPY(newmem, oldmem, oldsize - SIZE_SZ); top = chunk_at_offset(newp, nb); set_head(top, (newsize - nb) | PREV_INUSE); set_head_size(newp, nb); + VALGRIND_FREELIKE_BLOCK(oldmem, SIZE_SZ); return newmem; } } @@ -1826,6 +1842,7 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; newp = prev; newsize += nextsize + prevsize; newmem = chunk2mem(newp); + VALGRIND_MALLOCLIKE_BLOCK(newmem, bytes, SIZE_SZ, false); MALLOC_COPY(newmem, oldmem, oldsize - SIZE_SZ); goto split; } @@ -1838,6 +1855,7 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; newp = prev; newsize += prevsize; newmem = chunk2mem(newp); + VALGRIND_MALLOCLIKE_BLOCK(newmem, bytes, SIZE_SZ, false); MALLOC_COPY(newmem, oldmem, oldsize - SIZE_SZ); goto split; } @@ -1864,6 +1882,14 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; MALLOC_COPY(newmem, oldmem, oldsize - SIZE_SZ); fREe(oldmem); return newmem; + } else { + /* + * This marks the whole allocation as undefined, but we don't keep around + * the original request, so we don't know what the actual change is + */ + VALGRIND_RESIZEINPLACE_BLOCK(oldmem, 0, bytes, SIZE_SZ); + /* so just make everything defined again */ + VALGRIND_MAKE_MEM_DEFINED(oldmem, bytes); }
@@ -1876,6 +1902,8 @@ Void_t* rEALLOc(oldmem, bytes) Void_t* oldmem; size_t bytes; set_head_size(newp, nb); set_head(remainder, remainder_size | PREV_INUSE); set_inuse_bit_at_offset(remainder, remainder_size); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(remainder), remainder_size, SIZE_SZ, + false); fREe(chunk2mem(remainder)); /* let free() deal with it */ } else @@ -2033,6 +2061,7 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; set_head_size(p, leadsize); fREe(chunk2mem(p)); p = newp; + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(p), bytes, SIZE_SZ, false);
assert (newsize >= nb && (((unsigned long)(chunk2mem(p))) % alignment) == 0); } @@ -2046,6 +2075,8 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; remainder = chunk_at_offset(p, nb); set_head(remainder, remainder_size | PREV_INUSE); set_head_size(p, nb); + VALGRIND_MALLOCLIKE_BLOCK(chunk2mem(remainder), remainder_size, SIZE_SZ, + false); fREe(chunk2mem(remainder)); }
@@ -2148,7 +2179,9 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; #endif #endif
+ /* Prevent valgrind complaining that we're zeroing unallocated space */ MALLOC_ZERO(mem, csz - SIZE_SZ); + VALGRIND_MAKE_MEM_DEFINED(mem, sz); return mem; } } diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 0267fb6bec..205e2c3c11 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -13,6 +13,7 @@ #include <mapmem.h> #include <asm/global_data.h> #include <asm/io.h> +#include <valgrind/valgrind.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -45,6 +46,7 @@ void *malloc_simple(size_t bytes) return ptr;
log_debug("%lx\n", (ulong)ptr); + VALGRIND_MALLOCLIKE_BLOCK(ptr, bytes, 0, false);
return ptr; } @@ -57,6 +59,7 @@ void *memalign_simple(size_t align, size_t bytes) if (!ptr) return ptr; log_debug("aligned to %lx\n", (ulong)ptr); + VALGRIND_MALLOCLIKE_BLOCK(ptr, bytes, 0, false);
return ptr; } @@ -74,6 +77,13 @@ void *calloc(size_t nmemb, size_t elem_size)
return ptr; } + +#if IS_ENABLED(CONFIG_VALGRIND) +void free_simple(void *ptr) +{ + VALGRIND_FREELIKE_BLOCK(ptr, 0); +} +#endif #endif
void malloc_simple_info(void) diff --git a/include/malloc.h b/include/malloc.h index 024b18be00..6f6087571b 100644 --- a/include/malloc.h +++ b/include/malloc.h @@ -886,7 +886,11 @@ void malloc_simple_info(void); #define malloc malloc_simple #define realloc realloc_simple #define memalign memalign_simple +#if IS_ENABLED(CONFIG_VALGRIND) +#define free free_simple +#else static inline void free(void *ptr) {} +#endif void *calloc(size_t nmemb, size_t size); void *realloc_simple(void *ptr, size_t size); #else diff --git a/scripts/u-boot.supp b/scripts/u-boot.supp new file mode 100644 index 0000000000..9562b27a61 --- /dev/null +++ b/scripts/u-boot.supp @@ -0,0 +1,53 @@ +{ + dlmalloc + Memcheck:Addr1 + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Addr4 + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Addr8 + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Addr1 + fun:* + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Addr4 + fun:* + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Addr8 + fun:* + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Value4 + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Value8 + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Cond + src:dlmalloc.c +} +{ + dlmalloc + Memcheck:Free + src:dlmalloc.c +}