[llvm] [orc-rt] Hoist run-alloc-actions functions, add error reporting. (PR #209356)
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 13 19:16:00 PDT 2026
https://github.com/lhames created https://github.com/llvm/llvm-project/pull/209356
Hoist runFinalizeActions and runDeallocActions into AllocAction.h and turn them into function templates with a ReportErrorFn argument that enables error reporting. This is used to report errors from dealloc actions.
Update SimpleNativeMemoryMap to direct such errors to the Session via ReportErrorsViaSession.
Add unit tests covering the new error-reporting paths.
>From 0563a71e10a5072bd493fac857174dae4a7b28bc Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Mon, 13 Jul 2026 23:33:13 +1000
Subject: [PATCH] [orc-rt] Hoist run-alloc-actions functions, add error
reporting.
Hoist runFinalizeActions and runDeallocActions into AllocAction.h and
turn them into function templates with a ReportErrorFn argument that
enables error reporting. This is used to report errors from dealloc
actions.
Update SimpleNativeMemoryMap to direct such errors to the Session via
ReportErrorsViaSession.
Add unit tests covering the new error-reporting paths.
---
orc-rt/include/orc-rt/AllocAction.h | 39 ++++-
orc-rt/lib/executor/AllocAction.cpp | 55 --------
orc-rt/lib/executor/CMakeLists.txt | 1 -
orc-rt/lib/executor/SimpleNativeMemoryMap.cpp | 9 +-
orc-rt/test/unit/AllocActionTest.cpp | 133 +++++++++++++++---
orc-rt/test/unit/CommonTestUtils.h | 16 +++
6 files changed, 174 insertions(+), 79 deletions(-)
delete mode 100644 orc-rt/lib/executor/AllocAction.cpp
diff --git a/orc-rt/include/orc-rt/AllocAction.h b/orc-rt/include/orc-rt/AllocAction.h
index d4e5dd7584d3a..c2afd6de1cb7b 100644
--- a/orc-rt/include/orc-rt/AllocAction.h
+++ b/orc-rt/include/orc-rt/AllocAction.h
@@ -16,6 +16,7 @@
#include "orc-rt/CallableTraitsHelper.h"
#include "orc-rt/Error.h"
#include "orc-rt/WrapperFunction.h"
+#include "orc-rt/scope_exit.h"
#include <vector>
@@ -74,11 +75,45 @@ struct AllocActionPair {
///
/// Both finalize and dealloc actions are permitted to be null (i.e. have a
/// null action function) in which case they are ignored.
+template <typename ReportErrorFn>
Expected<std::vector<AllocAction>>
-runFinalizeActions(std::vector<AllocActionPair> AAPs);
+runFinalizeActions(std::vector<AllocActionPair> AAPs,
+ ReportErrorFn &&ReportError) {
+ std::vector<AllocAction> DeallocActions;
+ auto RunDeallocActions = scope_exit([&]() {
+ while (!DeallocActions.empty()) {
+ auto B = DeallocActions.back()();
+ if (auto *ErrMsg = B.getOutOfBandError())
+ ReportError(make_error<StringError>(ErrMsg));
+ DeallocActions.pop_back();
+ }
+ });
+
+ for (auto &AAP : AAPs) {
+ if (AAP.Finalize) {
+ auto B = AAP.Finalize();
+ if (const char *ErrMsg = B.getOutOfBandError())
+ return make_error<StringError>(ErrMsg);
+ }
+ if (AAP.Dealloc)
+ DeallocActions.push_back(std::move(AAP.Dealloc));
+ }
+
+ RunDeallocActions.release();
+ return DeallocActions;
+}
/// Run the given deallocation actions in revwerse order.
-void runDeallocActions(std::vector<AllocAction> DAAs);
+template <typename ReportErrorFn>
+void runDeallocActions(std::vector<AllocAction> DAAs,
+ ReportErrorFn &&ReportError) {
+ while (!DAAs.empty()) {
+ auto B = DAAs.back()();
+ if (const char *ErrMsg = B.getOutOfBandError())
+ ReportError(make_error<StringError>(ErrMsg));
+ DAAs.pop_back();
+ }
+}
} // namespace orc_rt
diff --git a/orc-rt/lib/executor/AllocAction.cpp b/orc-rt/lib/executor/AllocAction.cpp
deleted file mode 100644
index 4211f467c6832..0000000000000
--- a/orc-rt/lib/executor/AllocAction.cpp
+++ /dev/null
@@ -1,55 +0,0 @@
-//===- AllocAction.cpp ----------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-//
-// AllocAction and related APIs.
-//
-//===----------------------------------------------------------------------===//
-
-#include "orc-rt/AllocAction.h"
-#include "orc-rt/scope_exit.h"
-
-namespace orc_rt {
-
-Expected<std::vector<AllocAction>>
-runFinalizeActions(std::vector<AllocActionPair> AAPs) {
- std::vector<AllocAction> DeallocActions;
- auto RunDeallocActions = scope_exit([&]() {
- while (!DeallocActions.empty()) {
- // TODO: Log errors from cleanup dealloc actions.
- {
- [[maybe_unused]] auto B = DeallocActions.back()();
- }
- DeallocActions.pop_back();
- }
- });
-
- for (auto &AAP : AAPs) {
- if (AAP.Finalize) {
- auto B = AAP.Finalize();
- if (const char *ErrMsg = B.getOutOfBandError())
- return make_error<StringError>(ErrMsg);
- }
- if (AAP.Dealloc)
- DeallocActions.push_back(std::move(AAP.Dealloc));
- }
-
- RunDeallocActions.release();
- return DeallocActions;
-}
-
-void runDeallocActions(std::vector<AllocAction> DAAs) {
- while (!DAAs.empty()) {
- // TODO: Log errors from cleanup dealloc actions.
- {
- [[maybe_unused]] auto B = DAAs.back()();
- }
- DAAs.pop_back();
- }
-}
-
-} // namespace orc_rt
diff --git a/orc-rt/lib/executor/CMakeLists.txt b/orc-rt/lib/executor/CMakeLists.txt
index f9f404bdcd3d8..29ae0f860070b 100644
--- a/orc-rt/lib/executor/CMakeLists.txt
+++ b/orc-rt/lib/executor/CMakeLists.txt
@@ -1,5 +1,4 @@
set(files
- AllocAction.cpp
BootstrapInfo.cpp
SimpleSymbolTable.cpp
Environment.cpp
diff --git a/orc-rt/lib/executor/SimpleNativeMemoryMap.cpp b/orc-rt/lib/executor/SimpleNativeMemoryMap.cpp
index 46b19ba7f5a65..9a6726fa70625 100644
--- a/orc-rt/lib/executor/SimpleNativeMemoryMap.cpp
+++ b/orc-rt/lib/executor/SimpleNativeMemoryMap.cpp
@@ -93,7 +93,7 @@ void SimpleNativeMemoryMap::release(OnReleaseCompleteFn &&OnComplete,
}
for (auto &[Addr, DAAs] : SI->DeallocActions)
- runDeallocActions(std::move(DAAs));
+ runDeallocActions(std::move(DAAs), ReportErrorsViaSession(S));
OnComplete(hostOSMemoryRelease(Addr, SI->Size));
}
@@ -153,12 +153,13 @@ void SimpleNativeMemoryMap::initialize(OnInitializeCompleteFn &&OnComplete,
"finalization requires at least "
"one standard-lifetime segment"));
- auto DeallocActions = runFinalizeActions(std::move(IR.AAPs));
+ auto DeallocActions =
+ runFinalizeActions(std::move(IR.AAPs), ReportErrorsViaSession(S));
if (!DeallocActions)
return OnComplete(DeallocActions.takeError());
if (auto Err = recordDeallocActions(Base, std::move(*DeallocActions))) {
- runDeallocActions(std::move(*DeallocActions));
+ runDeallocActions(std::move(*DeallocActions), ReportErrorsViaSession(S));
return OnComplete(std::move(Err));
}
@@ -192,7 +193,7 @@ void SimpleNativeMemoryMap::deinitialize(OnDeinitializeCompleteFn &&OnComplete,
SI->DeallocActions.erase(I);
}
- runDeallocActions(std::move(DAAs));
+ runDeallocActions(std::move(DAAs), ReportErrorsViaSession(S));
OnComplete(Error::success());
}
diff --git a/orc-rt/test/unit/AllocActionTest.cpp b/orc-rt/test/unit/AllocActionTest.cpp
index 0777eb27aa79e..8c4ed92d035dd 100644
--- a/orc-rt/test/unit/AllocActionTest.cpp
+++ b/orc-rt/test/unit/AllocActionTest.cpp
@@ -18,6 +18,8 @@
#include "gtest/gtest.h"
+#include "CommonTestUtils.h"
+
#include <cstring>
using namespace orc_rt;
@@ -84,6 +86,13 @@ static orc_rt_WrapperFunctionBuffer fail_action(const char *ArgData,
return WrapperFunctionBuffer::createOutOfBandError("failed").release();
}
+// Always returns an out-of-band error with a distinct message. Used to tell
+// finalize-path and dealloc-path failures apart in error-reporting tests.
+static orc_rt_WrapperFunctionBuffer fail_action_2(const char *ArgData,
+ size_t ArgSize) {
+ return WrapperFunctionBuffer::createOutOfBandError("failed_2").release();
+}
+
TEST(AllocActionTest, DefaultConstruct) {
AllocAction AA;
EXPECT_FALSE(AA);
@@ -116,29 +125,40 @@ TEST(AllocActionTest, RunFinalizationActionsComplete) {
InitialActions.push_back({MakeAAOnVal(increment_int_ptr_action),
MakeAAOnVal(decrement_int_ptr_action)});
- auto DeallocActions = cantFail(runFinalizeActions(std::move(InitialActions)));
+ auto DeallocActions =
+ cantFail(runFinalizeActions(std::move(InitialActions), noErrors));
EXPECT_EQ(Val, 2);
- runDeallocActions(std::move(DeallocActions));
+ runDeallocActions(std::move(DeallocActions), noErrors);
EXPECT_EQ(Val, 0);
}
TEST(AllocActionTest, RunFinalizeActionsFail) {
- int Val = 0;
+ int SucceedingPairVal = 0;
+ int FailingPairDeallocVal = 0;
+ int AfterFailurePairVal = 0;
std::vector<AllocActionPair> InitialActions;
- auto MakeAAOnVal = [&](AllocActionFn Fn) {
- return AllocAction(Fn, makeIntPtrArgBuffer(&Val));
+ auto MakeAA = [&](AllocActionFn Fn, int *P) {
+ return AllocAction(Fn, makeIntPtrArgBuffer(P));
};
- InitialActions.push_back({MakeAAOnVal(increment_int_ptr_action),
- MakeAAOnVal(decrement_int_ptr_action)});
- InitialActions.push_back({AllocAction(fail_action, WrapperFunctionBuffer()),
- MakeAAOnVal(decrement_int_ptr_action)});
-
- auto DeallocActions = runFinalizeActions(std::move(InitialActions));
+ // First pair's finalize and dealloc actions both increment SucceedingPairVal.
+ InitialActions.push_back(
+ {MakeAA(increment_int_ptr_action, &SucceedingPairVal),
+ MakeAA(increment_int_ptr_action, &SucceedingPairVal)});
+ // Second pair's finalize fails, so its dealloc action must not run.
+ InitialActions.push_back(
+ {AllocAction(fail_action, WrapperFunctionBuffer()), // finalize fails
+ MakeAA(increment_int_ptr_action, &FailingPairDeallocVal)});
+ // Third pair: finalize actions past the failure point must not run.
+ InitialActions.push_back(
+ {MakeAA(increment_int_ptr_action, &AfterFailurePairVal),
+ MakeAA(increment_int_ptr_action, &AfterFailurePairVal)});
+
+ auto DeallocActions = runFinalizeActions(std::move(InitialActions), noErrors);
if (DeallocActions) {
ADD_FAILURE() << "Failed to report error from runFinalizeActions";
@@ -147,8 +167,15 @@ TEST(AllocActionTest, RunFinalizeActionsFail) {
EXPECT_EQ(toString(DeallocActions.takeError()), std::string("failed"));
- // Check that we ran the decrement corresponding to the first increment.
- EXPECT_EQ(Val, 0);
+ // First pair fully ran: +1 from its finalize action, +1 from its dealloc
+ // action during cleanup.
+ EXPECT_EQ(SucceedingPairVal, 2);
+
+ // Second pair's finalize failed, so its dealloc action must not run.
+ EXPECT_EQ(FailingPairDeallocVal, 0);
+
+ // Third pair's finalize is past the failure point, so it must not run.
+ EXPECT_EQ(AfterFailurePairVal, 0);
}
TEST(AllocActionTest, RunFinalizeActionsNullFinalize) {
@@ -164,13 +191,14 @@ TEST(AllocActionTest, RunFinalizeActionsNullFinalize) {
InitialActions.push_back({AllocAction(nullptr, WrapperFunctionBuffer()),
MakeAAOnVal(decrement_int_ptr_action)});
- auto DeallocActions = cantFail(runFinalizeActions(std::move(InitialActions)));
+ auto DeallocActions =
+ cantFail(runFinalizeActions(std::move(InitialActions), noErrors));
// Both dealloc actions should be included in the returned list, despite one
// of them having a null finalize action.
EXPECT_EQ(DeallocActions.size(), 2U);
- runDeallocActions(std::move(DeallocActions));
+ runDeallocActions(std::move(DeallocActions), noErrors);
EXPECT_EQ(Val, -1);
}
@@ -188,12 +216,83 @@ TEST(AllocActionTest, RunFinalizeActionsNullDealloc) {
InitialActions.push_back({MakeAAOnVal(increment_int_ptr_action),
AllocAction(nullptr, WrapperFunctionBuffer())});
- auto DeallocActions = cantFail(runFinalizeActions(std::move(InitialActions)));
+ auto DeallocActions =
+ cantFail(runFinalizeActions(std::move(InitialActions), noErrors));
// Null dealloc actions should be filtered out of the returned list.
EXPECT_EQ(DeallocActions.size(), 1U);
- runDeallocActions(std::move(DeallocActions));
+ runDeallocActions(std::move(DeallocActions), noErrors);
+
+ EXPECT_EQ(Val, 1);
+}
+
+TEST(AllocActionTest, RunDeallocActionsReportsError) {
+ int Val = 0;
+
+ std::vector<AllocAction> DeallocActions;
+ // First dealloc action runs second (dealloc runs in reverse), but must still
+ // run despite the failure reported below.
+ DeallocActions.push_back(
+ AllocAction(increment_int_ptr_action, makeIntPtrArgBuffer(&Val)));
+ // Runs first: fails and should be reported.
+ DeallocActions.push_back(AllocAction(fail_action, WrapperFunctionBuffer()));
+
+ std::vector<std::string> ErrMsgs;
+ runDeallocActions(std::move(DeallocActions), AccumulateErrors(ErrMsgs));
+
+ ASSERT_EQ(ErrMsgs.size(), 1U);
+ EXPECT_EQ(ErrMsgs[0], "failed");
+
+ // The non-failing dealloc action still ran.
+ EXPECT_EQ(Val, 1);
+}
+
+TEST(AllocActionTest, RunDeallocActionsReportsAllErrors) {
+ std::vector<AllocAction> DeallocActions;
+ DeallocActions.push_back(AllocAction(fail_action, WrapperFunctionBuffer()));
+ DeallocActions.push_back(AllocAction(fail_action_2, WrapperFunctionBuffer()));
+
+ std::vector<std::string> ErrMsgs;
+ runDeallocActions(std::move(DeallocActions), AccumulateErrors(ErrMsgs));
+
+ // Both failures reported; dealloc runs in reverse, so fail_action_2 first.
+ ASSERT_EQ(ErrMsgs.size(), 2U);
+ EXPECT_EQ(ErrMsgs[0], "failed_2");
+ EXPECT_EQ(ErrMsgs[1], "failed");
+}
+
+TEST(AllocActionTest, RunFinalizeActionsFailReportsCleanupErrors) {
+ int Val = 0;
+
+ std::vector<AllocActionPair> InitialActions;
+ // Finalize succeeds; its dealloc decrements Val during cleanup. Proves
+ // cleanup continues past the failure of the pair below.
+ InitialActions.push_back(
+ {AllocAction(increment_int_ptr_action, makeIntPtrArgBuffer(&Val)),
+ AllocAction(decrement_int_ptr_action, makeIntPtrArgBuffer(&Val))});
+ // Finalize succeeds; its dealloc fails during the cleanup triggered below.
+ InitialActions.push_back(
+ {AllocAction(increment_int_ptr_action, makeIntPtrArgBuffer(&Val)),
+ AllocAction(fail_action, WrapperFunctionBuffer())});
+ // Finalize fails, triggering cleanup of the accumulated dealloc actions.
+ InitialActions.push_back({AllocAction(fail_action_2, WrapperFunctionBuffer()),
+ AllocAction(nullptr, WrapperFunctionBuffer())});
+
+ std::vector<std::string> ErrMsgs;
+ auto DeallocActions =
+ runFinalizeActions(std::move(InitialActions), AccumulateErrors(ErrMsgs));
+
+ // The finalize failure is the returned error...
+ ASSERT_FALSE(DeallocActions);
+ EXPECT_EQ(toString(DeallocActions.takeError()), std::string("failed_2"));
+
+ // ...while the cleanup dealloc failure is reported out-of-band.
+ ASSERT_EQ(ErrMsgs.size(), 1U);
+ EXPECT_EQ(ErrMsgs[0], "failed");
+ // Both finalize actions ran (Val += 2) and both cleanup dealloc actions ran:
+ // the failing one left Val untouched, the surviving one decremented it. That
+ // the decrement took effect proves cleanup continued past the failure.
EXPECT_EQ(Val, 1);
}
diff --git a/orc-rt/test/unit/CommonTestUtils.h b/orc-rt/test/unit/CommonTestUtils.h
index 4d584627fa6d6..9fa4f7f29c66c 100644
--- a/orc-rt/test/unit/CommonTestUtils.h
+++ b/orc-rt/test/unit/CommonTestUtils.h
@@ -21,9 +21,25 @@
#include <cstddef>
#include <cstdint>
#include <future>
+#include <string>
+#include <vector>
inline void noErrors(orc_rt::Error Err) { orc_rt::cantFail(std::move(Err)); }
+/// ReportError callback for tests that records the message of every reported
+/// error, in the order reported.
+class AccumulateErrors {
+public:
+ AccumulateErrors(std::vector<std::string> &ErrMsgs) : ErrMsgs(ErrMsgs) {}
+
+ void operator()(orc_rt::Error Err) {
+ ErrMsgs.push_back(orc_rt::toString(std::move(Err)));
+ }
+
+private:
+ std::vector<std::string> &ErrMsgs;
+};
+
inline orc_rt::ExecutorProcessInfo mockExecutorProcessInfo() noexcept {
return orc_rt::ExecutorProcessInfo("arm64-apple-darwin", 16384);
}
More information about the llvm-commits
mailing list