[compiler-rt] 16baf59 - tsan: avoid false positives related to epoll
Dmitry Vyukov via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 27 07:59:57 PDT 2022
Author: Dmitry Vyukov
Date: 2022-04-27T16:59:46+02:00
New Revision: 16baf59c6d0b3bf7392995e3e55fc9e2ba9cb5e7
URL: https://github.com/llvm/llvm-project/commit/16baf59c6d0b3bf7392995e3e55fc9e2ba9cb5e7
DIFF: https://github.com/llvm/llvm-project/commit/16baf59c6d0b3bf7392995e3e55fc9e2ba9cb5e7.diff
LOG: tsan: avoid false positives related to epoll
An application can use the mere fact of epoll_wait returning an fd
as synchronization with the write on the fd that triggered the notification.
This pattern come up in an internal networking server (b/229276331).
If an fd is added to epoll, setup a link from the fd to the epoll fd
and use it for synchronization as well.
Reviewed By: melver
Differential Revision: https://reviews.llvm.org/D124518
Added:
compiler-rt/test/tsan/Linux/epoll_norace.cpp
Modified:
compiler-rt/lib/tsan/rtl/tsan_fd.cpp
compiler-rt/lib/tsan/rtl/tsan_fd.h
compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/tsan/rtl/tsan_fd.cpp b/compiler-rt/lib/tsan/rtl/tsan_fd.cpp
index 9f358eabd21c8..cf8f491fdbf09 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_fd.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_fd.cpp
@@ -29,6 +29,9 @@ struct FdSync {
struct FdDesc {
FdSync *sync;
+ // This is used to establish write -> epoll_wait synchronization
+ // where epoll_wait receives notification about the write.
+ atomic_uintptr_t aux_sync; // FdSync*
Tid creation_tid;
StackID creation_stack;
};
@@ -103,6 +106,10 @@ static void init(ThreadState *thr, uptr pc, int fd, FdSync *s,
unref(thr, pc, d->sync);
d->sync = 0;
}
+ unref(thr, pc,
+ reinterpret_cast<FdSync *>(
+ atomic_load(&d->aux_sync, memory_order_relaxed)));
+ atomic_store(&d->aux_sync, 0, memory_order_relaxed);
if (flags()->io_sync == 0) {
unref(thr, pc, s);
} else if (flags()->io_sync == 1) {
@@ -185,6 +192,8 @@ void FdRelease(ThreadState *thr, uptr pc, int fd) {
MemoryAccess(thr, pc, (uptr)d, 8, kAccessRead);
if (s)
Release(thr, pc, (uptr)s);
+ if (uptr aux_sync = atomic_load(&d->aux_sync, memory_order_acquire))
+ Release(thr, pc, aux_sync);
}
void FdAccess(ThreadState *thr, uptr pc, int fd) {
@@ -229,6 +238,10 @@ void FdClose(ThreadState *thr, uptr pc, int fd, bool write) {
}
unref(thr, pc, d->sync);
d->sync = 0;
+ unref(thr, pc,
+ reinterpret_cast<FdSync *>(
+ atomic_load(&d->aux_sync, memory_order_relaxed)));
+ atomic_store(&d->aux_sync, 0, memory_order_relaxed);
d->creation_tid = kInvalidTid;
d->creation_stack = kInvalidStackID;
}
@@ -287,6 +300,30 @@ void FdPollCreate(ThreadState *thr, uptr pc, int fd) {
init(thr, pc, fd, allocsync(thr, pc));
}
+void FdPollAdd(ThreadState *thr, uptr pc, int epfd, int fd) {
+ DPrintf("#%d: FdPollAdd(%d, %d)\n", thr->tid, epfd, fd);
+ if (bogusfd(epfd) || bogusfd(fd))
+ return;
+ FdDesc *d = fddesc(thr, pc, fd);
+ // Associate fd with epoll fd only once.
+ // While an fd can be associated with multiple epolls at the same time,
+ // or with
diff erent epolls during
diff erent phases of lifetime,
+ // synchronization semantics (and examples) of this are unclear.
+ // So we don't support this for now.
+ // If we change the association, it will also create lifetime management
+ // problem for FdRelease which accesses the aux_sync.
+ if (atomic_load(&d->aux_sync, memory_order_relaxed))
+ return;
+ FdDesc *epd = fddesc(thr, pc, epfd);
+ FdSync *s = epd->sync;
+ if (!s)
+ return;
+ uptr cmp = 0;
+ if (atomic_compare_exchange_strong(
+ &d->aux_sync, &cmp, reinterpret_cast<uptr>(s), memory_order_release))
+ ref(s);
+}
+
void FdSocketCreate(ThreadState *thr, uptr pc, int fd) {
DPrintf("#%d: FdSocketCreate(%d)\n", thr->tid, fd);
if (bogusfd(fd))
diff --git a/compiler-rt/lib/tsan/rtl/tsan_fd.h b/compiler-rt/lib/tsan/rtl/tsan_fd.h
index d9648178481c6..92625dc4b4a19 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_fd.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_fd.h
@@ -49,6 +49,7 @@ void FdEventCreate(ThreadState *thr, uptr pc, int fd);
void FdSignalCreate(ThreadState *thr, uptr pc, int fd);
void FdInotifyCreate(ThreadState *thr, uptr pc, int fd);
void FdPollCreate(ThreadState *thr, uptr pc, int fd);
+void FdPollAdd(ThreadState *thr, uptr pc, int epfd, int fd);
void FdSocketCreate(ThreadState *thr, uptr pc, int fd);
void FdSocketAccept(ThreadState *thr, uptr pc, int fd, int newfd);
void FdSocketConnecting(ThreadState *thr, uptr pc, int fd);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index d6021a754e71a..8e40ea76fe32c 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -1900,8 +1900,10 @@ TSAN_INTERCEPTOR(int, epoll_ctl, int epfd, int op, int fd, void *ev) {
FdAccess(thr, pc, epfd);
if (epfd >= 0 && fd >= 0)
FdAccess(thr, pc, fd);
- if (op == EPOLL_CTL_ADD && epfd >= 0)
+ if (op == EPOLL_CTL_ADD && epfd >= 0) {
+ FdPollAdd(thr, pc, epfd, fd);
FdRelease(thr, pc, epfd);
+ }
int res = REAL(epoll_ctl)(epfd, op, fd, ev);
return res;
}
diff --git a/compiler-rt/test/tsan/Linux/epoll_norace.cpp b/compiler-rt/test/tsan/Linux/epoll_norace.cpp
new file mode 100644
index 0000000000000..1cef61919e9ee
--- /dev/null
+++ b/compiler-rt/test/tsan/Linux/epoll_norace.cpp
@@ -0,0 +1,42 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+
+// The test captures what some high-performance networking servers do.
+// One thread writes to an fd, and another just receives an epoll
+// notification about the write to synchronize with the first thread
+// w/o actually reading from the fd.
+
+#include "../test.h"
+#include <errno.h>
+#include <sys/epoll.h>
+#include <sys/eventfd.h>
+
+int main() {
+ int efd = epoll_create(1);
+ if (efd == -1)
+ exit(printf("epoll_create failed: %d\n", errno));
+ int fd = eventfd(0, 0);
+ if (fd == -1)
+ exit(printf("eventfd failed: %d\n", errno));
+ epoll_event event = {.events = EPOLLIN | EPOLLET};
+ if (epoll_ctl(efd, EPOLL_CTL_ADD, fd, &event))
+ exit(printf("epoll_ctl failed: %d\n", errno));
+ pthread_t th;
+ pthread_create(
+ &th, nullptr,
+ +[](void *arg) -> void * {
+ long long to_add = 1;
+ if (write((long)arg, &to_add, sizeof(to_add)) != sizeof(to_add))
+ exit(printf("write failed: %d\n", errno));
+ return nullptr;
+ },
+ (void *)(long)fd);
+ struct epoll_event events[1] = {};
+ if (epoll_wait(efd, events, 1, -1) != 1)
+ exit(printf("epoll_wait failed: %d\n", errno));
+ close(fd);
+ pthread_join(th, nullptr);
+ close(efd);
+ fprintf(stderr, "DONE\n");
+}
+
+// CHECK: DONE
More information about the llvm-commits
mailing list