On 5/4/21 11:26 AM, Simon Glass wrote:
Hi Sean,
On Sun, 2 May 2021 at 20:55, Sean Anderson <[email protected]> wrote:
This fixes memory being cleared after releasing it. Instead, clear memory
before releasing it. In addition, suppress valgrind warnings about writing
to free'd memory.
Signed-off-by: Sean Anderson <[email protected]>
---
common/dlmalloc.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/common/dlmalloc.c b/common/dlmalloc.c
index 05c8fd87e7..ea51bdf6a6 100644
--- a/common/dlmalloc.c
+++ b/common/dlmalloc.c
@@ -592,11 +592,13 @@ void *sbrk(ptrdiff_t increment)
ulong new = old + increment;
/*
- * if we are giving memory back make sure we clear it out since
- * we set MORECORE_CLEARS to 1
+ * if we are allocating memory make sure we clear it out since we set
+ * MORECORE_CLEARS to 1
*/
- if (increment < 0)
- memset((void *)new, 0, -increment);
+ if (increment > 0) {
+ VALGRIND_MAKE_MEM_UNDEFINED(old, increment);
+ memset((void *)old, 0, increment);
+ }
Can you explain this a bit more? What is the difference?
As it turns out, this patch is wrong. We need to clear memory when we
release it if SYS_MALLOC_CLEAR_ON_INIT is set, since calloc assumes that
memory has already been cleared if it gets it from sbrk.
Do you need the cast?
Yes (but this is moot)
common/dlmalloc.c: In function ‘sbrk’:
common/dlmalloc.c:600:10: warning: passing argument 1 of ‘memset’ makes pointer
from integer without a cast [-Wint-conversion]
600 | memset(old, 0, increment);
| ^~~
| |
| ulong {aka long unsigned int}
In file included from include/common.h:21,
from common/dlmalloc.c:1:
include/linux/string.h:111:22: note: expected ‘void *’ but argument is of type
‘ulong’ {aka ‘long unsigned int’}
111 | extern void * memset(void *,int,__kernel_size_t);
| ^~~~~~
--Sean
if ((new < mem_malloc_start) || (new > mem_malloc_end))
return (void *)MORECORE_FAILURE;
--
2.31.0
Regards,
Simon