Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Lancern
Copy link
Member

@Lancern Lancern commented Jun 30, 2024

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

@llvmbot
Copy link
Member

llvmbot commented Jun 30, 2024

@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);

Copy link

github-actions bot commented Jun 30, 2024

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

@Lancern Lancern force-pushed the compiler-rt-internal-pipe branch from cc6373b to 9bd9072 Compare June 30, 2024 02:49
@cjappl
Copy link
Contributor

cjappl commented Oct 24, 2024

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 cjappl requested a review from dvyukov October 24, 2024 18:08
This patch tries to resolve google/sanitizers#1106 by introducing a new
internal_pipe function which will not be intercepted by TSAN.
@Lancern Lancern force-pushed the compiler-rt-internal-pipe branch from 9bd9072 to 23e4389 Compare October 28, 2024 13:39
@llvmbot llvmbot added the compiler-rt:tsan Thread sanitizer label Oct 28, 2024
@Lancern
Copy link
Member Author

Lancern commented Oct 28, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ubsan IsAccessibleMemoryRange use of pipe causes tsan false positives
3 participants