Re: [PATCH v2 12/45] test: Support testing malloc() failures

Hi Sean,

On Wed, 28 Sept 2022 at 15:50, Sean Anderson seanga2@gmail.com wrote:

On 9/28/22 17:06, Simon Glass wrote:

...

Hi Sean,

On Wed, 28 Sept 2022 at 11:18, Sean Anderson seanga2@gmail.com 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 sjg@chromium.org

(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 seanga2@gmail.com

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