Add better free tracking.
Included in this change:
- Change the tag when a pointer is freed so it's easy to detect if
an already freed pointer is being used.
- Move the free backtrace out of the header. This backtrace is only
used under only some circumstances, so no need to allocate space
in all headers for it.
- Add new option free_track_backtrace_num_frames to specify how many
frames to record when the free occurs. This removes the dependency
on the backtrace option to get backtraces.
Bug: 26739265
Change-Id: I76f5209507dcf46af67ada162a7cb2bf282116f2
diff --git a/libc/malloc_debug/tests/backtrace_fake.cpp b/libc/malloc_debug/tests/backtrace_fake.cpp
index 32da696..db542e5 100644
--- a/libc/malloc_debug/tests/backtrace_fake.cpp
+++ b/libc/malloc_debug/tests/backtrace_fake.cpp
@@ -52,7 +52,7 @@
return total_frames;
}
-void backtrace_log(uintptr_t* frames, size_t frame_count) {
+void backtrace_log(const uintptr_t* frames, size_t frame_count) {
for (size_t i = 0; i < frame_count; i++) {
error_log(" #%02zd pc %p", i, reinterpret_cast<void*>(frames[i]));
}
diff --git a/libc/malloc_debug/tests/malloc_debug_config_tests.cpp b/libc/malloc_debug/tests/malloc_debug_config_tests.cpp
index 247e319..551e498 100644
--- a/libc/malloc_debug/tests/malloc_debug_config_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_config_tests.cpp
@@ -99,9 +99,17 @@
"6 malloc_debug Instead, keep XX of these allocations around and then verify\n"
"6 malloc_debug that they have not been modified when the total number of freed\n"
"6 malloc_debug allocations exceeds the XX amount. When the program terminates,\n"
- "6 malloc_debug the rest of these allocations are verified.\n"
+ "6 malloc_debug the rest of these allocations are verified. When this option is\n"
+ "6 malloc_debug enabled, it automatically records the backtrace at the time of the free.\n"
"6 malloc_debug The default is to record 100 allocations.\n"
"6 malloc_debug \n"
+ "6 malloc_debug free_track_backtrace_num_frames[=XX]\n"
+ "6 malloc_debug This option only has meaning if free_track is set. This indicates\n"
+ "6 malloc_debug how many backtrace frames to capture when an allocation is freed.\n"
+ "6 malloc_debug If XX is set, that is the number of frames to capture. If XX\n"
+ "6 malloc_debug is set to zero, then no backtrace will be captured.\n"
+ "6 malloc_debug The default is to record 16 frames.\n"
+ "6 malloc_debug \n"
"6 malloc_debug leak_track\n"
"6 malloc_debug Enable the leak tracking of memory allocations.\n"
);
@@ -339,11 +347,13 @@
ASSERT_EQ(FREE_TRACK | FILL_ON_FREE, config->options);
ASSERT_EQ(1234U, config->free_track_allocations);
ASSERT_EQ(SIZE_MAX, config->fill_on_free_bytes);
+ ASSERT_EQ(16U, config->free_track_backtrace_num_frames);
ASSERT_TRUE(InitConfig("free_track"));
ASSERT_EQ(FREE_TRACK | FILL_ON_FREE, config->options);
ASSERT_EQ(100U, config->free_track_allocations);
ASSERT_EQ(SIZE_MAX, config->fill_on_free_bytes);
+ ASSERT_EQ(16U, config->free_track_backtrace_num_frames);
ASSERT_STREQ("", getFakeLogBuf().c_str());
ASSERT_STREQ("", getFakeLogPrint().c_str());
@@ -354,11 +364,50 @@
ASSERT_EQ(FREE_TRACK | FILL_ON_FREE, config->options);
ASSERT_EQ(1234U, config->free_track_allocations);
ASSERT_EQ(32U, config->fill_on_free_bytes);
+ ASSERT_EQ(16U, config->free_track_backtrace_num_frames);
ASSERT_TRUE(InitConfig("free_track fill_on_free=60"));
ASSERT_EQ(FREE_TRACK | FILL_ON_FREE, config->options);
ASSERT_EQ(100U, config->free_track_allocations);
ASSERT_EQ(60U, config->fill_on_free_bytes);
+ ASSERT_EQ(16U, config->free_track_backtrace_num_frames);
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ ASSERT_STREQ("", getFakeLogPrint().c_str());
+}
+
+TEST_F(MallocDebugConfigTest, free_track_backtrace_num_frames) {
+ ASSERT_TRUE(InitConfig("free_track_backtrace_num_frames=123"));
+
+ ASSERT_EQ(0U, config->options);
+ ASSERT_EQ(123U, config->free_track_backtrace_num_frames);
+
+ ASSERT_TRUE(InitConfig("free_track_backtrace_num_frames"));
+ ASSERT_EQ(0U, config->options);
+ ASSERT_EQ(16U, config->free_track_backtrace_num_frames);
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ ASSERT_STREQ("", getFakeLogPrint().c_str());
+}
+
+TEST_F(MallocDebugConfigTest, free_track_backtrace_num_frames_zero) {
+ ASSERT_TRUE(InitConfig("free_track_backtrace_num_frames=0"));
+
+ ASSERT_EQ(0U, config->options);
+ ASSERT_EQ(0U, config->free_track_backtrace_num_frames);
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ ASSERT_STREQ("", getFakeLogPrint().c_str());
+}
+
+TEST_F(MallocDebugConfigTest, free_track_backtrace_num_frames_and_free_track) {
+ ASSERT_TRUE(InitConfig("free_track free_track_backtrace_num_frames=123"));
+ ASSERT_EQ(FREE_TRACK | FILL_ON_FREE, config->options);
+ ASSERT_EQ(123U, config->free_track_backtrace_num_frames);
+
+ ASSERT_TRUE(InitConfig("free_track free_track_backtrace_num_frames"));
+ ASSERT_EQ(FREE_TRACK | FILL_ON_FREE, config->options);
+ ASSERT_EQ(16U, config->free_track_backtrace_num_frames);
ASSERT_STREQ("", getFakeLogBuf().c_str());
ASSERT_STREQ("", getFakeLogPrint().c_str());
@@ -550,3 +599,13 @@
"value must be <= 16384: 21000\n");
ASSERT_STREQ((log_msg + usage_string).c_str(), getFakeLogPrint().c_str());
}
+
+TEST_F(MallocDebugConfigTest, free_track_backtrace_num_frames_max_error) {
+ ASSERT_FALSE(InitConfig("free_track_backtrace_num_frames=400"));
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ std::string log_msg(
+ "6 malloc_debug malloc_testing: bad value for option 'free_track_backtrace_num_frames', "
+ "value must be <= 256: 400\n");
+ ASSERT_STREQ((log_msg + usage_string).c_str(), getFakeLogPrint().c_str());
+}
diff --git a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
index 08731c2..4b8aaeb 100644
--- a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
@@ -76,8 +76,7 @@
offset += BIONIC_ALIGN(sizeof(TrackHeader), sizeof(uintptr_t));
}
if (flags & BACKTRACE_HEADER) {
- offset += BIONIC_ALIGN(sizeof(BacktraceHeader) + sizeof(uintptr_t) * backtrace_frames - 1, sizeof(uintptr_t));
- offset += BIONIC_ALIGN(sizeof(BacktraceHeader) + sizeof(uintptr_t) * backtrace_frames - 1, sizeof(uintptr_t));
+ offset += BIONIC_ALIGN(sizeof(BacktraceHeader) + sizeof(uintptr_t) * backtrace_frames, sizeof(uintptr_t));
}
return offset;
}
@@ -209,7 +208,7 @@
}
TEST_F(MallocDebugTest, fill_on_free) {
- Init("fill_on_free free_track");
+ Init("fill_on_free free_track free_track_backtrace_num_frames=0");
uint8_t* pointer = reinterpret_cast<uint8_t*>(debug_malloc(100));
ASSERT_TRUE(pointer != nullptr);
@@ -226,7 +225,7 @@
}
TEST_F(MallocDebugTest, fill_on_free_partial) {
- Init("fill_on_free=30 free_track");
+ Init("fill_on_free=30 free_track free_track_backtrace_num_frames=0");
uint8_t* pointer = reinterpret_cast<uint8_t*>(debug_malloc(100));
ASSERT_TRUE(pointer != nullptr);
@@ -246,7 +245,7 @@
}
TEST_F(MallocDebugTest, free_track_partial) {
- Init("fill_on_free=30 free_track");
+ Init("fill_on_free=30 free_track free_track_backtrace_num_frames=0");
uint8_t* pointer = reinterpret_cast<uint8_t*>(debug_malloc(100));
ASSERT_TRUE(pointer != nullptr);
@@ -688,7 +687,7 @@
}
TEST_F(MallocDebugTest, free_track) {
- Init("free_track=5");
+ Init("free_track=5 free_track_backtrace_num_frames=0");
void* pointers[10];
for (size_t i = 0; i < sizeof(pointers) / sizeof(void*); i++) {
@@ -714,7 +713,7 @@
}
TEST_F(MallocDebugTest, free_track_use_after_free) {
- Init("free_track=5");
+ Init("free_track=5 free_track_backtrace_num_frames=0");
uint8_t* pointers[5];
for (size_t i = 0; i < sizeof(pointers) / sizeof(void*); i++) {
@@ -779,7 +778,7 @@
}
TEST_F(MallocDebugTest, free_track_use_after_free_finalize) {
- Init("free_track=100");
+ Init("free_track=100 free_track_backtrace_num_frames=0");
uint8_t* pointer = reinterpret_cast<uint8_t*>(debug_malloc(100));
ASSERT_TRUE(pointer != nullptr);
@@ -803,10 +802,8 @@
}
TEST_F(MallocDebugTest, free_track_use_after_free_with_backtrace) {
- Init("free_track=100 backtrace");
+ Init("free_track=100");
- // Alloc backtrace.
- backtrace_fake_add(std::vector<uintptr_t> {0xf0, 0xe, 0xd});
// Free backtrace.
backtrace_fake_add(std::vector<uintptr_t> {0xfa, 0xeb, 0xdc});
@@ -835,6 +832,72 @@
ASSERT_STREQ(expected_log.c_str(), getFakeLogPrint().c_str());
}
+TEST_F(MallocDebugTest, free_track_use_after_free_call_realloc) {
+ Init("free_track=100");
+
+ // Free backtrace.
+ backtrace_fake_add(std::vector<uintptr_t> {0xfa, 0xeb, 0xdc});
+ // Backtrace at realloc.
+ backtrace_fake_add(std::vector<uintptr_t> {0x12, 0x22, 0x32, 0x42});
+
+ void* pointer = debug_malloc(200);
+ ASSERT_TRUE(pointer != nullptr);
+ memset(pointer, 0, 200);
+ debug_free(pointer);
+
+ // Choose a size that should not trigger a realloc to verify tag is
+ // verified early.
+ ASSERT_TRUE(debug_realloc(pointer, 200) == nullptr);
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ std::string expected_log(DIVIDER);
+ expected_log += android::base::StringPrintf(
+ "6 malloc_debug +++ ALLOCATION %p USED AFTER FREE (realloc)\n", pointer);
+ expected_log += "6 malloc_debug Backtrace of original free:\n";
+ expected_log += "6 malloc_debug #00 pc 0xfa\n";
+ expected_log += "6 malloc_debug #01 pc 0xeb\n";
+ expected_log += "6 malloc_debug #02 pc 0xdc\n";
+ expected_log += "6 malloc_debug Backtrace at time of failure:\n";
+ expected_log += "6 malloc_debug #00 pc 0x12\n";
+ expected_log += "6 malloc_debug #01 pc 0x22\n";
+ expected_log += "6 malloc_debug #02 pc 0x32\n";
+ expected_log += "6 malloc_debug #03 pc 0x42\n";
+ expected_log += DIVIDER;
+ ASSERT_STREQ(expected_log.c_str(), getFakeLogPrint().c_str());
+}
+
+TEST_F(MallocDebugTest, free_track_use_after_free_call_free) {
+ Init("free_track=100");
+
+ // Free backtrace.
+ backtrace_fake_add(std::vector<uintptr_t> {0xfa, 0xeb, 0xdc});
+ // Backtrace at second free.
+ backtrace_fake_add(std::vector<uintptr_t> {0x12, 0x22, 0x32, 0x42});
+
+ void* pointer = debug_malloc(200);
+ ASSERT_TRUE(pointer != nullptr);
+ memset(pointer, 0, 200);
+ debug_free(pointer);
+
+ debug_free(pointer);
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ std::string expected_log(DIVIDER);
+ expected_log += android::base::StringPrintf(
+ "6 malloc_debug +++ ALLOCATION %p USED AFTER FREE (free)\n", pointer);
+ expected_log += "6 malloc_debug Backtrace of original free:\n";
+ expected_log += "6 malloc_debug #00 pc 0xfa\n";
+ expected_log += "6 malloc_debug #01 pc 0xeb\n";
+ expected_log += "6 malloc_debug #02 pc 0xdc\n";
+ expected_log += "6 malloc_debug Backtrace at time of failure:\n";
+ expected_log += "6 malloc_debug #00 pc 0x12\n";
+ expected_log += "6 malloc_debug #01 pc 0x22\n";
+ expected_log += "6 malloc_debug #02 pc 0x32\n";
+ expected_log += "6 malloc_debug #03 pc 0x42\n";
+ expected_log += DIVIDER;
+ ASSERT_STREQ(expected_log.c_str(), getFakeLogPrint().c_str());
+}
+
TEST_F(MallocDebugTest, get_malloc_leak_info_invalid) {
Init("fill");