[libc-commits] [libc] f152796 - [libc][rpc] Fix the memory ordering in lock.

[Jon Chesterfield via libc-commits]

Author: Jon Chesterfield
Date: 2023-05-03T23:23:45+01:00
New Revision: f152796fd3e028d22dbe59e8157549bb98852800

URL: https://github.com/llvm/llvm-project/commit/f152796fd3e028d22dbe59e8157549bb98852800
DIFF: https://github.com/llvm/llvm-project/commit/f152796fd3e028d22dbe59e8157549bb98852800.diff

LOG: [libc][rpc] Fix the memory ordering in lock.

Prevent operation reordering with fence instead of a comment.

The mailboxes are in shared memory and the locks structure in device memory.
If the mailboxes are read and then the lock taken, the lock says nothing
about the current or future state of those mail boxes. The relaxed atomic
fetch_or can be reordered before the relaxed atomic loads of unrelated
variables unless there is a fence preventing this.

Patches both Client::try_open and Server::try_open, one of which is missing
an optimisation and the other is missing the comment, but which otherwise
could be Process::try_open followed by buffer->opcode = opcode in Client.

Reviewed By: jhuber6

Differential Revision: https://reviews.llvm.org/D149790

Added: 
    

Modified: 
    libc/src/__support/RPC/rpc.h

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/RPC/rpc.h b/libc/src/__support/RPC/rpc.h
index be5856bfa7254..ec432f3450962 100644
--- a/libc/src/__support/RPC/rpc.h
+++ b/libc/src/__support/RPC/rpc.h
@@ -248,6 +248,9 @@ LIBC_INLINE cpp::optional<Port> Client::try_open(uint16_t opcode) {
   if (lock->fetch_or(1, cpp::MemoryOrder::RELAXED))
     return cpp::nullopt;
 
+  // The mailbox state must be read with the lock held.
+  atomic_thread_fence(cpp::MemoryOrder::ACQUIRE);
+
   uint32_t in = inbox->load(cpp::MemoryOrder::RELAXED);
   uint32_t out = outbox->load(cpp::MemoryOrder::RELAXED);
 
@@ -285,6 +288,9 @@ LIBC_INLINE cpp::optional<Port> Server::try_open() {
   if (lock->fetch_or(1, cpp::MemoryOrder::RELAXED))
     return cpp::nullopt;
 
+  // The mailbox state must be read with the lock held.
+  atomic_thread_fence(cpp::MemoryOrder::ACQUIRE);
+
   in = inbox->load(cpp::MemoryOrder::RELAXED);
   out = outbox->load(cpp::MemoryOrder::RELAXED);