diff options
author | Colin Cross <ccross@android.com> | 2018-05-14 14:55:39 -0700 |
---|---|---|
committer | Colin Cross <ccross@android.com> | 2018-05-14 15:24:28 -0700 |
commit | ca71f170b78e4707dbaa5bf4ddace47a455b4699 (patch) | |
tree | 5f8e73aca9d8d0b597af10320520334560fc98a5 /libmemunreachable | |
parent | 86dade8f6fcf3b0e72667b78341d8a74a1bb9162 (diff) | |
download | system_core-ca71f170b78e4707dbaa5bf4ddace47a455b4699.tar.gz system_core-ca71f170b78e4707dbaa5bf4ddace47a455b4699.tar.bz2 system_core-ca71f170b78e4707dbaa5bf4ddace47a455b4699.zip |
Make memunreachable_test more robust against false negative leaks
For some reason, the memunreachable tests are rock solid on the
devices covered by APCT, but catch a ton of false-negatives on
hikey960, which show up as failures that look like:
system/core/libmemunreachable/tests/MemUnreachable_test.cpp:200: Failure
Expected equality of these values:
1U
Which is: 1
info.leaks.size()
Which is: 0
These happen when a stray copy of a pointer is lying around that
points to the memory it is expected to leak. The stray pointers
can be on the stack or in the jemalloc thread cache of freed
allocations, which is always considered active memory.
Add some extra cleanups to get rid of old pointers.
1. Clear the tcache when destructing UnreachableMemoryInfo
2. Clear the stack and tcache before and after each test
3. Make MemunreachbleTest.twice match MemunreachableTest.stack
Also fix MemunreachableTest.notdumpable, which was only passing
when run as root, which was bypassing what the test was trying
to cover. Make the test pass when run as non-root, and skip
when the test is running as root.
Bug: 79701104
Test: memunreachable_test
Test: memunreachable_test as root
Change-Id: Ia6c6df11e76405d08118afcc19c1fe80a6684c56
Diffstat (limited to 'libmemunreachable')
-rw-r--r-- | libmemunreachable/MemUnreachable.cpp | 15 | ||||
-rw-r--r-- | libmemunreachable/include/memunreachable/memunreachable.h | 7 | ||||
-rw-r--r-- | libmemunreachable/tests/MemUnreachable_test.cpp | 66 |
3 files changed, 73 insertions, 15 deletions
diff --git a/libmemunreachable/MemUnreachable.cpp b/libmemunreachable/MemUnreachable.cpp index 24fdc7f37..529a0437e 100644 --- a/libmemunreachable/MemUnreachable.cpp +++ b/libmemunreachable/MemUnreachable.cpp @@ -495,6 +495,21 @@ std::string UnreachableMemoryInfo::ToString(bool log_contents) const { return oss.str(); } +UnreachableMemoryInfo::~UnreachableMemoryInfo() { + // Clear the memory that holds the leaks, otherwise the next attempt to + // detect leaks may find the old data (for example in the jemalloc tcache) + // and consider all the leaks to be referenced. + memset(leaks.data(), 0, leaks.capacity() * sizeof(Leak)); + + std::vector<Leak> tmp; + leaks.swap(tmp); + + // Disable and re-enable malloc to flush the jemalloc tcache to make sure + // there are no copies of the leaked pointer addresses there. + malloc_disable(); + malloc_enable(); +} + std::string GetUnreachableMemoryString(bool log_contents, size_t limit) { UnreachableMemoryInfo info; if (!GetUnreachableMemory(info, limit)) { diff --git a/libmemunreachable/include/memunreachable/memunreachable.h b/libmemunreachable/include/memunreachable/memunreachable.h index 438fcafe1..c028eabf2 100644 --- a/libmemunreachable/include/memunreachable/memunreachable.h +++ b/libmemunreachable/include/memunreachable/memunreachable.h @@ -62,12 +62,7 @@ struct UnreachableMemoryInfo { size_t allocation_bytes; UnreachableMemoryInfo() {} - ~UnreachableMemoryInfo() { - // Clear the memory that holds the leaks, otherwise the next attempt to - // detect leaks may find the old data (for example in the jemalloc tcache) - // and consider all the leaks to be referenced. - memset(leaks.data(), 0, leaks.capacity() * sizeof(Leak)); - } + ~UnreachableMemoryInfo(); std::string ToString(bool log_contents) const; }; diff --git a/libmemunreachable/tests/MemUnreachable_test.cpp b/libmemunreachable/tests/MemUnreachable_test.cpp index 87417f132..bba0c6d11 100644 --- a/libmemunreachable/tests/MemUnreachable_test.cpp +++ b/libmemunreachable/tests/MemUnreachable_test.cpp @@ -23,6 +23,8 @@ #include <memunreachable/memunreachable.h> +#include "bionic.h" + namespace android { class HiddenPointer { @@ -48,7 +50,35 @@ static void Ref(void** ptr) { write(0, ptr, 0); } -TEST(MemunreachableTest, clean) { +class MemunreachableTest : public ::testing::Test { + protected: + virtual void SetUp() { + CleanStack(8192); + CleanTcache(); + } + + virtual void TearDown() { + CleanStack(8192); + CleanTcache(); + } + + // Allocate a buffer on the stack and zero it to make sure there are no + // stray pointers from old test runs. + void __attribute__((noinline)) CleanStack(size_t size) { + void* buf = alloca(size); + memset(buf, 0, size); + Ref(&buf); + } + + // Disable and re-enable malloc to flush the jemalloc tcache to make sure + // there are stray pointers from old test runs there. + void CleanTcache() { + malloc_disable(); + malloc_enable(); + } +}; + +TEST_F(MemunreachableTest, clean) { UnreachableMemoryInfo info; ASSERT_TRUE(LogUnreachableMemory(true, 100)); @@ -57,7 +87,7 @@ TEST(MemunreachableTest, clean) { ASSERT_EQ(0U, info.leaks.size()); } -TEST(MemunreachableTest, stack) { +TEST_F(MemunreachableTest, stack) { HiddenPointer hidden_ptr; { @@ -91,7 +121,7 @@ TEST(MemunreachableTest, stack) { void* g_ptr; -TEST(MemunreachableTest, global) { +TEST_F(MemunreachableTest, global) { HiddenPointer hidden_ptr; g_ptr = hidden_ptr.Get(); @@ -122,7 +152,7 @@ TEST(MemunreachableTest, global) { } } -TEST(MemunreachableTest, tls) { +TEST_F(MemunreachableTest, tls) { HiddenPointer hidden_ptr; pthread_key_t key; pthread_key_create(&key, nullptr); @@ -157,10 +187,22 @@ TEST(MemunreachableTest, tls) { pthread_key_delete(key); } -TEST(MemunreachableTest, twice) { +TEST_F(MemunreachableTest, twice) { HiddenPointer hidden_ptr; { + void* ptr = hidden_ptr.Get(); + Ref(&ptr); + + UnreachableMemoryInfo info; + + ASSERT_TRUE(GetUnreachableMemory(info)); + ASSERT_EQ(0U, info.leaks.size()); + + ptr = nullptr; + } + + { UnreachableMemoryInfo info; ASSERT_TRUE(GetUnreachableMemory(info)); @@ -184,7 +226,7 @@ TEST(MemunreachableTest, twice) { } } -TEST(MemunreachableTest, log) { +TEST_F(MemunreachableTest, log) { HiddenPointer hidden_ptr; ASSERT_TRUE(LogUnreachableMemory(true, 100)); @@ -199,17 +241,23 @@ TEST(MemunreachableTest, log) { } } -TEST(MemunreachableTest, notdumpable) { +TEST_F(MemunreachableTest, notdumpable) { + if (getuid() == 0) { + // TODO(ccross): make this a skipped test when gtest supports them + printf("[ SKIP ] Not testable when running as root\n"); + return; + } + ASSERT_EQ(0, prctl(PR_SET_DUMPABLE, 0)); HiddenPointer hidden_ptr; - ASSERT_TRUE(LogUnreachableMemory(true, 100)); + EXPECT_FALSE(LogUnreachableMemory(true, 100)); ASSERT_EQ(0, prctl(PR_SET_DUMPABLE, 1)); } -TEST(MemunreachableTest, leak_lots) { +TEST_F(MemunreachableTest, leak_lots) { std::vector<HiddenPointer> hidden_ptrs; hidden_ptrs.resize(1024); |