tequilaOS/platform_bionic
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix the way malloc debug returns info.

Browse source 
When I rewrote malloc debug, I accidentally thought that each
value returned in the info buffer contained the number of backtrace
frames in the backtrace. This was incorrect, it should have been
the total number of allocations with the same backtrace.

This is a temporary fix that sets that value to 1. The better fix is
to de-dupe backtraces and then return all allocations of the same size
with the same backtrace.

I updated the documents to describe this.

Bug: 31854476

Test: Unit tests pass.
Change-Id: Idf9efaa3d363923b5d7543d90dc7c65a0ed553d9
This commit is contained in:
Christopher Ferris 2017-03-09 13:47:37 -08:00
parent b3b11edda1
commit 426b00a29b
3 changed files with 17 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
libc/malloc_debug/README_api.md
View file

@ -26,7 +26,7 @@ In order to free the buffer allocated by the function, call:
### Format of info Buffer
size_t size_of_original_allocation
size_t num_backtrace_frames
size_t num_allocations
uintptr_t pc1
uintptr_t pc2
uintptr_t pc3
@ -38,8 +38,12 @@ The number of *uintptr\_t* values is determined by the value
*backtrace\_size* as returned by the original call to
*get\_malloc\_leak\_info*. This value is not variable, it is the same
for all the returned data. The value
*num\_backtrace\_frames* contains the real number of frames found. The
extra frames are set to zero. Each *uintptr\_t* is a pc of the callstack.
*num\_allocations* contains the total number of allocations with the same
backtrace and size as this allocation. On Android Nougat, this value was
incorrectly set to the number of frames in the backtrace.
Each *uintptr\_t* is a pc of the callstack. If the total number
of backtrace entries is less than *backtrace\_size*, the rest of the
entries are zero.
The calls from within the malloc debug library are automatically removed.
For 32 bit systems, *size\_t* and *uintptr\_t* are both 4 byte values.

3
libc/malloc_debug/TrackData.cpp
View file

@ -123,11 +123,12 @@ void TrackData::GetInfo(uint8_t** info, size_t* overall_size, size_t* info_size,
GetList(&list);
uint8_t* data = *info;
size_t num_allocations = 1;
for (const auto& header : list) {
BacktraceHeader* back_header = debug_->GetAllocBacktrace(header);
if (back_header->num_frames > 0) {
memcpy(data, &header->size, sizeof(size_t));
memcpy(&data[sizeof(size_t)], &back_header->num_frames, sizeof(size_t));
memcpy(&data[sizeof(size_t)], &num_allocations, sizeof(size_t));
memcpy(&data[2 * sizeof(size_t)], &back_header->frames[0],
back_header->num_frames * sizeof(uintptr_t));

16
libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
View file

@ -999,7 +999,7 @@ TEST_F(MallocDebugTest, get_malloc_leak_info_not_enabled) {
struct InfoEntry {
size_t size;
size_t num_frames;
size_t num_allocations;
uintptr_t frames[0];
} __attribute__((packed));
@ -1033,7 +1033,7 @@ TEST_F(MallocDebugTest, get_malloc_leak_info_single) {
InfoEntry* entry = reinterpret_cast<InfoEntry*>(expected_info.data());
entry->size = 200;
entry->num_frames = 3;
entry->num_allocations = 1;
entry->frames[0] = 0xf;
entry->frames[1] = 0xe;
entry->frames[2] = 0xd;
@ -1082,7 +1082,7 @@ TEST_F(MallocDebugTest, get_malloc_leak_info_multi) {
// These values will be in the reverse order that we create.
entry2->size = 500;
entry2->num_frames = 4;
entry2->num_allocations = 1;
entry2->frames[0] = 0xf;
entry2->frames[1] = 0xe;
entry2->frames[2] = 0xd;
@ -1097,7 +1097,7 @@ TEST_F(MallocDebugTest, get_malloc_leak_info_multi) {
memset(pointers[0], 0, entry2->size);
entry1->size = 4100;
entry1->num_frames = 16;
entry1->num_allocations = 1;
for (size_t i = 0; i < 16; i++) {
entry1->frames[i] = 0xbc000 + i;
}
@ -1112,7 +1112,7 @@ TEST_F(MallocDebugTest, get_malloc_leak_info_multi) {
memset(pointers[1], 0, entry1->size);
entry0->size = 9000;
entry0->num_frames = 1;
entry0->num_allocations = 1;
entry0->frames[0] = 0x104;
backtrace_fake_add(std::vector<uintptr_t> {0x104});
@ -1159,7 +1159,7 @@ TEST_F(MallocDebugTest, get_malloc_leak_info_multi_skip_empty_backtrace) {
// These values will be in the reverse order that we create.
entry1->size = 500;
entry1->num_frames = 4;
entry1->num_allocations = 1;
entry1->frames[0] = 0xf;
entry1->frames[1] = 0xe;
entry1->frames[2] = 0xd;
@ -1174,7 +1174,7 @@ TEST_F(MallocDebugTest, get_malloc_leak_info_multi_skip_empty_backtrace) {
memset(pointers[0], 0, entry1->size);
entry0->size = 4100;
entry0->num_frames = 16;
entry0->num_allocations = 1;
for (size_t i = 0; i < 16; i++) {
entry0->frames[i] = 0xbc000 + i;
}
@ -1373,7 +1373,7 @@ static void VerifyZygoteSet(size_t memory_bytes) {
memset(expected_info.data(), 0, expected_info_size);
InfoEntry* entry = reinterpret_cast<InfoEntry*>(expected_info.data());
entry->size = memory_bytes | (1U << 31);
entry->num_frames = 1;
entry->num_allocations = 1;
entry->frames[0] = 0x1;
uint8_t* info;