Skip to content

[scudo] Remove ResidentMemorySize test. #145955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cferris1000
Copy link
Contributor

This test is not very accurate, and nearly everything in Scudo would fail if this didn't work. It's now failing in Android, which is the reason for deleting.

Filed bug to replace with a more accurate version of this test.

This test is not very accurate, and nearly everything in Scudo would
fail if this didn't work. It's now failing in Android, which is the
reason for deleting.

Filed bug to replace with a more accurate version of this test.
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Christopher Ferris (cferris1000)

Changes

This test is not very accurate, and nearly everything in Scudo would fail if this didn't work. It's now failing in Android, which is the reason for deleting.

Filed bug to replace with a more accurate version of this test.


Full diff: https://github.com/llvm/llvm-project/pull/145955.diff

1 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/tests/common_test.cpp (-37)
diff --git a/compiler-rt/lib/scudo/standalone/tests/common_test.cpp b/compiler-rt/lib/scudo/standalone/tests/common_test.cpp
index e6ddbb00b843c..b63706694c3ac 100644
--- a/compiler-rt/lib/scudo/standalone/tests/common_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/common_test.cpp
@@ -16,43 +16,6 @@
 
 namespace scudo {
 
-static uptr getResidentMemorySize() {
-  if (!SCUDO_LINUX)
-    UNREACHABLE("Not implemented!");
-  uptr Size;
-  uptr Resident;
-  std::ifstream IFS("/proc/self/statm");
-  IFS >> Size;
-  IFS >> Resident;
-  return Resident * getPageSizeCached();
-}
-
-// Fuchsia needs getResidentMemorySize implementation.
-TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) {
-  uptr OnStart = getResidentMemorySize();
-  EXPECT_GT(OnStart, 0UL);
-
-  const uptr Size = 1ull << 30;
-  const uptr Threshold = Size >> 3;
-
-  MemMapT MemMap;
-  ASSERT_TRUE(MemMap.map(/*Addr=*/0U, Size, "ResidentMemorySize"));
-  ASSERT_NE(MemMap.getBase(), 0U);
-  void *P = reinterpret_cast<void *>(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);
-
-  MemMap.unmap();
-}
-
 TEST(ScudoCommonTest, Zeros) {
   const uptr Size = 1ull << 20;
 

@vitalybuka
Copy link
Collaborator

This is the only test which detect change like this, and we have qemu bug was broken in exactly like this. So I believe it's still useful.

--- a/compiler-rt/lib/scudo/standalone/mem_map_linux.cpp
+++ b/compiler-rt/lib/scudo/standalone/mem_map_linux.cpp
@@ -122,8 +122,9 @@ void MemMapLinux::setMemoryPermissionImpl(uptr Addr, uptr Size, uptr Flags) {
 void MemMapLinux::releaseAndZeroPagesToOSImpl(uptr From, uptr Size) {
   void *Addr = reinterpret_cast<void *>(From);
 
-  while (madvise(Addr, Size, MADV_DONTNEED) == -1 && errno == EAGAIN) {
-  }
+  memset((void*)(Addr), 0, Size);
+  // while (madvise(Addr, Size, MADV_DONTNEED) == -1 && errno == EAGAIN) {
+  // }
 }
 
 bool ReservedMemoryLinux::createImpl(uptr Addr, uptr Size, const char *Name,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants