Skip to content

Commit

Permalink
tsan: avoid false positives related to epoll
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dvyukov committed Apr 27, 2022
1 parent 03482bc commit 16baf59
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 1 deletion.
37 changes: 37 additions & 0 deletions compiler-rt/lib/tsan/rtl/tsan_fd.cpp
Expand Up @@ -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;
};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 different epolls during different 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))
Expand Down
1 change: 1 addition & 0 deletions compiler-rt/lib/tsan/rtl/tsan_fd.h
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
Expand Up @@ -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;
}
Expand Down
42 changes: 42 additions & 0 deletions 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

0 comments on commit 16baf59

Please sign in to comment.