[libc][mincore] use correct page_size for test #73984

SchrodingerZhu · 2023-11-30T20:41:36Z

No description provided.

llvmbot · 2023-11-30T21:30:15Z

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

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

2 Files Affected:

(modified) libc/test/src/sys/mman/linux/CMakeLists.txt (+1)
(modified) libc/test/src/sys/mman/linux/mincore_test.cpp (+29-25)

diff --git a/libc/test/src/sys/mman/linux/CMakeLists.txt b/libc/test/src/sys/mman/linux/CMakeLists.txt
index 5402ae030b345c6..c60377eb2cc1f73 100644
--- a/libc/test/src/sys/mman/linux/CMakeLists.txt
+++ b/libc/test/src/sys/mman/linux/CMakeLists.txt
@@ -76,5 +76,6 @@ add_libc_unittest(
     libc.src.sys.mman.munmap
     libc.src.sys.mman.madvise
     libc.src.sys.mman.mincore
+    libc.src.unistd.sysconf
     libc.test.UnitTest.ErrnoSetterMatcher
 )
diff --git a/libc/test/src/sys/mman/linux/mincore_test.cpp b/libc/test/src/sys/mman/linux/mincore_test.cpp
index 7c8ca3b4ffa4ccc..596e74aada1c89d 100644
--- a/libc/test/src/sys/mman/linux/mincore_test.cpp
+++ b/libc/test/src/sys/mman/linux/mincore_test.cpp
@@ -11,12 +11,13 @@
 #include "src/sys/mman/mincore.h"
 #include "src/sys/mman/mmap.h"
 #include "src/sys/mman/munmap.h"
+#include "src/unistd/sysconf.h"
 #include "test/UnitTest/ErrnoSetterMatcher.h"
 #include "test/UnitTest/LibcTest.h"
 #include "test/UnitTest/Test.h"
 
-#include <linux/param.h> // For EXEC_PAGESIZE
 #include <sys/mman.h>
+#include <unistd.h> // For sysconf.
 
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
 using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
@@ -29,66 +30,69 @@ TEST(LlvmLibcMincoreTest, UnMappedMemory) {
 }
 
 TEST(LlvmLibcMincoreTest, InvalidVec) {
-  void *addr = LIBC_NAMESPACE::mmap(nullptr, 4 * EXEC_PAGESIZE, PROT_READ,
+  size_t page_size = static_cast<size_t>(LIBC_NAMESPACE::sysconf(_SC_PAGESIZE));
+  void *addr = LIBC_NAMESPACE::mmap(nullptr, 4 * page_size, PROT_READ,
                                     MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   EXPECT_NE(addr, MAP_FAILED);
-  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % EXEC_PAGESIZE, 0ul);
+  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % page_size, 0ul);
   libc_errno = 0;
   int res = LIBC_NAMESPACE::mincore(addr, 1, nullptr);
   EXPECT_THAT(res, Fails(EFAULT, -1));
-  void *area =
-      LIBC_NAMESPACE::mmap(nullptr, EXEC_PAGESIZE, PROT_READ | PROT_WRITE,
-                           MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  void *area = LIBC_NAMESPACE::mmap(nullptr, page_size, PROT_READ | PROT_WRITE,
+                                    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   EXPECT_NE(area, MAP_FAILED);
-  unsigned char *ptr = static_cast<unsigned char *>(area) + EXEC_PAGESIZE - 3;
-  res = LIBC_NAMESPACE::mincore(addr, 4 * EXEC_PAGESIZE, ptr);
+  unsigned char *ptr = static_cast<unsigned char *>(area) + page_size - 3;
+  res = LIBC_NAMESPACE::mincore(addr, 4 * page_size, ptr);
   EXPECT_THAT(res, Fails(EFAULT, -1));
-  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, EXEC_PAGESIZE), Succeeds());
+  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, page_size), Succeeds());
   EXPECT_THAT(LIBC_NAMESPACE::munmap(area, 2), Succeeds());
 }
 
 TEST(LlvmLibcMincoreTest, UnalignedAddr) {
-  void *addr = LIBC_NAMESPACE::mmap(nullptr, EXEC_PAGESIZE, PROT_READ,
+  size_t page_size = static_cast<size_t>(LIBC_NAMESPACE::sysconf(_SC_PAGESIZE));
+  void *addr = LIBC_NAMESPACE::mmap(nullptr, page_size, PROT_READ,
                                     MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   EXPECT_NE(addr, MAP_FAILED);
-  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % EXEC_PAGESIZE, 0ul);
+  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % page_size, 0ul);
   libc_errno = 0;
   int res = LIBC_NAMESPACE::mincore(static_cast<char *>(addr) + 1, 1, nullptr);
   EXPECT_THAT(res, Fails(EINVAL, -1));
-  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, EXEC_PAGESIZE), Succeeds());
+  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, page_size), Succeeds());
 }
 
 TEST(LlvmLibcMincoreTest, NoError) {
-  void *addr = LIBC_NAMESPACE::mmap(nullptr, EXEC_PAGESIZE, PROT_READ,
+  size_t page_size = static_cast<size_t>(LIBC_NAMESPACE::sysconf(_SC_PAGESIZE));
+  void *addr = LIBC_NAMESPACE::mmap(nullptr, page_size, PROT_READ,
                                     MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   EXPECT_NE(addr, MAP_FAILED);
-  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % EXEC_PAGESIZE, 0ul);
+  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % page_size, 0ul);
   unsigned char vec;
   libc_errno = 0;
   int res = LIBC_NAMESPACE::mincore(addr, 1, &vec);
   EXPECT_THAT(res, Succeeds());
-  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, EXEC_PAGESIZE), Succeeds());
+  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, page_size), Succeeds());
 }
 
 TEST(LlvmLibcMincoreTest, NegativeLength) {
-  void *addr = LIBC_NAMESPACE::mmap(nullptr, EXEC_PAGESIZE, PROT_READ,
+  size_t page_size = static_cast<size_t>(LIBC_NAMESPACE::sysconf(_SC_PAGESIZE));
+  void *addr = LIBC_NAMESPACE::mmap(nullptr, page_size, PROT_READ,
                                     MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   EXPECT_NE(addr, MAP_FAILED);
-  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % EXEC_PAGESIZE, 0ul);
+  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % page_size, 0ul);
   unsigned char vec;
   libc_errno = 0;
   int res = LIBC_NAMESPACE::mincore(addr, -1, &vec);
   EXPECT_THAT(res, Fails(ENOMEM, -1));
-  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, EXEC_PAGESIZE), Succeeds());
+  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, page_size), Succeeds());
 }
 
 TEST(LlvmLibcMincoreTest, PageOut) {
   unsigned char vec;
-  void *addr =
-      LIBC_NAMESPACE::mmap(nullptr, EXEC_PAGESIZE, PROT_READ | PROT_WRITE,
-                           MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  size_t page_size = static_cast<size_t>(LIBC_NAMESPACE::sysconf(_SC_PAGESIZE));
+  void *addr = LIBC_NAMESPACE::mmap(nullptr, page_size, PROT_READ | PROT_WRITE,
+                                    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   EXPECT_NE(addr, MAP_FAILED);
-  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % EXEC_PAGESIZE, 0ul);
+  EXPECT_EQ(reinterpret_cast<unsigned long>(addr) % page_size, 0ul);
 
   // touch the page
   {
@@ -102,14 +106,14 @@ TEST(LlvmLibcMincoreTest, PageOut) {
   // page out the memory
   {
     libc_errno = 0;
-    EXPECT_THAT(LIBC_NAMESPACE::madvise(addr, EXEC_PAGESIZE, MADV_DONTNEED),
+    EXPECT_THAT(LIBC_NAMESPACE::madvise(addr, page_size, MADV_DONTNEED),
                 Succeeds());
 
     libc_errno = 0;
-    int res = LIBC_NAMESPACE::mincore(addr, EXEC_PAGESIZE, &vec);
+    int res = LIBC_NAMESPACE::mincore(addr, page_size, &vec);
     EXPECT_EQ(vec & 1u, 0u);
     EXPECT_THAT(res, Succeeds());
   }
 
-  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, EXEC_PAGESIZE), Succeeds());
+  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, page_size), Succeeds());
 }

lntue · 2023-11-30T22:44:58Z

libc/test/src/sys/mman/linux/mincore_test.cpp

                Succeeds());

    libc_errno = 0;
-    int res = LIBC_NAMESPACE::mincore(addr, EXEC_PAGESIZE, &vec);
+    int res = LIBC_NAMESPACE::mincore(addr, page_size, &vec);


Your suggestion of changing this to:

int res = LIBC_NAMESPACE::mincore(addr, 1, &vec);

does fix the segfault in AArch64. Otherwise, it is consistently crashed on AArch64.

SchrodingerZhu · 2023-12-01T01:17:32Z

libc/test/src/sys/mman/linux/mincore_test.cpp

  EXPECT_THAT(res, Fails(EFAULT, -1));
-  EXPECT_THAT(LIBC_NAMESPACE::munmap(addr, page_size), Succeeds());
-  EXPECT_THAT(LIBC_NAMESPACE::munmap(area, 2), Succeeds());


This is a bug that was not captured before.

lntue

The latest commits fixed the problems on the AArch64 machine that I tried on.

The test cases of mincore require getting correct page size from OS. As `sysconf` is not functioning correctly, these patches are implemented in a somewhat confusing way. We revert such patches and will reintroduce mincore after we correct sysconf. This reverts 54878b8, 985c0d1 and 418a3a4.

[libc][mincore] use correct page_size for test

3a4df5b

SchrodingerZhu marked this pull request as ready for review November 30, 2023 21:29

llvmbot added the libc label Nov 30, 2023

nickdesaulniers approved these changes Nov 30, 2023

View reviewed changes

lntue reviewed Nov 30, 2023

View reviewed changes

SchrodingerZhu force-pushed the libc/fix-page-size branch from 1078e53 to 0628a2e Compare December 1, 2023 00:36

workaround asan problem

2c5af52

SchrodingerZhu force-pushed the libc/fix-page-size branch from 0628a2e to 2c5af52 Compare December 1, 2023 00:37

remove extra cast

fffb9a4

SchrodingerZhu commented Dec 1, 2023

View reviewed changes

lntue self-requested a review December 1, 2023 01:32

lntue approved these changes Dec 1, 2023

View reviewed changes

lntue merged commit 985c0d1 into llvm:main Dec 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc][mincore] use correct page_size for test #73984

[libc][mincore] use correct page_size for test #73984

Uh oh!

SchrodingerZhu commented Nov 30, 2023

Uh oh!

llvmbot commented Nov 30, 2023

Uh oh!

lntue Nov 30, 2023

Uh oh!

SchrodingerZhu Dec 1, 2023

Uh oh!

lntue left a comment

Uh oh!

Uh oh!

[libc][mincore] use correct page_size for test #73984

[libc][mincore] use correct page_size for test #73984

Uh oh!

Conversation

SchrodingerZhu commented Nov 30, 2023

Uh oh!

llvmbot commented Nov 30, 2023

Uh oh!

lntue Nov 30, 2023

Choose a reason for hiding this comment

Uh oh!

SchrodingerZhu Dec 1, 2023

Choose a reason for hiding this comment

Uh oh!

lntue left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!