[libc-commits] [libc] [libc] Skip the RPC doorbell interrupt when the mailbox is uninitialized (PR #214037)
via libc-commits
libc-commits at lists.llvm.org
Tue Aug 4 11:49:41 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Minseob Shin (minseobshin11)
<details>
<summary>Changes</summary>
`rpc::Process::notify()` guards `doorbell->value` but then unconditionally
dereferences `doorbell->mailbox`. The two are not initialized together. The
AMDGPU offload plugin sets `value` to the address of a field in its HSA signal,
so it is never null, while `mailbox` is that signal's `event_mailbox_ptr`, which
is zero unless the signal is interrupt-backed:
```c++
Value = reinterpret_cast<uint64_t *>(&Doorbell->value);
Mailbox = reinterpret_cast<uint64_t *>(Doorbell->event_mailbox_ptr);
```
The existing guard can therefore never fire on AMDGPU, and `notify()` stores to
a null address. Guard the mailbox store instead, which matches the intent stated
in 4961700c1004 that the interrupt is "completely optional, as it is ignored if
uninitialized".
The check is inside the block rather than in the outer condition because `value`
is the pending-work counter the server observes; skipping the increment starves
a polling server instead of fixing anything.
## Reproducer
An OpenMP offload program calling `malloc` inside a target region, with a
doorbell that is not interrupt-backed. No profiler needed:
```console
$ ./dm # ok
$ HSA_ENABLE_INTERRUPT=0 ./dm # memory access fault at virtual address (nil)
```
Originally found under `rocprofv3 --kernel-trace`, where Quantum ESPRESSO faults
on its first SCF iteration because amdflang lowers non-contiguous array-section
assignments to device-heap temporaries.
## Testing
New unit test covering a partially initialized doorbell and an unconfigured one;
the former segfaults without this change.
Also verified end to end on MI350X (gfx950, ROCm 7.14, clang 23) by rebuilding
both consumers of this header, `libc.a` and the OpenMP DeviceRTL, with and
without the change. In-kernel `malloc`, in-kernel `printf`, and a Fortran
array-section assignment all fault before it and complete with correct results
after.
---
Full diff: https://github.com/llvm/llvm-project/pull/214037.diff
3 Files Affected:
- (modified) libc/shared/rpc.h (+10-5)
- (modified) libc/test/src/__support/RPC/CMakeLists.txt (+10)
- (added) libc/test/src/__support/RPC/rpc_doorbell_test.cpp (+52)
``````````diff
diff --git a/libc/shared/rpc.h b/libc/shared/rpc.h
index 5bc518ab97af7..eb6051f329e4a 100644
--- a/libc/shared/rpc.h
+++ b/libc/shared/rpc.h
@@ -158,11 +158,16 @@ template <bool Invert> struct Process {
if (rpc::is_first_lane(lane_mask)) {
if (!__scoped_atomic_fetch_add(doorbell->value, 1UL, __ATOMIC_RELAXED,
__MEMORY_SCOPE_SYSTEM)) {
- __scoped_atomic_store_n(doorbell->mailbox,
- static_cast<uint64_t>(doorbell->event_id),
- __ATOMIC_RELAXED, __MEMORY_SCOPE_SYSTEM);
- __scoped_atomic_thread_fence(__ATOMIC_RELEASE, __MEMORY_SCOPE_SYSTEM);
- signal_interrupt(event_id);
+ // A doorbell that is not interrupt-backed has no mailbox. The counter
+ // incremented above already publishes the work, so skipping the
+ // interrupt is safe.
+ if (doorbell->mailbox) {
+ __scoped_atomic_store_n(doorbell->mailbox,
+ static_cast<uint64_t>(doorbell->event_id),
+ __ATOMIC_RELAXED, __MEMORY_SCOPE_SYSTEM);
+ __scoped_atomic_thread_fence(__ATOMIC_RELEASE, __MEMORY_SCOPE_SYSTEM);
+ signal_interrupt(event_id);
+ }
}
}
#endif
diff --git a/libc/test/src/__support/RPC/CMakeLists.txt b/libc/test/src/__support/RPC/CMakeLists.txt
index 8f79a49cec40c..11d09226077c8 100644
--- a/libc/test/src/__support/RPC/CMakeLists.txt
+++ b/libc/test/src/__support/RPC/CMakeLists.txt
@@ -9,3 +9,13 @@ add_libc_test(
DEPENDS
libc.src.__support.RPC.rpc
)
+
+add_libc_test(
+ rpc_doorbell_test
+ SUITE
+ libc-rpc-tests
+ SRCS
+ rpc_doorbell_test.cpp
+ DEPENDS
+ libc.src.__support.RPC.rpc
+)
diff --git a/libc/test/src/__support/RPC/rpc_doorbell_test.cpp b/libc/test/src/__support/RPC/rpc_doorbell_test.cpp
new file mode 100644
index 0000000000000..5a1874647b4fd
--- /dev/null
+++ b/libc/test/src/__support/RPC/rpc_doorbell_test.cpp
@@ -0,0 +1,52 @@
+//===-- tests for the RPC doorbell ---------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/RPC/rpc.h"
+
+#include "test/UnitTest/Test.h"
+
+namespace {
+enum { lane_size = 8, port_count = 4 };
+
+using ProcType = LIBC_NAMESPACE::rpc::Process<false>;
+
+enum { alloc_size = ProcType::allocation_size(port_count, 1) };
+
+alignas(64) char buffer[alloc_size] = {0};
+} // namespace
+
+// A doorbell can be only partially initialized. The AMDGPU offload plugin sets
+// 'value' to the address of a field inside its HSA signal, so it is never null,
+// while 'mailbox' is that signal's event mailbox pointer, which is null unless
+// the signal is backed by an interrupt. Ringing such a doorbell must not
+// dereference the null mailbox, and must still publish the pending work so a
+// polling server makes progress.
+TEST(LlvmLibcRPCDoorbell, NotifyWithoutMailbox) {
+ ProcType Proc(port_count, buffer);
+
+ uint64_t pending = 0;
+ Proc.doorbell->value = &pending;
+ Proc.doorbell->mailbox = nullptr;
+ Proc.doorbell->event_id = 0;
+
+ Proc.notify(/*lane_mask=*/1);
+
+#ifndef _MSC_VER
+ EXPECT_EQ(pending, static_cast<uint64_t>(1));
+#endif
+}
+
+// A doorbell that was never configured at all is ignored entirely.
+TEST(LlvmLibcRPCDoorbell, NotifyWithoutDoorbell) {
+ ProcType Proc(port_count, buffer);
+
+ Proc.doorbell->value = nullptr;
+ Proc.doorbell->mailbox = nullptr;
+
+ Proc.notify(/*lane_mask=*/1);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/214037
More information about the libc-commits
mailing list