Hi Sean, On Wed, 28 Sept 2022 at 15:50, Sean Anderson <[email protected]> wrote: > > On 9/28/22 17:06, Simon Glass wrote: > > Hi Sean, > > > > On Wed, 28 Sept 2022 at 11:18, Sean Anderson <[email protected]> wrote: > >> > >> On 9/6/22 22:27, Simon Glass wrote: > >>> It is helpful to test that out-of-memory checks work correctly in code > >>> that calls malloc(). > >>> > >>> Add a simple way to force failure after a given number of malloc() calls. > >>> > >>> Fix a header guard to avoid a build error on sandbox_vpl. > >>> > >>> Signed-off-by: Simon Glass <[email protected]> > >>> --- > >>> > >>> (no changes since v1) > >>> > >>> arch/sandbox/include/asm/malloc.h | 1 + > >>> common/dlmalloc.c | 19 +++++++++++++++++++ > >>> include/malloc.h | 12 ++++++++++++ > >>> test/test-main.c | 1 + > >>> 4 files changed, 33 insertions(+) > >>> > >>> diff --git a/arch/sandbox/include/asm/malloc.h > >>> b/arch/sandbox/include/asm/malloc.h > >>> index a1467b5eadd..8aaaa9cb87b 100644 > >>> --- a/arch/sandbox/include/asm/malloc.h > >>> +++ b/arch/sandbox/include/asm/malloc.h > >>> @@ -6,6 +6,7 @@ > >>> */ > >>> > >>> #ifndef __ASM_MALLOC_H > >>> +#define __ASM_MALLOC_H > >>> > >>> void *malloc(size_t size); > >>> void free(void *ptr); > >>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c > >>> index f48cd2a333d..41c7230424c 100644 > >>> --- a/common/dlmalloc.c > >>> +++ b/common/dlmalloc.c > >>> @@ -596,6 +596,9 @@ ulong mem_malloc_start = 0; > >>> ulong mem_malloc_end = 0; > >>> ulong mem_malloc_brk = 0; > >>> > >>> +static bool malloc_testing; /* enable test mode */ > >>> +static int malloc_max_allocs; /* return NULL after this many > >>> calls to malloc() */ > >>> + > >>> void *sbrk(ptrdiff_t increment) > >>> { > >>> ulong old = mem_malloc_brk; > >>> @@ -1307,6 +1310,11 @@ Void_t* mALLOc(bytes) size_t bytes; > >>> return malloc_simple(bytes); > >>> #endif > >>> > >>> + if (CONFIG_IS_ENABLED(UNIT_TEST) && malloc_testing) { > >>> + if (--malloc_max_allocs < 0) > >>> + return NULL; > >>> + } > >>> + > >>> /* check if mem_malloc_init() was run */ > >>> if ((mem_malloc_start == 0) && (mem_malloc_end == 0)) { > >>> /* not initialized yet */ > >>> @@ -2470,6 +2478,17 @@ int initf_malloc(void) > >>> return 0; > >>> } > >>> > >>> +void malloc_enable_testing(int max_allocs) > >>> +{ > >>> + malloc_testing = true; > >>> + malloc_max_allocs = max_allocs; > >>> +} > >>> + > >>> +void malloc_disable_testing(void) > >>> +{ > >>> + malloc_testing = false; > >>> +} > >>> + > >>> /* > >>> > >>> History: > >>> diff --git a/include/malloc.h b/include/malloc.h > >>> index e8c8b254c0d..161ccbd1298 100644 > >>> --- a/include/malloc.h > >>> +++ b/include/malloc.h > >>> @@ -883,6 +883,18 @@ extern Void_t* sbrk(); > >>> > >>> void malloc_simple_info(void); > >>> > >>> +/** > >>> + * malloc_enable_testing() - Put malloc() into test mode > >>> + * > >>> + * This only works if UNIT_TESTING is enabled > >>> + * > >>> + * @max_allocs: return -ENOMEM after max_allocs calls to malloc() > >>> + */ > >>> +void malloc_enable_testing(int max_allocs); > >>> + > >>> +/** malloc_disable_testing() - Put malloc() into normal mode */ > >>> +void malloc_disable_testing(void); > >>> + > >>> #if CONFIG_IS_ENABLED(SYS_MALLOC_SIMPLE) > >>> #define malloc malloc_simple > >>> #define realloc realloc_simple > >>> diff --git a/test/test-main.c b/test/test-main.c > >>> index c65cbd7c351..5b6b5ba5bbe 100644 > >>> --- a/test/test-main.c > >>> +++ b/test/test-main.c > >>> @@ -46,6 +46,7 @@ static int dm_test_pre_run(struct unit_test_state *uts) > >>> uts->force_fail_alloc = false; > >>> uts->skip_post_probe = false; > >>> gd->dm_root = NULL; > >>> + malloc_disable_testing(); > >>> if (CONFIG_IS_ENABLED(UT_DM) && !CONFIG_IS_ENABLED(OF_PLATDATA)) > >>> memset(dm_testdrv_op_count, '\0', > >>> sizeof(dm_testdrv_op_count)); > >>> arch_reset_for_test(); > >> > >> Reviewed-by: Sean Anderson <[email protected]> > >> > >> but do we need to instrument rEALLOc too? > > > > We could, but I'd prefer to have a test which needs it first. Actually > > realloc() calls malloc() in some cases, so we are already messing with > > things here. > > Maybe it would be better to do this at the "top-level" instead. E.g. > redefine the malloc() et al macro. That way we remove any double-counting.
Yes, that makes sense. But again, I'd rather do this when we have a use case for making realloc() fail. BTW has dlmalloc been updated in recent years? Regards, Simon > > --Sean >
