llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

[compiler-rt] Replace direct calls to pipe with internal_pipe

Open Lancern opened this issue 1 year ago • 3 comments

This PR tries to resolve google/sanitizers#1106 by introducing a new internal_pipe function which will not be intercepted by TSAN.

Lancern avatar Jun 30 '24 02:06 Lancern

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

Author: Sirui Mu (Lancern)

Changes

This PR tries to resolve google/sanitizers#1106 by introducing a new internal_pipe function which will not be intercepted by TSAN.


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

5 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp (+4)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp (+4)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_netbsd.cpp (+5)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_posix.h (+2)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp (+1-1)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
index 12df3ef73da4b..c70a689c7e599 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
@@ -796,6 +796,10 @@ uptr internal_lseek(fd_t fd, OFF_T offset, int whence) {
   return internal_syscall(SYSCALL(lseek), fd, offset, whence);
 }
 
+uptr internal_pipe(fd_t pipefd[2]) {
+  return internal_syscall(SYSCALL(pipe), pipefd);
+}
+
 #    if SANITIZER_LINUX
 uptr internal_prctl(int option, uptr arg2, uptr arg3, uptr arg4, uptr arg5) {
   return internal_syscall(SYSCALL(prctl), option, arg2, arg3, arg4, arg5);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index cbdf3e95925bf..f11d681f2a388 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -213,6 +213,10 @@ uptr internal_unlink(const char *path) {
   return unlink(path);
 }
 
+uptr internal_pipe(fd_t fildes[2]) {
+  return pipe(fildes);
+}
+
 uptr internal_sched_yield() {
   return sched_yield();
 }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_netbsd.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_netbsd.cpp
index 5e601bdcde1e5..eff8ceb3b60b9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_netbsd.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_netbsd.cpp
@@ -204,6 +204,11 @@ uptr internal_rename(const char *oldpath, const char *newpath) {
   return _REAL(rename, oldpath, newpath);
 }
 
+uptr internal_pipe(fd_t fildes[2]) {
+  DEFINE__REAL(int, pipe, int a[2]);
+  return _REAL(pipe, fildes);
+}
+
 uptr internal_sched_yield() {
   CHECK(&_sys_sched_yield);
   return _sys_sched_yield();
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix.h b/compiler-rt/lib/sanitizer_common/sanitizer_posix.h
index 14617e4771bec..c0a93eabda57d 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix.h
@@ -56,6 +56,8 @@ uptr internal_unlink(const char *path);
 uptr internal_rename(const char *oldpath, const char *newpath);
 uptr internal_lseek(fd_t fd, OFF_T offset, int whence);
 
+uptr internal_pipe(fd_t pipefd[2]);
+
 #if SANITIZER_NETBSD
 uptr internal_ptrace(int request, int pid, void *addr, int data);
 #else
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
index ece2d7d63dd61..93c49fa1dafa4 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cpp
@@ -293,7 +293,7 @@ bool IsAccessibleMemoryRange(uptr beg, uptr size) {
   // Checking too large memory ranges is slow.
   CHECK_LT(size, page_size * 10);
   int sock_pair[2];
-  if (pipe(sock_pair))
+  if (internal_pipe(sock_pair))
     return false;
   uptr bytes_written =
       internal_write(sock_pair[1], reinterpret_cast<void *>(beg), size);

llvmbot avatar Jun 30 '24 02:06 llvmbot

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

github-actions[bot] avatar Jun 30 '24 02:06 github-actions[bot]

This seems reasonable to me.

Please:

  • Fix the code formatting issue
  • Add a test that catches this issue and proves it doesn't happen anymore

I'm going to add one of the original commenters on your issue to this PR.

cjappl avatar Oct 24 '24 18:10 cjappl

@cjappl Sorry for the delay here. Updated this PR and added a test that verify TSAN does not complain any more.

Lancern avatar Oct 28 '24 13:10 Lancern