Fix heap stats overhead_size calculation #13486
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ciarmcom
added
the
release-type: patch
rajkan01
changed the title
evedon
reviewed
Aug 24, 2020
| if (c->size < 0) { | ||
| c = (malloc_internal_overhead_t *)((char *)c + c->size); | ||
| } | ||
| return (c->size & ~0x1); |
There was a problem hiding this comment.
I am not sure why & ~0x1 is required.
There was a problem hiding this comment.
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 .
There was a problem hiding this comment.
I think that the LSB is used for usage flags so we have to mask it.
rajkan01
force-pushed
the
fix_malloc_overhead_calculation
branch
from
75aaa5b to
f10c97e
Compare
evedon
previously requested changes
Aug 25, 2020
rajkan01
force-pushed
the
fix_malloc_overhead_calculation
branch
from
f10c97e to
ec2ca2e
Compare
rajkan01
force-pushed
the
fix_malloc_overhead_calculation
branch
from
ec2ca2e to
f3c9132
Compare
|
@0xc0170 Could you review this PR and start the CI |
0xc0170
approved these changes
Sep 3, 2020
|
CI started |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
@0xc0170 Thanks for your review. This PR is ready to merge, please merge |
mbedmain
added
release-version: 6.3.0
Release-pending
and removed
release-type: patch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
overhead_sizeheap stats element was added to maintain real malloc internal overhead size but Mbed OS assumes that( malloc returned address - 4 )stores the actual allocated block size( including internal overhead size), but GCC_ARM newlib and newlib nano this location is used for padding when non-alignment memory allocation requested and it stores the negative offset.So updated Mbed OS allocated block size calculation like when reading from
( malloc returned address - 4 )is non-negative value consider to be an allocated block size. If it is a negative value ( offset ), then add this negative value into(malloc returned address - 4 )and read the allocated block size from that location.Fixes #13324
Impact of changes
With these changes, Mbed OS
overhead_sizereports proper malloc internal overhead size when using GCC_ARM with newlib-nanoMigration actions required
None.
Documentation
None.
Pull request type
Test results
Reviewers
@evedon