Fix the way malloc debug returns info.

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

diff --git a/libc/malloc_debug/README_api.md b/libc/malloc_debug/README_api.md
index 9d07cb3..b2c46d8 100644 (file)
--- a/libc/malloc_debug/README_api.md
+++ b/libc/malloc_debug/README_api.md
@@ -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.

diff --git a/libc/malloc_debug/TrackData.cpp b/libc/malloc_debug/TrackData.cpp
index 18f428b..d4064f8 100644 (file)
--- a/libc/malloc_debug/TrackData.cpp
+++ b/libc/malloc_debug/TrackData.cpp
@@ -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));
 

diff --git a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
index 1b08a39..219c21e 100644 (file)
--- a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
@@ -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;