From: Paul Floyd <pjfloyd@wanadoo.fr>
Date: Sat, 18 Nov 2023 06:34:00 +0000 (+0100)
Subject: Darwin: remove global variable used in aio_return
X-Git-Tag: VALGRIND_3_23_0~263
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e19b6dc604f99dbe7dc8c3676dce5a4f186c5235;p=thirdparty%2Fvalgrind.git

Darwin: remove global variable used in aio_return

This was using a global variable to signify a read success
between pre- and post- aio_return. I don't think that this
was safe. There could be multiple calls to aio_return on
different threads. The global aiocpb_table is safe enough
as the pointers to control block are unique. However if two
threads manipulate was_a_successful_aio_read there's no way
of telling them apart. Rather than trying to make
was_a_successful_aio_read thread safe I did the same as on
FreeBSD and removed post- aio_return and now do everything
in pre- aio_return. No global variable, no thread hazard.
---

diff --git a/coregrind/m_syswrap/syswrap-darwin.c b/coregrind/m_syswrap/syswrap-darwin.c
index 3b8ceda3e2..01281caf4b 100644
--- a/coregrind/m_syswrap/syswrap-darwin.c
+++ b/coregrind/m_syswrap/syswrap-darwin.c
@@ -4580,8 +4580,6 @@ static void aio_init(void)
    aio_init_done = True;
 }
 
-static Bool was_a_successful_aio_read = False;
-
 PRE(aio_return)
 {
    struct vki_aiocb* aiocbp = (struct vki_aiocb*)ARG1;
@@ -4589,22 +4587,21 @@ PRE(aio_return)
    // contents of the struct.
    PRINT( "aio_return ( %#lx )", ARG1 );
    PRE_REG_READ1(long, "aio_return", struct vki_aiocb*, aiocbp);
-
-   if (!aio_init_done) aio_init();
-   was_a_successful_aio_read = VG_(OSetWord_Remove)(aiocbp_table, (UWord)aiocbp);
-}
-POST(aio_return)
-{
-   // If we found the aiocbp in our own table it must have been an aio_read(),
-   // so mark the buffer as written.  If we didn't find it, it must have been
-   // an aio_write() or a bogus aio_return() (eg. a second one on the same
-   // aiocbp).  Either way, the buffer won't have been written so we don't
-   // have to mark the buffer as written.
-   if (was_a_successful_aio_read) {
-      struct vki_aiocb* aiocbp = (struct vki_aiocb*)ARG1;
-      POST_MEM_WRITE((Addr)aiocbp->aio_buf, aiocbp->aio_nbytes);
-      was_a_successful_aio_read = False;
+   if (ML_(safe_to_deref)((struct vki_aiocb *)ARG1, sizeof(struct vki_aiocb))) {
+      SET_STATUS_from_SysRes(VG_(do_syscall1)(SYSNO, ARG1));
+      if (SUCCESS && RES >= 0) {
+         struct vki_aiocb* aiocbp = (struct vki_aiocb*)ARG1;
+         if (!aio_init_done) {
+            aio_init();
+         }
+         if (VG_(OSetWord_Remove)(aiocbp_table, (UWord)aiocbp)) {
+            POST_MEM_WRITE((Addr)aiocbp->aio_buf, aiocbp->aio_nbytes);
+         }
+      }
+   } else {
+      SET_STATUS_Failure(VKI_EINVAL);
    }
+
 }
 
 PRE(aio_suspend)
@@ -10460,7 +10457,7 @@ const SyscallTableEntry ML_(syscall_table)[] = {
    _____(VG_DARWIN_SYSCALL_CONSTRUCT_UNIX(308)),   // old __pthread_cond_timedwait
 #endif
 // _____(__NR_aio_fsync), 
-   MACXY(__NR_aio_return,     aio_return), 
+   MACX_(__NR_aio_return,     aio_return),
    MACX_(__NR_aio_suspend,    aio_suspend), 
 // _____(__NR_aio_cancel), 
    MACX_(__NR_aio_error,      aio_error),