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

[Sean Anderson]

On 9/28/22 22:36, Simon Glass wrote:
> Hi Sean,
> 
> On Wed, 28 Sept 2022 at 15:50, Sean Anderson <seanga2 at 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 at 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 at 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 at 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.

Fine by me.

> BTW has dlmalloc been updated in recent years?

Apparently there's a 2.8.6, last updated in 2012 [1].

--Sean

[1] https://gee.cs.oswego.edu/pub/misc/