From 9a63dddd279dd73c90ceb651045a48cbac8cfd86 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Wed, 9 Jul 2025 15:33:17 -0700 Subject: [PATCH 1/2] [scudo] Make release to OS test more specific. The original version of ResidentMemorySize could be a little flaky. Replace the test with a version that verifies exactly how much of the map is resident. --- .../scudo/standalone/tests/common_test.cpp | 142 +++++++++++++++--- 1 file changed, 117 insertions(+), 25 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/tests/common_test.cpp b/compiler-rt/lib/scudo/standalone/tests/common_test.cpp index e6ddbb00b843c..df692aca99621 100644 --- a/compiler-rt/lib/scudo/standalone/tests/common_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/common_test.cpp @@ -11,44 +11,136 @@ #include "common.h" #include "mem_map.h" + +#include +#include +#include +#include +#include +#include + #include -#include +#include namespace scudo { -static uptr getResidentMemorySize() { +static void GetRssKbFromString(uptr MapAddress, std::string &Buffer, + size_t &ParsedBytes, size_t &RssKb, + bool &Found) { + size_t LineStart = 0; + bool FindRss = false; + while (true) { + size_t LineEnd = Buffer.find('\n', LineStart); + if (LineEnd == std::string::npos) { + ParsedBytes = LineStart; + ASSERT_NE(0U, ParsedBytes) + << "The current buffer size (" << Buffer.size() + << ") is not large enough to contain a single line."; + break; + } + Buffer[LineEnd] = '\0'; + // The format of the address line is: + // 55ecba642000-55ecba644000 r--p 00000000 fd:01 66856632 + uptr StartAddr; + uptr EndAddr; + char Perms[5]; + if (sscanf(&Buffer[LineStart], "%" SCNxPTR "-%" SCNxPTR " %4s", &StartAddr, + &EndAddr, Perms) == 3) { + if (StartAddr == MapAddress) { + FindRss = true; + } + } else if (FindRss && strncmp(&Buffer[LineStart], "Rss:", 4) == 0) { + // The format of the RSS line is: + // Rss: 8 kB + ASSERT_EQ(1, sscanf(&Buffer[LineStart], "Rss: %zd kB", &RssKb)) + << "Bad Rss Line: " << &Buffer[LineStart]; + Found = true; + ParsedBytes = LineStart; + break; + } + LineStart = LineEnd + 1; + } +} + +static void GetRssKb(void *BaseAddress, size_t &RssKb) { if (!SCUDO_LINUX) UNREACHABLE("Not implemented!"); - uptr Size; - uptr Resident; - std::ifstream IFS("/proc/self/statm"); - IFS >> Size; - IFS >> Resident; - return Resident * getPageSizeCached(); + + size_t MapAddress = reinterpret_cast(BaseAddress); + + int Fd = open("/proc/self/smaps", O_RDONLY); + ASSERT_NE(-1, Fd) << "Failed to open /proc/self/smaps: " << strerror(errno); + + std::string Buffer(10 * 1024, '\0'); + size_t LeftoverBytes = 0; + RssKb = 0; + bool FoundMap = false; + while (LeftoverBytes != Buffer.size()) { + ssize_t ReadBytes = + read(Fd, &Buffer[LeftoverBytes], Buffer.size() - LeftoverBytes); + if (ReadBytes < 0) { + EXPECT_GT(0, ReadBytes) << "read failed: " << strerror(errno); + break; + } + if (ReadBytes == 0) { + // Nothing left to read. + break; + } + size_t End = static_cast(ReadBytes) + LeftoverBytes; + Buffer[End] = '\0'; + size_t ParsedBytes = 0; + GetRssKbFromString(MapAddress, Buffer, ParsedBytes, RssKb, FoundMap); + if (TEST_HAS_FAILURE || FoundMap) + break; + // Need to copy the leftover bytes back to the front of the buffer. + LeftoverBytes = End - ParsedBytes; + if (LeftoverBytes != 0) { + memmove(Buffer.data(), &Buffer[ParsedBytes], LeftoverBytes); + } + } + close(Fd); + + EXPECT_TRUE(FoundMap) << "Could not find map at address " << BaseAddress; } -// Fuchsia needs getResidentMemorySize implementation. +// Fuchsia needs getRssKb implementation. TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { - uptr OnStart = getResidentMemorySize(); - EXPECT_GT(OnStart, 0UL); - - const uptr Size = 1ull << 30; - const uptr Threshold = Size >> 3; + // Make sure to have the size of the map on a page boundary. + const uptr PageSize = getPageSizeCached(); + const uptr SizeBytes = 1000 * PageSize; + const uptr ActualSizeBytes = SizeBytes - 2 * PageSize; MemMapT MemMap; - ASSERT_TRUE(MemMap.map(/*Addr=*/0U, Size, "ResidentMemorySize")); + ASSERT_TRUE(MemMap.map(/*Addr=*/0U, SizeBytes, "ResidentMemorySize")); ASSERT_NE(MemMap.getBase(), 0U); - void *P = reinterpret_cast(MemMap.getBase()); - EXPECT_LT(getResidentMemorySize(), OnStart + Threshold); - memset(P, 1, Size); - EXPECT_GT(getResidentMemorySize(), OnStart + Size - Threshold); - - MemMap.releasePagesToOS(MemMap.getBase(), Size); - EXPECT_LT(getResidentMemorySize(), OnStart + Threshold); - - memset(P, 1, Size); - EXPECT_GT(getResidentMemorySize(), OnStart + Size - Threshold); + // Mark the first page and the last page as unreadable to make sure that + // the map shows up as distinct from all other maps. + EXPECT_EQ(0, mprotect(reinterpret_cast(MemMap.getBase()), PageSize, + PROT_NONE)); + EXPECT_EQ(0, mprotect(reinterpret_cast(MemMap.getBase() + SizeBytes - + PageSize), + PageSize, PROT_NONE)); + + size_t RssKb = 0; + void *P = reinterpret_cast(MemMap.getBase() + PageSize); + GetRssKb(P, RssKb); + EXPECT_EQ(RssKb, 0U); + + // Make the entire map resident. + memset(P, 1, ActualSizeBytes); + GetRssKb(P, RssKb); + EXPECT_EQ(RssKb, (ActualSizeBytes >> 10)); + + // Should release the memory to the kernel immediately. + MemMap.releasePagesToOS(MemMap.getBase(), SizeBytes); + GetRssKb(P, RssKb); + EXPECT_EQ(RssKb, 0U); + + // Make the entire map resident again. + memset(P, 1, ActualSizeBytes); + GetRssKb(P, RssKb); + EXPECT_EQ(RssKb, ActualSizeBytes >> 10); MemMap.unmap(); } From 106bdc566822004b967293c4d1dea15fecc04aad Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Tue, 15 Jul 2025 15:22:08 -0700 Subject: [PATCH 2/2] Switch implementation to mincore. --- .../scudo/standalone/tests/common_test.cpp | 128 ++++-------------- 1 file changed, 26 insertions(+), 102 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/tests/common_test.cpp b/compiler-rt/lib/scudo/standalone/tests/common_test.cpp index df692aca99621..71f810e9d9724 100644 --- a/compiler-rt/lib/scudo/standalone/tests/common_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/common_test.cpp @@ -13,134 +13,58 @@ #include "mem_map.h" #include -#include -#include #include #include -#include #include -#include +#include namespace scudo { -static void GetRssKbFromString(uptr MapAddress, std::string &Buffer, - size_t &ParsedBytes, size_t &RssKb, - bool &Found) { - size_t LineStart = 0; - bool FindRss = false; - while (true) { - size_t LineEnd = Buffer.find('\n', LineStart); - if (LineEnd == std::string::npos) { - ParsedBytes = LineStart; - ASSERT_NE(0U, ParsedBytes) - << "The current buffer size (" << Buffer.size() - << ") is not large enough to contain a single line."; - break; +static void getResidentPages(void *BaseAddress, size_t TotalPages, + size_t *ResidentPages) { + std::vector Pages(TotalPages, 0); + ASSERT_EQ( + 0, mincore(BaseAddress, TotalPages * getPageSizeCached(), Pages.data())) + << strerror(errno); + *ResidentPages = 0; + for (unsigned char Value : Pages) { + if (Value & 1) { + ++*ResidentPages; } - Buffer[LineEnd] = '\0'; - // The format of the address line is: - // 55ecba642000-55ecba644000 r--p 00000000 fd:01 66856632 - uptr StartAddr; - uptr EndAddr; - char Perms[5]; - if (sscanf(&Buffer[LineStart], "%" SCNxPTR "-%" SCNxPTR " %4s", &StartAddr, - &EndAddr, Perms) == 3) { - if (StartAddr == MapAddress) { - FindRss = true; - } - } else if (FindRss && strncmp(&Buffer[LineStart], "Rss:", 4) == 0) { - // The format of the RSS line is: - // Rss: 8 kB - ASSERT_EQ(1, sscanf(&Buffer[LineStart], "Rss: %zd kB", &RssKb)) - << "Bad Rss Line: " << &Buffer[LineStart]; - Found = true; - ParsedBytes = LineStart; - break; - } - LineStart = LineEnd + 1; } } -static void GetRssKb(void *BaseAddress, size_t &RssKb) { - if (!SCUDO_LINUX) - UNREACHABLE("Not implemented!"); - - size_t MapAddress = reinterpret_cast(BaseAddress); - - int Fd = open("/proc/self/smaps", O_RDONLY); - ASSERT_NE(-1, Fd) << "Failed to open /proc/self/smaps: " << strerror(errno); - - std::string Buffer(10 * 1024, '\0'); - size_t LeftoverBytes = 0; - RssKb = 0; - bool FoundMap = false; - while (LeftoverBytes != Buffer.size()) { - ssize_t ReadBytes = - read(Fd, &Buffer[LeftoverBytes], Buffer.size() - LeftoverBytes); - if (ReadBytes < 0) { - EXPECT_GT(0, ReadBytes) << "read failed: " << strerror(errno); - break; - } - if (ReadBytes == 0) { - // Nothing left to read. - break; - } - size_t End = static_cast(ReadBytes) + LeftoverBytes; - Buffer[End] = '\0'; - size_t ParsedBytes = 0; - GetRssKbFromString(MapAddress, Buffer, ParsedBytes, RssKb, FoundMap); - if (TEST_HAS_FAILURE || FoundMap) - break; - // Need to copy the leftover bytes back to the front of the buffer. - LeftoverBytes = End - ParsedBytes; - if (LeftoverBytes != 0) { - memmove(Buffer.data(), &Buffer[ParsedBytes], LeftoverBytes); - } - } - close(Fd); - - EXPECT_TRUE(FoundMap) << "Could not find map at address " << BaseAddress; -} - -// Fuchsia needs getRssKb implementation. +// Fuchsia needs getResidentPages implementation. TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { // Make sure to have the size of the map on a page boundary. const uptr PageSize = getPageSizeCached(); - const uptr SizeBytes = 1000 * PageSize; - const uptr ActualSizeBytes = SizeBytes - 2 * PageSize; + const size_t NumPages = 1000; + const uptr SizeBytes = NumPages * PageSize; MemMapT MemMap; ASSERT_TRUE(MemMap.map(/*Addr=*/0U, SizeBytes, "ResidentMemorySize")); ASSERT_NE(MemMap.getBase(), 0U); - // Mark the first page and the last page as unreadable to make sure that - // the map shows up as distinct from all other maps. - EXPECT_EQ(0, mprotect(reinterpret_cast(MemMap.getBase()), PageSize, - PROT_NONE)); - EXPECT_EQ(0, mprotect(reinterpret_cast(MemMap.getBase() + SizeBytes - - PageSize), - PageSize, PROT_NONE)); - - size_t RssKb = 0; - void *P = reinterpret_cast(MemMap.getBase() + PageSize); - GetRssKb(P, RssKb); - EXPECT_EQ(RssKb, 0U); + void *P = reinterpret_cast(MemMap.getBase()); + size_t ResidentPages; + getResidentPages(P, NumPages, &ResidentPages); + EXPECT_EQ(0U, ResidentPages); // Make the entire map resident. - memset(P, 1, ActualSizeBytes); - GetRssKb(P, RssKb); - EXPECT_EQ(RssKb, (ActualSizeBytes >> 10)); + memset(P, 1, SizeBytes); + getResidentPages(P, NumPages, &ResidentPages); + EXPECT_EQ(NumPages, ResidentPages); // Should release the memory to the kernel immediately. MemMap.releasePagesToOS(MemMap.getBase(), SizeBytes); - GetRssKb(P, RssKb); - EXPECT_EQ(RssKb, 0U); + getResidentPages(P, NumPages, &ResidentPages); + EXPECT_EQ(0U, ResidentPages); // Make the entire map resident again. - memset(P, 1, ActualSizeBytes); - GetRssKb(P, RssKb); - EXPECT_EQ(RssKb, ActualSizeBytes >> 10); + memset(P, 1, SizeBytes); + getResidentPages(P, NumPages, &ResidentPages); + EXPECT_EQ(NumPages, ResidentPages); MemMap.unmap(); }