[clang] [compiler-rt] Move interceptors for libresolv functions to MSan (PR #119071)

via cfe-commits cfe-commits at lists.llvm.org
Sat Dec 7 07:05:44 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Aaron Puchert (aaronpuchert)

<details>
<summary>Changes</summary>

The functions are not relevant for most sanitizers and only required for MSan to see which regions have been written to. This eliminates a link dependency for all other sanitizers and fixes #<!-- -->59007: while `-lresolv` had been added for the static runtime in 6dce56b2a308, it wasn't added to the shared runtimes.

Instead of just moving the interceptors, we adapt them to MSan conventions:
* We don't skip intercepting when `msan_init_is_running` is true, but directly call ENSURE_MSAN_INITED() like most other interceptors. It seems unlikely that these functions are called during initialization.
* We don't unpoison `errno`, because none of the functions is specified to use it.

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


5 Files Affected:

- (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+1-1) 
- (modified) compiler-rt/lib/msan/msan_interceptors.cpp (+76) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc (-78) 
- (modified) compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h (-2) 
- (removed) compiler-rt/test/sanitizer_common/TestCases/Linux/b64.cpp (-42) 


``````````diff
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 7d3d7f8f03c491..03dbdc27975b42 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1410,7 +1410,7 @@ void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
   // libresolv.a, even if exists, is an empty archive to satisfy POSIX -lresolv
   // requirement.
   if (TC.getTriple().isOSLinux() && !TC.getTriple().isAndroid() &&
-      !TC.getTriple().isMusl())
+      !TC.getTriple().isMusl() && TC.getSanitizerArgs(Args).needsMsanRt())
     CmdArgs.push_back("-lresolv");
 }
 
diff --git a/compiler-rt/lib/msan/msan_interceptors.cpp b/compiler-rt/lib/msan/msan_interceptors.cpp
index f05c20618780b7..b2098d8a26d229 100644
--- a/compiler-rt/lib/msan/msan_interceptors.cpp
+++ b/compiler-rt/lib/msan/msan_interceptors.cpp
@@ -1358,6 +1358,79 @@ INTERCEPTOR(int, forkpty, int *aparent, char *name, const void *termp,
 #define MSAN_MAYBE_INTERCEPT_FORKPTY
 #endif
 
+#if SANITIZER_LINUX && !SANITIZER_ANDROID
+INTERCEPTOR(int, __b64_ntop, unsigned char const *src, SIZE_T srclength,
+            char *target, SIZE_T targsize) {
+  ENSURE_MSAN_INITED();
+  CHECK_UNPOISONED(src, srclength);
+  InterceptorScope interceptor_scope;
+  int res = REAL(__b64_ntop)(src, srclength, target, targsize);
+  if (res >= 0)
+    __msan_unpoison(target, res + 1);
+  return res;
+}
+INTERCEPTOR(int, __b64_pton, char const *src, char *target, SIZE_T targsize) {
+  ENSURE_MSAN_INITED();
+  CHECK_UNPOISONED(src, internal_strlen(src) + 1);
+  InterceptorScope interceptor_scope;
+  int res = REAL(__b64_pton)(src, target, targsize);
+  if (res >= 0)
+    __msan_unpoison(target, res);
+  return res;
+}
+#  define MSAN_MAYBE_INTERCEPT___B64_TO \
+    MSAN_INTERCEPT_FUNC(__b64_ntop);    \
+    COMMON_INTERCEPT_FUNCTION(__b64_pton);
+#else
+#  define MSAN_MAYBE_INTERCEPT___B64_TO
+#endif
+
+#if SANITIZER_LINUX && !SANITIZER_ANDROID
+#  if __GLIBC_PREREQ(2, 34)
+// Changed with https://sourceware.org/git/?p=glibc.git;h=640bbdf
+#    define DN_COMP_INTERCEPTOR_NAME dn_comp
+#    define DN_EXPAND_INTERCEPTOR_NAME dn_expand
+#  else
+#    define DN_COMP_INTERCEPTOR_NAME __dn_comp
+#    define DN_EXPAND_INTERCEPTOR_NAME __dn_expand
+#  endif
+INTERCEPTOR(int, DN_COMP_INTERCEPTOR_NAME, unsigned char *exp_dn,
+            unsigned char *comp_dn, int length, unsigned char **dnptrs,
+            unsigned char **lastdnptr) {
+  ENSURE_MSAN_INITED();
+  InterceptorScope interceptor_scope;
+  int res = REAL(DN_COMP_INTERCEPTOR_NAME)(exp_dn, comp_dn, length, dnptrs,
+                                           lastdnptr);
+  if (res >= 0) {
+    __msan_unpoison(comp_dn, res);
+    if (dnptrs && lastdnptr) {
+      unsigned char **p = dnptrs;
+      for (; p != lastdnptr && *p; ++p);
+      if (p != lastdnptr)
+        ++p;
+      __msan_unpoison(dnptrs, (p - dnptrs) * sizeof(*p));
+    }
+  }
+  return res;
+}
+INTERCEPTOR(int, DN_EXPAND_INTERCEPTOR_NAME, unsigned char const *base,
+            unsigned char const *end, unsigned char const *src, char *dest,
+            int space) {
+  ENSURE_MSAN_INITED();
+  // TODO: add read check if __dn_comp intercept added
+  InterceptorScope interceptor_scope;
+  int res = REAL(DN_EXPAND_INTERCEPTOR_NAME)(base, end, src, dest, space);
+  if (res >= 0)
+    __msan_unpoison(dest, internal_strlen(dest) + 1);
+  return res;
+}
+#  define MSAN_MAYBE_INTERCEPT_DN_COMP_EXPAND      \
+    MSAN_INTERCEPT_FUNC(DN_COMP_INTERCEPTOR_NAME); \
+    MSAN_INTERCEPT_FUNC(DN_EXPAND_INTERCEPTOR_NAME);
+#else
+#  define MSAN_MAYBE_INTERCEPT_DN_COMP_EXPAND
+#endif
+
 struct MSanInterceptorContext {
   bool in_interceptor_scope;
 };
@@ -1916,6 +1989,9 @@ void InitializeInterceptors() {
   MSAN_MAYBE_INTERCEPT_OPENPTY;
   MSAN_MAYBE_INTERCEPT_FORKPTY;
 
+  MSAN_MAYBE_INTERCEPT___B64_TO;
+  MSAN_MAYBE_INTERCEPT_DN_COMP_EXPAND;
+
   inited = 1;
 }
 } // namespace __msan
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
index ba3693dbd11f63..9cf99a192dba49 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -2538,82 +2538,6 @@ INTERCEPTOR(int, glob64, const char *pattern, int flags,
 #define INIT_GLOB64
 #endif  // SANITIZER_INTERCEPT_GLOB64
 
-#if SANITIZER_INTERCEPT___B64_TO
-INTERCEPTOR(int, __b64_ntop, unsigned char const *src, SIZE_T srclength,
-            char *target, SIZE_T targsize) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, __b64_ntop, src, srclength, target, targsize);
-  COMMON_INTERCEPTOR_READ_RANGE(ctx, src, srclength);
-  int res = REAL(__b64_ntop)(src, srclength, target, targsize);
-  if (res >= 0)
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, target, res + 1);
-  return res;
-}
-INTERCEPTOR(int, __b64_pton, char const *src, char *target, SIZE_T targsize) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, __b64_pton, src, target, targsize);
-  COMMON_INTERCEPTOR_READ_RANGE(ctx, src, internal_strlen(src) + 1);
-  int res = REAL(__b64_pton)(src, target, targsize);
-  if (res >= 0)
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, target, res);
-  return res;
-}
-#define INIT___B64_TO                      \
-    COMMON_INTERCEPT_FUNCTION(__b64_ntop); \
-    COMMON_INTERCEPT_FUNCTION(__b64_pton);
-#else  // SANITIZER_INTERCEPT___B64_TO
-#define INIT___B64_TO
-#endif  // SANITIZER_INTERCEPT___B64_TO
-
-#if SANITIZER_INTERCEPT_DN_COMP_EXPAND
-#  if __GLIBC_PREREQ(2, 34)
-// Changed with https://sourceware.org/git/?p=glibc.git;h=640bbdf
-#    define DN_COMP_INTERCEPTOR_NAME dn_comp
-#    define DN_EXPAND_INTERCEPTOR_NAME dn_expand
-#  else
-#    define DN_COMP_INTERCEPTOR_NAME __dn_comp
-#    define DN_EXPAND_INTERCEPTOR_NAME __dn_expand
-#  endif
-INTERCEPTOR(int, DN_COMP_INTERCEPTOR_NAME, unsigned char *exp_dn,
-            unsigned char *comp_dn, int length, unsigned char **dnptrs,
-            unsigned char **lastdnptr) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, DN_COMP_INTERCEPTOR_NAME, exp_dn, comp_dn,
-                           length, dnptrs, lastdnptr);
-  int res = REAL(DN_COMP_INTERCEPTOR_NAME)(exp_dn, comp_dn, length, dnptrs,
-                                           lastdnptr);
-  if (res >= 0) {
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, comp_dn, res);
-    if (dnptrs && lastdnptr) {
-      unsigned char **p = dnptrs;
-      for (; p != lastdnptr && *p; ++p)
-        ;
-      if (p != lastdnptr)
-        ++p;
-      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dnptrs, (p - dnptrs) * sizeof(*p));
-    }
-  }
-  return res;
-}
-INTERCEPTOR(int, DN_EXPAND_INTERCEPTOR_NAME, unsigned char const *base,
-            unsigned char const *end, unsigned char const *src, char *dest,
-            int space) {
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, DN_EXPAND_INTERCEPTOR_NAME, base, end, src,
-                           dest, space);
-  // TODO: add read check if __dn_comp intercept added
-  int res = REAL(DN_EXPAND_INTERCEPTOR_NAME)(base, end, src, dest, space);
-  if (res >= 0)
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dest, internal_strlen(dest) + 1);
-  return res;
-}
-#  define INIT_DN_COMP_EXPAND                            \
-    COMMON_INTERCEPT_FUNCTION(DN_COMP_INTERCEPTOR_NAME); \
-    COMMON_INTERCEPT_FUNCTION(DN_EXPAND_INTERCEPTOR_NAME);
-#else  // SANITIZER_INTERCEPT_DN_COMP_EXPAND
-#  define INIT_DN_COMP_EXPAND
-#endif  // SANITIZER_INTERCEPT_DN_COMP_EXPAND
-
 #if SANITIZER_INTERCEPT_POSIX_SPAWN
 
 template <class RealSpawnPtr>
@@ -10346,8 +10270,6 @@ static void InitializeCommonInterceptors() {
   INIT_TIMESPEC_GET;
   INIT_GLOB;
   INIT_GLOB64;
-  INIT___B64_TO;
-  INIT_DN_COMP_EXPAND;
   INIT_POSIX_SPAWN;
   INIT_WAIT;
   INIT_WAIT4;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
index 190cad7cf7c3f7..b4940b66da9717 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -266,8 +266,6 @@ SANITIZER_WEAK_IMPORT void *aligned_alloc(__sanitizer::usize __alignment,
 #define SANITIZER_INTERCEPT_TIMESPEC_GET SI_LINUX
 #define SANITIZER_INTERCEPT_GLOB (SI_GLIBC || SI_SOLARIS)
 #define SANITIZER_INTERCEPT_GLOB64 SI_GLIBC
-#define SANITIZER_INTERCEPT___B64_TO SI_LINUX_NOT_ANDROID
-#define SANITIZER_INTERCEPT_DN_COMP_EXPAND SI_LINUX_NOT_ANDROID
 #define SANITIZER_INTERCEPT_POSIX_SPAWN SI_POSIX
 #define SANITIZER_INTERCEPT_WAIT SI_POSIX
 #define SANITIZER_INTERCEPT_INET SI_POSIX
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Linux/b64.cpp b/compiler-rt/test/sanitizer_common/TestCases/Linux/b64.cpp
deleted file mode 100644
index 96fd0138648b34..00000000000000
--- a/compiler-rt/test/sanitizer_common/TestCases/Linux/b64.cpp
+++ /dev/null
@@ -1,42 +0,0 @@
-// RUN: %clangxx %s -o %t && %run %t %p
-
-#include <assert.h>
-#include <resolv.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/types.h>
-
-int main(int iArgc, char *szArgv[]) {
-  // Check NTOP writing
-  const char *src = "base64 test data";
-  size_t src_len = strlen(src);
-  size_t dst_len = ((src_len + 2) / 3) * 4 + 1;
-  char dst[dst_len];
-  int res = b64_ntop(reinterpret_cast<const unsigned char *>(src), src_len, dst,
-                     dst_len);
-  assert(res >= 0);
-  assert(strcmp(dst, "YmFzZTY0IHRlc3QgZGF0YQ==") == 0);
-
-  // Check PTON writing
-  unsigned char target[dst_len];
-  res = b64_pton(dst, target, dst_len);
-  assert(res >= 0);
-  assert(strncmp(reinterpret_cast<const char *>(target), src, res) == 0);
-
-  // Check NTOP writing for zero length src
-  src = "";
-  src_len = strlen(src);
-  assert(((src_len + 2) / 3) * 4 + 1 < dst_len);
-  res = b64_ntop(reinterpret_cast<const unsigned char *>(src), src_len, dst,
-                 dst_len);
-  assert(res >= 0);
-  assert(strcmp(dst, "") == 0);
-
-  // Check PTON writing for zero length src
-  dst[0] = '\0';
-  res = b64_pton(dst, target, dst_len);
-  assert(res >= 0);
-  assert(strncmp(reinterpret_cast<const char *>(target), src, res) == 0);
-
-  return 0;
-}

``````````

</details>


https://github.com/llvm/llvm-project/pull/119071


More information about the cfe-commits mailing list