Skip to content

Fix heap stats overhead_size calculation #13486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 3, 2020
Merged

Fix heap stats overhead_size calculation #13486

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d30ba6b
Fix malloc internal overhead size calculation for newlib and newlib-nano
rajkan01 Aug 24, 2020
f3c9132
Incorporated the review comments
rajkan01 Aug 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions platform/source/mbed_alloc_wrappers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,22 @@ typedef struct {
static SingletonPtr<PlatformMutex> malloc_stats_mutex;
static mbed_stats_heap_t heap_stats = {0, 0, 0, 0, 0, 0, 0};

typedef struct {
size_t size;
typedef struct mbed_heap_overhead {
int size; // Size of the allocated memory block, including internal overhead size
struct mbed_heap_overhead *next; // The memory is either the next free block, or allocated memory block
} mbed_heap_overhead_t;

#define MALLOC_HEADER_SIZE (sizeof(mbed_heap_overhead_t))
#define MALLOC_HEADER_PTR(p) (mbed_heap_overhead_t *)((char *)(p) - MALLOC_HEADER_SIZE)
#define MALLOC_HEAP_TOTAL_SIZE(p) (((p)->size) & (~0x1))
static int get_malloc_block_total_size(void *ptr)
{
mbed_heap_overhead_t *c = (mbed_heap_overhead_t *)((char *)ptr - offsetof(mbed_heap_overhead, next));

// Skip the padding area
if (c->size < 0) {
c = (mbed_heap_overhead_t *)((char *)c + c->size);
}
// Mask LSB as it is used for usage flags
return (c->size & ~0x1);
Copy link
Contributor

@evedon evedon Aug 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why & ~0x1 is required.

Copy link
Contributor Author

@rajkan01 rajkan01 Aug 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using the newlib, overhead size (total allocated block size + size of overhead) value always size + 1 (Bug in newlib malloc implementation) so added a workaround size & ~0x1 logic as memory allocation is always aligned with 8 bytes block. Even this logic was there on existing code https://github.com/ARMmbed/mbed-os/blob/master/platform/source/mbed_alloc_wrappers.cpp#L61 .

Copy link
Contributor

@evedon evedon Aug 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the LSB is used for usage flags so we have to mask it.

}
#endif

void mbed_stats_heap_get(mbed_stats_heap_t *stats)
Expand Down Expand Up @@ -116,7 +125,7 @@ extern "C" void *malloc_wrapper(struct _reent *r, size_t size, void *caller)
if (heap_stats.current_size > heap_stats.max_size) {
heap_stats.max_size = heap_stats.current_size;
}
heap_stats.overhead_size += MALLOC_HEAP_TOTAL_SIZE(MALLOC_HEADER_PTR(alloc_info)) - size;
heap_stats.overhead_size += get_malloc_block_total_size((void *)alloc_info) - size;
} else {
heap_stats.alloc_fail_cnt += 1;
}
Expand Down Expand Up @@ -191,7 +200,7 @@ extern "C" void free_wrapper(struct _reent *r, void *ptr, void *caller)
alloc_info = ((alloc_info_t *)ptr) - 1;
if (MBED_HEAP_STATS_SIGNATURE == alloc_info->signature) {
size_t user_size = alloc_info->size;
size_t alloc_size = MALLOC_HEAP_TOTAL_SIZE(MALLOC_HEADER_PTR(alloc_info));
size_t alloc_size = get_malloc_block_total_size((void *)alloc_info);
alloc_info->signature = 0x0;
heap_stats.current_size -= user_size;
heap_stats.alloc_cnt -= 1;
Expand Down Expand Up @@ -303,7 +312,7 @@ extern "C" void *malloc_wrapper(size_t size, void *caller)
if (heap_stats.current_size > heap_stats.max_size) {
heap_stats.max_size = heap_stats.current_size;
}
heap_stats.overhead_size += MALLOC_HEAP_TOTAL_SIZE(MALLOC_HEADER_PTR(alloc_info)) - size;
heap_stats.overhead_size += get_malloc_block_total_size((void *)alloc_info) - size;
} else {
heap_stats.alloc_fail_cnt += 1;
}
Expand Down Expand Up @@ -416,7 +425,7 @@ extern "C" void free_wrapper(void *ptr, void *caller)
alloc_info = ((alloc_info_t *)ptr) - 1;
if (MBED_HEAP_STATS_SIGNATURE == alloc_info->signature) {
size_t user_size = alloc_info->size;
size_t alloc_size = MALLOC_HEAP_TOTAL_SIZE(MALLOC_HEADER_PTR(alloc_info));
size_t alloc_size = get_malloc_block_total_size((void *)alloc_info);
alloc_info->signature = 0x0;
heap_stats.current_size -= user_size;
heap_stats.alloc_cnt -= 1;
Expand Down