[compiler-rt] [sanitizer] Add TryMemCpy (PR #112668)

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 16:59:11 PDT 2024


https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/112668

>From 23159ed20fa10bef1b63efb08d86861abf9362b2 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Wed, 16 Oct 2024 23:43:25 -0700
Subject: [PATCH 1/7] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20ch?=
 =?UTF-8?q?anges=20to=20main=20this=20commit=20is=20based=20on?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4

[skip ci]
---
 .../sanitizer_posix_libcdep.cpp               | 51 ++++++++++++-------
 .../tests/sanitizer_posix_test.cpp            | 11 ++++
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
index 9ffb36f812c45d..f87af4bb3a5a6a 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
@@ -288,26 +288,41 @@ bool SignalContext::IsStackOverflow() const {
 
 #endif  // SANITIZER_GO
 
+static void SetNonBlock(int fd) {
+  int res = fcntl(fd, F_GETFL, 0);
+  CHECK(!internal_iserror(res, nullptr));
+
+  res |= O_NONBLOCK;
+  res = fcntl(fd, F_SETFL, res);
+  CHECK(!internal_iserror(res, nullptr));
+}
+
 bool IsAccessibleMemoryRange(uptr beg, uptr size) {
-  uptr page_size = GetPageSizeCached();
-  // Checking too large memory ranges is slow.
-  CHECK_LT(size, page_size * 10);
-  int sock_pair[2];
-  if (pipe(sock_pair))
-    return false;
-  uptr bytes_written =
-      internal_write(sock_pair[1], reinterpret_cast<void *>(beg), size);
-  int write_errno;
-  bool result;
-  if (internal_iserror(bytes_written, &write_errno)) {
-    CHECK_EQ(EFAULT, write_errno);
-    result = false;
-  } else {
-    result = (bytes_written == size);
+  while (size) {
+    // `read` from `sock_pair[0]` into a dummy buffer to free up the pipe buffer
+    // for more `write` is slower than just recreating a pipe.
+    int sock_pair[2];
+    if (pipe(sock_pair))
+      return false;
+
+    auto cleanup = at_scope_exit([&]() {
+      internal_close(sock_pair[0]);
+      internal_close(sock_pair[1]);
+    });
+
+    SetNonBlock(sock_pair[1]);
+
+    int write_errno;
+    uptr w = internal_write(sock_pair[1], reinterpret_cast<char *>(beg), size);
+    if (internal_iserror(w, &write_errno)) {
+      CHECK_EQ(EFAULT, write_errno);
+      return false;
+    }
+    size -= w;
+    beg += w;
   }
-  internal_close(sock_pair[0]);
-  internal_close(sock_pair[1]);
-  return result;
+
+  return true;
 }
 
 void PlatformPrepareForSandboxing(void *args) {
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
index bed19d15a8ec77..9feb22221f005e 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
@@ -82,6 +82,17 @@ TEST(SanitizerCommon, IsAccessibleMemoryRange) {
   munmap((void *)mem, 3 * page_size);
 }
 
+TEST(SanitizerCommon, IsAccessibleMemoryRangeLarge) {
+  const int size = GetPageSize() * 10000;
+
+  uptr mem = (uptr)mmap(0, size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON,
+                        -1, 0);
+
+  EXPECT_TRUE(IsAccessibleMemoryRange(mem, size));
+
+  munmap((void *)mem, size);
+}
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_POSIX

>From 9571c266d478f2cb49a8005b0d19f40b3de72052 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 17 Oct 2024 15:47:37 -0700
Subject: [PATCH 2/7] rebase

Created using spr 1.3.4
---
 compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp    | 2 ++
 compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp | 1 +
 2 files changed, 3 insertions(+)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
index 6459c64985faab..e1117c1bec33fa 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
@@ -327,6 +327,8 @@ bool IsAccessibleMemoryRange(uptr beg, uptr size) {
 }
 
 bool TryMemCpy(void *dest, const void *src, uptr n) {
+  if (!n)
+    return true;
   int fds[2];
   CHECK_EQ(0, pipe(fds));
 
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
index 9e463796b2c678..03a841a6f438cb 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
@@ -16,6 +16,7 @@
 #  include <pthread.h>
 #  include <sys/mman.h>
 
+#  include <algorithm>
 #  include <numeric>
 
 #  include "gtest/gtest.h"

>From 2f85748304589ba78560fa8bedae42863c14c343 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 17 Oct 2024 15:51:24 -0700
Subject: [PATCH 3/7] move test into a different pr

Created using spr 1.3.4
---
 .../tests/sanitizer_posix_test.cpp                | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
index 03a841a6f438cb..658ca60175b3bd 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_posix_test.cpp
@@ -127,21 +127,6 @@ TEST(SanitizerCommon, TryMemCpyNull) {
   EXPECT_FALSE(TryMemCpy(dst.data(), nullptr, dst.size()));
 }
 
-TEST(SanitizerCommon, TryMemCpyProtected) {
-  const int page_size = GetPageSize();
-  InternalMmapVector<char> src(3 * page_size);
-  std::iota(src.begin(), src.end(), 123);
-  std::vector<char> dst;
-  // Protect the middle page.
-  mprotect(src.data() + page_size, page_size, PROT_NONE);
-
-  dst.assign(src.size(), 0);
-  EXPECT_FALSE(TryMemCpy(dst.data(), src.data(), dst.size()));
-
-  mprotect(src.data() + page_size, page_size, PROT_READ | PROT_WRITE);
-  EXPECT_TRUE(std::equal(dst.begin(), dst.end(), src.begin()));
-}
-
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_POSIX

>From 5337fadaa02592ed1e1fcdd9205ff0496c07a044 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 17 Oct 2024 16:20:05 -0700
Subject: [PATCH 4/7] EINTR

Created using spr 1.3.4
---
 compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
index e1117c1bec33fa..f113b8ec58a15d 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
@@ -357,7 +357,10 @@ bool TryMemCpy(void *dest, const void *src, uptr n) {
 
     while (w) {
       uptr r = internal_read(fds[0], d, w);
-      CHECK(!internal_iserror(r, &e));
+      if(internal_iserror(r, &e)) {
+        CHECK_EQ(EINTR, e);
+        continue;
+      }
 
       d += r;
       w -= r;

>From 4e67e629322695226c4d7d0d2ee5d92effd99e53 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 17 Oct 2024 16:22:04 -0700
Subject: [PATCH 5/7] format

Created using spr 1.3.4
---
 compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
index f113b8ec58a15d..7ee2319456d23e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
@@ -357,7 +357,7 @@ bool TryMemCpy(void *dest, const void *src, uptr n) {
 
     while (w) {
       uptr r = internal_read(fds[0], d, w);
-      if(internal_iserror(r, &e)) {
+      if (internal_iserror(r, &e)) {
         CHECK_EQ(EINTR, e);
         continue;
       }

>From 44a74388ff1fbdc301e7101927b191429976e3d0 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 17 Oct 2024 16:55:14 -0700
Subject: [PATCH 6/7] Update comment

---
 compiler-rt/lib/sanitizer_common/sanitizer_common.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index 9b1e58f5e7a61d..785c2034cd884d 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -270,8 +270,9 @@ extern uptr stoptheworld_tracer_ppid;
 
 // Returns true if we can read a memory range.
 bool IsAccessibleMemoryRange(uptr beg, uptr size);
-// Returns true if we can read a memory range starting at `src`, and copies
-// content into `dest`.
+// Attempts to copy `n` bytes from memory range starting at `src` to `dest`.
+// Returns true if the entire range can be read and copied, false otherwise.
+// If the copy fails, the contents of `dest` are undefined.
 bool TryMemCpy(void *dest, const void *src, uptr n);
 
 // Error report formatting.

>From cb4cb67045ab848fd54046141ab47e8650f7571f Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at google.com>
Date: Thu, 17 Oct 2024 16:59:01 -0700
Subject: [PATCH 7/7] Update comment

---
 compiler-rt/lib/sanitizer_common/sanitizer_common.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index 785c2034cd884d..3a28420ed02d78 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -268,11 +268,12 @@ class ScopedErrorReportLock {
 extern uptr stoptheworld_tracer_pid;
 extern uptr stoptheworld_tracer_ppid;
 
-// Returns true if we can read a memory range.
+// Returns true if the entire range can be read.
 bool IsAccessibleMemoryRange(uptr beg, uptr size);
 // Attempts to copy `n` bytes from memory range starting at `src` to `dest`.
-// Returns true if the entire range can be read and copied, false otherwise.
-// If the copy fails, the contents of `dest` are undefined.
+// Returns true if the entire range can be read. Returns `false` if any part of
+// the source range cannot be read, in which case the contents of `dest` are
+// undefined.
 bool TryMemCpy(void *dest, const void *src, uptr n);
 
 // Error report formatting.



More information about the llvm-commits mailing list