Skip to content

[rtsan] NFC: Add comment about O_NONBLOCK behavior #116189

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

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

cjappl
Copy link
Contributor

@cjappl cjappl commented Nov 14, 2024

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Nov 14, 2024

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

Author: Chris Apple (cjappl)

Changes

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

1 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+10-12)
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index c3fcd4f2da85ce..3a1b1f6524745f 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -60,8 +60,11 @@ struct DlsymAlloc : public DlSymAllocator<DlsymAlloc> {
 // Filesystem
 
 INTERCEPTOR(int, open, const char *path, int oflag, ...) {
-  // TODO Establish whether we should intercept here if the flag contains
-  // O_NONBLOCK
+  // We do not early exit if O_NONBLOCK is set.
+  // O_NONBLOCK **does not prevent the syscall** it simply sets the FD to be in
+  // nonblocking mode, which is a different concept than our
+  // [[clang::nonblocking]], and is not rt-safe. This behavior was confirmed
+  // using Instruments on Darwin with a simple test program
   __rtsan_notify_intercepted_call("open");
 
   if (OpenReadsVaArgs(oflag)) {
@@ -77,8 +80,7 @@ INTERCEPTOR(int, open, const char *path, int oflag, ...) {
 
 #if SANITIZER_INTERCEPT_OPEN64
 INTERCEPTOR(int, open64, const char *path, int oflag, ...) {
-  // TODO Establish whether we should intercept here if the flag contains
-  // O_NONBLOCK
+  // See comment above about O_NONBLOCK
   __rtsan_notify_intercepted_call("open64");
 
   if (OpenReadsVaArgs(oflag)) {
@@ -97,8 +99,7 @@ INTERCEPTOR(int, open64, const char *path, int oflag, ...) {
 #endif // SANITIZER_INTERCEPT_OPEN64
 
 INTERCEPTOR(int, openat, int fd, const char *path, int oflag, ...) {
-  // TODO Establish whether we should intercept here if the flag contains
-  // O_NONBLOCK
+  // See comment above about O_NONBLOCK
   __rtsan_notify_intercepted_call("openat");
 
   if (OpenReadsVaArgs(oflag)) {
@@ -114,8 +115,7 @@ INTERCEPTOR(int, openat, int fd, const char *path, int oflag, ...) {
 
 #if SANITIZER_INTERCEPT_OPENAT64
 INTERCEPTOR(int, openat64, int fd, const char *path, int oflag, ...) {
-  // TODO Establish whether we should intercept here if the flag contains
-  // O_NONBLOCK
+  // See comment above about O_NONBLOCK
   __rtsan_notify_intercepted_call("openat64");
 
   if (OpenReadsVaArgs(oflag)) {
@@ -134,8 +134,7 @@ INTERCEPTOR(int, openat64, int fd, const char *path, int oflag, ...) {
 #endif // SANITIZER_INTERCEPT_OPENAT64
 
 INTERCEPTOR(int, creat, const char *path, mode_t mode) {
-  // TODO Establish whether we should intercept here if the flag contains
-  // O_NONBLOCK
+  // See comment above about O_NONBLOCK
   __rtsan_notify_intercepted_call("creat");
   const int result = REAL(creat)(path, mode);
   return result;
@@ -143,8 +142,7 @@ INTERCEPTOR(int, creat, const char *path, mode_t mode) {
 
 #if SANITIZER_INTERCEPT_CREAT64
 INTERCEPTOR(int, creat64, const char *path, mode_t mode) {
-  // TODO Establish whether we should intercept here if the flag contains
-  // O_NONBLOCK
+  // See comment above about O_NONBLOCK
   __rtsan_notify_intercepted_call("creat64");
   const int result = REAL(creat64)(path, mode);
   return result;

@cjappl cjappl merged commit d761b74 into llvm:main Nov 14, 2024
10 checks passed
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.

4 participants