[compiler-rt][memprof] memccpy interception

[devnexen]

No description provided.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- compiler-rt/test/memprof/TestCases/memprof_memccpy.cpp compiler-rt/lib/memprof/memprof_interceptors.cpp compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp compiler-rt/lib/sanitizer_common/sanitizer_libc.h compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

View the diff from clang-format here.

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp
index d941e4379..ff5d32ca3 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp
@@ -55,7 +55,8 @@ void *internal_memccpy(void *dest, const void *src, int c, uptr n) {
   uptr i = 0;
   for (; i < n; ++i) {
     d[i] = s[i];
-    if (s[i] == c) break;
+    if (s[i] == c)
+      break;
   }
   if (n > 0 && i < n)
     return d + i + 1;

[llvmbot]

@llvm/pr-subscribers-pgo

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

Author: David CARLIER (devnexen)

Changes

---

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

5 Files Affected:

• (modified) compiler-rt/lib/memprof/memprof_interceptors.cpp (+20)
• (modified) compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp (+10)
• (modified) compiler-rt/lib/sanitizer_common/sanitizer_libc.h (+1)
• (modified) compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h (+1)
• (added) compiler-rt/test/memprof/TestCases/memprof_memccpy.cpp (+17)

diff --git a/compiler-rt/lib/memprof/memprof_interceptors.cpp b/compiler-rt/lib/memprof/memprof_interceptors.cpp
index f4d7fd46e6198..e9ac39a3cf133 100644
--- a/compiler-rt/lib/memprof/memprof_interceptors.cpp
+++ b/compiler-rt/lib/memprof/memprof_interceptors.cpp
@@ -306,6 +306,22 @@ INTERCEPTOR(long long, atoll, const char *nptr) {
   return result;
 }
 
+#if SANITIZER_INTERCEPT_MEMCCPY
+INTERCEPTOR(void *, memccpy, void *dest, const void *src, int c, usize size) {
+  void *ctx;
+  MEMPROF_INTERCEPTOR_ENTER(ctx, strdup);
+  if (UNLIKELY(!memprof_inited))
+    return internal_memccpy(dest, src, c, size);
+  ENSURE_MEMPROF_INITED();
+  void *res = REAL(memccpy)(dest, src, c, size);
+  if (res != nullptr)
+    size = static_cast<char *>(res) - static_cast<char *>(dest);
+  MEMPROF_READ_RANGE(src, size);
+  MEMPROF_WRITE_RANGE(dest, size);
+  return res;
+}
+#endif
+
 // ---------------------- InitializeMemprofInterceptors ---------------- {{{1
 namespace __memprof {
 void InitializeMemprofInterceptors() {
@@ -333,6 +349,10 @@ void InitializeMemprofInterceptors() {
   MEMPROF_INTERCEPT_FUNC(pthread_create);
   MEMPROF_INTERCEPT_FUNC(pthread_join);
 
+#if SANITIZER_INTERCEPT_MEMCCPY
+  MEMPROF_INTERCEPT_FUNC(memccpy);
+#endif
+
   InitializePlatformInterceptors();
 
   VReport(1, "MemProfiler: libc interceptors initialized\n");
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp
index 9318066afed20..102d5964d74eb 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_libc.cpp
@@ -49,6 +49,16 @@ int internal_memcmp(const void* s1, const void* s2, uptr n) {
   return 0;
 }
 
+void *internal_memccpy(void *dest, const void *src, int c, uptr n) {
+  char *d = (char *)dest;
+  const char *s = (const char *)src;
+  uptr i = 0;
+  for (; i < n && s[i] != c; ++i) d[i] = s[i];
+  if (n > 0 && i < n - 1)
+    return d + i + 1;
+  return nullptr;
+}
+
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE void *__sanitizer_internal_memcpy(void *dest,
                                                                 const void *src,
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_libc.h b/compiler-rt/lib/sanitizer_common/sanitizer_libc.h
index 1906569e2a5fc..aa0a9788e084e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_libc.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_libc.h
@@ -41,6 +41,7 @@ s64 internal_atoll(const char *nptr);
 void *internal_memchr(const void *s, int c, uptr n);
 void *internal_memrchr(const void *s, int c, uptr n);
 int internal_memcmp(const void* s1, const void* s2, uptr n);
+void *internal_memccpy(void *d, const void *s, int c, uptr n);
 ALWAYS_INLINE void *internal_memcpy(void *dest, const void *src, uptr n) {
   return __sanitizer_internal_memcpy(dest, src, n);
 }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
index 29987decdff45..5d0f871d4bfc8 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -198,6 +198,7 @@ SANITIZER_WEAK_IMPORT void *aligned_alloc(__sanitizer::usize __alignment,
 #define SANITIZER_INTERCEPT_MEMMEM (SI_POSIX && !SI_MAC_DEPLOYMENT_BELOW_10_7)
 #define SANITIZER_INTERCEPT_MEMCHR SI_NOT_FUCHSIA
 #define SANITIZER_INTERCEPT_MEMRCHR (SI_FREEBSD || SI_LINUX || SI_NETBSD)
+#define SANITIZER_INTERCEPT_MEMCCPY (SI_FREEBSD || SI_LINUX || SI_NETBSD)
 
 #define SANITIZER_INTERCEPT_READ SI_POSIX
 #define SANITIZER_INTERCEPT_PREAD SI_POSIX
diff --git a/compiler-rt/test/memprof/TestCases/memprof_memccpy.cpp b/compiler-rt/test/memprof/TestCases/memprof_memccpy.cpp
new file mode 100644
index 0000000000000..68abdbba58ad4
--- /dev/null
+++ b/compiler-rt/test/memprof/TestCases/memprof_memccpy.cpp
@@ -0,0 +1,17 @@
+// RUN: %clangxx_memprof -O0 %s -o %t
+// %env_memprof_opts=print_text=true:log_path=stdout %run %t | FileCheck %s
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main() {
+  char *p = strdup("memccpy");
+  char *d = (char *)malloc(4);
+  void *r = memccpy(d, p, 'c', 8);
+  int cmp = memcmp(r, "mem", 3);
+  free(d);
+  free(p);
+  return cmp;
+}
+// CHECK: Memory allocation stack id

[teresajohnson]

thanks, few comments/questions.

[teresajohnson]

My reading of the description of memccpy is that when c is found it should also be copied, before terminating the copy. It looks like this would miss the copy of c.

[teresajohnson]

I think the second part of this check should be "i < n" ? I.e. if c was found as the nth character, I believe that i=n-1 after the above loop and it should return non-nullptr. In this case d+i+1 = d+n-1+1 = d+n, which seems correct.

[teresajohnson]

It would be good to have a test for this. Unfortunately, I don't see any internal_* functions being tested in compiler-rt/test right now, but it would make sense to start with this one.

[teresajohnson]

There are other allocations in this code without the memccpy interception, does this test fail without that support? If not, can the checking be tightened up?