From ff786cdea6524acf81f1a2e08fd2f241e59cf299 Mon Sep 17 00:00:00 2001 From: Howard Pritchard Date: Mon, 10 Aug 2026 14:14:18 -0600 Subject: [PATCH 1/3] Emit MPI Standard ABI handles in MPI_T event payloads Open MPI's MPI_T event producers publish MPI object handles (communicator, window, session, error handler, file) in their event payloads. Those values were always the internal object pointer, which is correct only for a tool built against the Open MPI ABI. A tool built against the MPI Standard ABI expects the integer handle instead, so every producer raise site had a placeholder that set the Standard-ABI handle to 0 pending this work. Record the process ABI at initialization and convert handles at each raise site accordingly. The Standard-ABI variants of MPI_Init, MPI_Init_thread, and MPI_Session_init set the process-global flag ompi_mpit_callback_abi to OMPI_MPIT_ABI_STANDARD (the Open MPI ABI keeps the default). Keying off process init, rather than callback registration, also covers the world-model initialization event, which is raised during MPI_Init before any tool can register a callback. The intern-to-ABI converters are generated only into libmpi_abi, a higher layer than the producer raise sites in libopen_mpi, so a raise site cannot call them directly without violating the one-way OPAL/OMPI linker boundary. Resolve this with the same downward callback-registration pattern the tree already uses for ompi_mpiext_init: the Standard-ABI init path installs a converter function pointer into libopen_mpi via ompi_mpit_register_abi_handle_convert(), and the raise sites reach it through ompi_mpit_abi_handle(). The converter itself lives in the new libmpi_abi-only translation unit mpit_abi_handle_convert.c and dispatches on the object's MPI_T_BIND_* kind to the appropriate generated reverse converter. When no converter is registered (the Open MPI ABI never installs one), ompi_mpit_abi_handle() returns 0, matching the previous placeholder rather than emitting a wrong pointer value. Rewrite all handle-emitting raise sites (communicator create/free/ name-set, window create/free, instance init/finalize, and error-handler invocation, including both the error-handler handle and the invoking object converted per its binding kind) to use the new helper in the Standard-ABI branch. Make the Standard-ABI copy of MPI_T_event_register_callback re-assert the flag as well, and drop its stale comment claiming the feature was unimplemented. Add an MPI Standard ABI test probe (callback_mpit_event_handle) to the mpi-abi harness: under a libmpi_abi-linked process it drives the communicator-created event and asserts the payload carries the Standard-ABI integer handle of the new communicator, exercising the downward converter registration and the raise-site conversion end to end. Update the MPI_T events specification to describe the registered converter and the init-time flag, and add a v6.1.x changelog entry. Signed-off-by: Howard Pritchard --- docs/release-notes/changelog/v6.1.x.rst | 7 + ompi/communicator/comm.c | 9 +- ompi/communicator/comm_cid.c | 5 +- ompi/errhandler/errhandler_invoke.c | 22 ++- ompi/instance/instance.c | 12 +- ompi/mpi/c/Makefile.am | 2 + ompi/mpi/c/Makefile_abi.include | 1 + ompi/mpi/c/init.c.in | 15 ++ ompi/mpi/c/init_thread.c.in | 15 ++ ompi/mpi/c/mpit_abi_handle_convert.c | 77 +++++++++ ompi/mpi/c/mpit_abi_handle_convert.h | 37 +++++ ompi/mpi/c/session_init.c.in | 15 ++ ompi/mpi/tool/event_register_callback.c.in | 19 ++- ompi/runtime/ompi_mpit_events.h | 31 +++- ompi/runtime/ompi_mpit_register_events.c | 27 ++- ompi/test/mpi-abi/Makefile.am | 2 + ompi/test/mpi-abi/_abi_tables.py | 30 ++++ .../callback_mpit_event_handle.cbody.in | 155 ++++++++++++++++++ .../callback_mpit_event_handle.prologue.in | 56 +++++++ ompi/win/win.c | 10 +- specs/mpi-t-events/spec.md | 35 +++- 21 files changed, 538 insertions(+), 44 deletions(-) create mode 100644 ompi/mpi/c/mpit_abi_handle_convert.c create mode 100644 ompi/mpi/c/mpit_abi_handle_convert.h create mode 100644 ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.cbody.in create mode 100644 ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.prologue.in diff --git a/docs/release-notes/changelog/v6.1.x.rst b/docs/release-notes/changelog/v6.1.x.rst index b712b142145..3190c89800d 100644 --- a/docs/release-notes/changelog/v6.1.x.rst +++ b/docs/release-notes/changelog/v6.1.x.rst @@ -36,6 +36,13 @@ Open MPI version v6.1.0 operation using the same communicator, peer, and tag, which then never completed. +- MPI_T event producers now emit the correct MPI object handle + representation for the tool's MPI ABI. When a process runs under the + MPI Standard ABI, the communicator, window, session, error-handler, + and file handles carried in MPI_T event payloads are now the Standard + ABI integer handles instead of a placeholder value; under the Open MPI + ABI they remain the internal object handle as before. + - Renamed the ``--enable-weak-symbols`` configure option to ``--enable-weak-aliases``, which more accurately reflects the linker feature (weak symbol *aliases*) that Open MPI actually tests for and diff --git a/ompi/communicator/comm.c b/ompi/communicator/comm.c index 6b0a7fa81f5..cebdb6e58a1 100644 --- a/ompi/communicator/comm.c +++ b/ompi/communicator/comm.c @@ -2362,8 +2362,8 @@ int ompi_comm_set_name (ompi_communicator_t *comm, const char *name ) if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.handle = (uint64_t) (uintptr_t) comm; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle value. */ - payload.handle = 0; + /* MPI Standard ABI: publish the integer MPI_Comm handle. */ + payload.handle = ompi_mpit_abi_handle(comm, MPI_T_BIND_MPI_COMM); } mca_base_event_raise_bound(ompi_event_comm_name_set, NULL, comm, &payload); } @@ -2528,9 +2528,8 @@ int ompi_comm_free( ompi_communicator_t **comm ) if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.handle = (uint64_t) (uintptr_t) *comm; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle value for the - communicator *comm. */ - payload.handle = 0; + /* MPI Standard ABI: publish the integer MPI_Comm handle. */ + payload.handle = ompi_mpit_abi_handle(*comm, MPI_T_BIND_MPI_COMM); } mca_base_event_raise(ompi_event_comm_freed, NULL, &payload); } diff --git a/ompi/communicator/comm_cid.c b/ompi/communicator/comm_cid.c index 6e3108bc7d3..77411dac2de 100644 --- a/ompi/communicator/comm_cid.c +++ b/ompi/communicator/comm_cid.c @@ -948,9 +948,8 @@ static int ompi_comm_activate_complete (ompi_comm_cid_context_t *context) if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.handle = (uint64_t) (uintptr_t) *newcomm; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle value for the - communicator *newcomm. */ - payload.handle = 0; + /* MPI Standard ABI: publish the integer MPI_Comm handle. */ + payload.handle = ompi_mpit_abi_handle(*newcomm, MPI_T_BIND_MPI_COMM); } mca_base_event_raise(ompi_event_comm_created, NULL, &payload); } diff --git a/ompi/errhandler/errhandler_invoke.c b/ompi/errhandler/errhandler_invoke.c index ad6c41d09c5..1b0e5bf3582 100644 --- a/ompi/errhandler/errhandler_invoke.c +++ b/ompi/errhandler/errhandler_invoke.c @@ -71,17 +71,25 @@ int ompi_errhandler_invoke(ompi_errhandler_t *errhandler, void *mpi_object, } payload.err_code = (int32_t) err_code; payload.object_type = object_bind; - /* XXX ABI: the MPI_Errhandler handle and the invoking object's handle - must match the registering MPI_T tool's ABI (ompi_mpit_callback_abi). */ + /* The MPI_Errhandler handle and the invoking object's handle must match + the registering MPI_T tool's ABI (ompi_mpit_callback_abi). */ if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.errhandler_handle = (uint64_t) (uintptr_t) errhandler; payload.object_handle = (uint64_t) (uintptr_t) mpi_object; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle values -- the - MPI_Errhandler, and mpi_object converted per object_type - (MPI_Comm / MPI_Win / MPI_File / MPI_Session). */ - payload.errhandler_handle = 0; - payload.object_handle = 0; + /* MPI Standard ABI: publish integer handles. The errhandler is + converted as an MPI_Errhandler; the invoking object is converted + per its binding kind (MPI_Comm / MPI_Win / MPI_File / + MPI_Session). An object with no binding (e.g. a predefined + handler routed before MPI_INIT) has object_bind == NO_OBJECT and + a 0 object_handle, matching the Open MPI ABI's "0 when not + available" behavior. */ + payload.errhandler_handle + = ompi_mpit_abi_handle(errhandler, MPI_T_BIND_MPI_ERRHANDLER); + payload.object_handle + = (MPI_T_BIND_NO_OBJECT == object_bind) + ? 0 + : ompi_mpit_abi_handle(mpi_object, object_bind); } mca_base_event_raise(ompi_event_errhandler_invoked, NULL, &payload); } diff --git a/ompi/instance/instance.c b/ompi/instance/instance.c index 7d92aa07085..0fcda68ac82 100644 --- a/ompi/instance/instance.c +++ b/ompi/instance/instance.c @@ -1132,9 +1132,9 @@ int ompi_mpi_instance_init (int ts_level, opal_info_t *info, ompi_errhandler_t if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.instance_id = (uint64_t) (uintptr_t) new_instance; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle value for the - session new_instance. */ - payload.instance_id = 0; + /* MPI Standard ABI: publish the integer MPI_Session handle. */ + payload.instance_id = ompi_mpit_abi_handle(new_instance, + MPI_T_BIND_MPI_SESSION); } mca_base_event_raise(ompi_event_initialization, NULL, &payload); } @@ -1269,9 +1269,9 @@ int ompi_mpi_instance_finalize (ompi_instance_t **instance) if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.instance_id = (uint64_t) (uintptr_t) *instance; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle value for the - session *instance. */ - payload.instance_id = 0; + /* MPI Standard ABI: publish the integer MPI_Session handle. */ + payload.instance_id = ompi_mpit_abi_handle(*instance, + MPI_T_BIND_MPI_SESSION); } mca_base_event_raise(ompi_event_finalization, NULL, &payload); } diff --git a/ompi/mpi/c/Makefile.am b/ompi/mpi/c/Makefile.am index d4b087630ec..fdcd8ecfb63 100644 --- a/ompi/mpi/c/Makefile.am +++ b/ompi/mpi/c/Makefile.am @@ -527,6 +527,8 @@ EXTRA_DIST = $(prototype_sources) \ abi_converters.h \ abi_converters.c \ abi_handle_convert.h \ + mpit_abi_handle_convert.h \ + mpit_abi_handle_convert.c \ abi_get_info.c.in \ abi.h.in diff --git a/ompi/mpi/c/Makefile_abi.include b/ompi/mpi/c/Makefile_abi.include index 0577770561b..2d26a75c569 100644 --- a/ompi/mpi/c/Makefile_abi.include +++ b/ompi/mpi/c/Makefile_abi.include @@ -38,6 +38,7 @@ BUILT_SOURCES = abi.h abi_converters.h standard_abi/mpi.h libmpi_c_abi_la_SOURCES = \ abi_converters.c \ + mpit_abi_handle_convert.c \ attr_fn_abi.c \ ompi_isendrecv.c \ ompi_sendrecv.c \ diff --git a/ompi/mpi/c/init.c.in b/ompi/mpi/c/init.c.in index 95e1fadec2f..f4920366626 100644 --- a/ompi/mpi/c/init.c.in +++ b/ompi/mpi/c/init.c.in @@ -35,6 +35,12 @@ #include "ompi/errhandler/errhandler.h" #include "ompi/constants.h" #include "ompi/mpiext/mpiext.h" +#include "ompi/runtime/ompi_mpit_events.h" +/* Declares ompi_mpit_abi_handle_convert_impl(); only *referenced* in the + Standard-ABI compile (guarded by OMPI_ABI_SRC in the body below), but + included unconditionally because OMPI_ABI_SRC is not yet defined this early + in the generated file. */ +#include "ompi/mpi/c/mpit_abi_handle_convert.h" PROTOTYPE INT init(INT_OUT argc, ARGV argv) { @@ -55,6 +61,15 @@ PROTOTYPE INT init(INT_OUT argc, ARGV argv) * through the registered function pointer, avoiding a direct symbol dependency on libmpi. * MPI extensions are only supported in the OMPI ABI, not the MPI Forum ABI. */ ompi_mpi_instance_register_mpiext_init(ompi_mpiext_init); +#else + /* This is the MPI Standard ABI entry point, so a tool observing MPI_T + * events from this process must see Standard-ABI integer handles in event + * payloads. Record the process ABI and install the intern->ABI handle + * converter downward into libopen_mpi (the converter lives here in + * libmpi_abi; see mpit_abi_handle_convert.c), mirroring the mpiext_init + * registration above. */ + ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; + ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); #endif /* Call the back-end initialization function (we need to put as diff --git a/ompi/mpi/c/init_thread.c.in b/ompi/mpi/c/init_thread.c.in index df05071efe1..e5a5f82808d 100644 --- a/ompi/mpi/c/init_thread.c.in +++ b/ompi/mpi/c/init_thread.c.in @@ -38,6 +38,12 @@ #include "ompi/mca/hook/base/base.h" #include "ompi/instance/instance.h" #include "ompi/mpiext/mpiext.h" +#include "ompi/runtime/ompi_mpit_events.h" +/* Declares ompi_mpit_abi_handle_convert_impl(); only *referenced* in the + Standard-ABI compile (guarded by OMPI_ABI_SRC in the body below), but + included unconditionally because OMPI_ABI_SRC is not yet defined this early + in the generated file. */ +#include "ompi/mpi/c/mpit_abi_handle_convert.h" PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, TS_LEVEL required, @@ -75,6 +81,15 @@ PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, TS_LEVEL required, * through the registered function pointer, avoiding a direct symbol dependency on libmpi. * MPI extensions are only supported in the OMPI ABI, not the MPI Forum ABI. */ ompi_mpi_instance_register_mpiext_init(ompi_mpiext_init); +#else + /* This is the MPI Standard ABI entry point, so a tool observing MPI_T + * events from this process must see Standard-ABI integer handles in event + * payloads. Record the process ABI and install the intern->ABI handle + * converter downward into libopen_mpi (the converter lives here in + * libmpi_abi; see mpit_abi_handle_convert.c), mirroring the mpiext_init + * registration above. */ + ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; + ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); #endif /* Call the back-end initialization function (we need to put as diff --git a/ompi/mpi/c/mpit_abi_handle_convert.c b/ompi/mpi/c/mpit_abi_handle_convert.c new file mode 100644 index 00000000000..18979f9cb86 --- /dev/null +++ b/ompi/mpi/c/mpit_abi_handle_convert.c @@ -0,0 +1,77 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ +/* + * Copyright (c) 2026 Triad National Security, LLC. All rights + * reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + * + * MPI Standard ABI handle converter for MPI_T event payloads. + * + * The MPI_T event producers in libopen_mpi publish MPI object handles + * (communicator, window, session, error handler, file) in their event + * payloads. Under the MPI Standard ABI a tool expects those handles as the + * Standard-ABI integer handle, not the internal object pointer. The + * intern->ABI converters (ompi_convert_comm_ompi_to_standard(), etc.) are + * generated into abi_converters.h and compiled only into libmpi_abi (the upper + * layer), so the raise sites in libopen_mpi cannot call them directly (the + * OPAL->OMPI layering forbids an upward link dependency). + * + * This translation unit lives in libmpi_abi and provides the converter. The + * Standard-ABI init entry points (MPI_Init / MPI_Init_thread / MPI_Session_init) + * install it downward with ompi_mpit_register_abi_handle_convert(), mirroring + * ompi_mpi_instance_register_mpiext_init(). + */ + +#include "ompi_config.h" + +#include +#include + +#include "ompi/communicator/communicator.h" +#include "ompi/win/win.h" +#include "ompi/file/file.h" +#include "ompi/instance/instance.h" +#include "ompi/errhandler/errhandler.h" + +#include "ompi/mpi/c/abi.h" +#include "ompi/mpi/c/abi_converters.h" +#include "ompi/mpi/c/mpit_abi_handle_convert.h" + +#include "ompi/runtime/ompi_mpit_events.h" + +uint64_t ompi_mpit_abi_handle_convert_impl(void *object, int handle_kind) +{ + if (NULL == object) { + return 0; + } + + /* handle_kind is a public MPI_T_BIND_* binding constant naming the class of + the object; convert the internal object pointer to the Standard-ABI + integer handle with the matching generated intern->ABI converter, then + widen to uint64_t. Each converter returns a mangled *_ABI_INTERNAL + handle (a pointer-width value carrying either a small reserved-handle + index or the object pointer), so route it through uintptr_t. */ + switch (handle_kind) { + case MPI_T_BIND_MPI_COMM: + return (uint64_t) (uintptr_t) + ompi_convert_comm_ompi_to_standard((ompi_communicator_t *) object); + case MPI_T_BIND_MPI_WIN: + return (uint64_t) (uintptr_t) + ompi_convert_win_ompi_to_standard((ompi_win_t *) object); + case MPI_T_BIND_MPI_SESSION: + return (uint64_t) (uintptr_t) + ompi_convert_session_ompi_to_standard((ompi_instance_t *) object); + case MPI_T_BIND_MPI_ERRHANDLER: + return (uint64_t) (uintptr_t) + ompi_convert_intern_errorhandler_abi_errorhandler( + (ompi_errhandler_t *) object); + case MPI_T_BIND_MPI_FILE: + return (uint64_t) (uintptr_t) + ompi_convert_file_ompi_to_standard((ompi_file_t *) object); + default: + return 0; + } +} \ No newline at end of file diff --git a/ompi/mpi/c/mpit_abi_handle_convert.h b/ompi/mpi/c/mpit_abi_handle_convert.h new file mode 100644 index 00000000000..75cdcd46633 --- /dev/null +++ b/ompi/mpi/c/mpit_abi_handle_convert.h @@ -0,0 +1,37 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ +/* + * Copyright (c) 2026 Triad National Security, LLC. All rights + * reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + * + * Declaration of the MPI Standard ABI handle converter for MPI_T event + * payloads. The implementation lives in mpit_abi_handle_convert.c and is + * compiled only into libmpi_abi. The Standard-ABI init entry points install + * it downward via ompi_mpit_register_abi_handle_convert() so the libopen_mpi + * producer raise sites can reach it without an upward link dependency. + */ + +#ifndef OMPI_MPI_C_MPIT_ABI_HANDLE_CONVERT_H +#define OMPI_MPI_C_MPIT_ABI_HANDLE_CONVERT_H + +#include "ompi_config.h" + +#include + +BEGIN_C_DECLS + +/* Convert an internal MPI object handle (pointer) to its MPI Standard ABI + integer handle, widened to uint64_t. `handle_kind` is a public MPI_T_BIND_* + binding constant (MPI_T_BIND_MPI_COMM / _WIN / _SESSION / _ERRHANDLER / + _FILE). This is the ompi_mpit_abi_handle_convert_fn_t installed via + ompi_mpit_register_abi_handle_convert(). */ +OMPI_DECLSPEC uint64_t ompi_mpit_abi_handle_convert_impl(void *object, + int handle_kind); + +END_C_DECLS + +#endif /* OMPI_MPI_C_MPIT_ABI_HANDLE_CONVERT_H */ \ No newline at end of file diff --git a/ompi/mpi/c/session_init.c.in b/ompi/mpi/c/session_init.c.in index 7f1fc0d4a35..267731781c5 100644 --- a/ompi/mpi/c/session_init.c.in +++ b/ompi/mpi/c/session_init.c.in @@ -17,6 +17,12 @@ #include "ompi/errhandler/errhandler.h" #include "ompi/instance/instance.h" #include "ompi/mpiext/mpiext.h" +#include "ompi/runtime/ompi_mpit_events.h" +/* Declares ompi_mpit_abi_handle_convert_impl(); only *referenced* in the + Standard-ABI compile (guarded by OMPI_ABI_SRC in the body below), but + included unconditionally because OMPI_ABI_SRC is not yet defined this early + in the generated file. */ +#include "ompi/mpi/c/mpit_abi_handle_convert.h" PROTOTYPE ERROR_CLASS session_init (INFO info, ERRHANDLER errhandler, SESSION_OUT session) { @@ -58,6 +64,15 @@ PROTOTYPE ERROR_CLASS session_init (INFO info, ERRHANDLER errhandler, SESSION_OU * through the registered function pointer, avoiding a direct symbol dependency on libmpi. * MPI extensions are only supported in the OMPI ABI, not the MPI Forum ABI. */ ompi_mpi_instance_register_mpiext_init(ompi_mpiext_init); +#else + /* This is the MPI Standard ABI entry point, so a tool observing MPI_T + * events from this process must see Standard-ABI integer handles in event + * payloads. Record the process ABI and install the intern->ABI handle + * converter downward into libopen_mpi (the converter lives here in + * libmpi_abi; see mpit_abi_handle_convert.c), mirroring the mpiext_init + * registration above. */ + ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; + ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); #endif rc = ompi_mpi_instance_init (ts_level, &info->super, errhandler, session, 0, NULL); diff --git a/ompi/mpi/tool/event_register_callback.c.in b/ompi/mpi/tool/event_register_callback.c.in index d39a363c490..525127793ee 100644 --- a/ompi/mpi/tool/event_register_callback.c.in +++ b/ompi/mpi/tool/event_register_callback.c.in @@ -34,13 +34,20 @@ PROTOTYPE ERROR_CLASS event_register_callback (EVENT_REGISTRATION event_registra return MPI_T_ERR_NOT_INITIALIZED; } - /* Record which MPI ABI this registering tool is using, so the producers - emit object handles (MPI_Comm / MPI_Win / MPI_Errhandler / MPI_Session) - in the matching representation. This entry point is the Open MPI ABI one, - so hard-code the Open MPI ABI; when the MPI Standard ABI lands - (open-mpi/ompi#13280) its separate entry point will set - OMPI_MPIT_ABI_STANDARD here instead. */ + /* ompi_mpit_callback_abi records which MPI ABI this process uses, so the + producers emit object handles (MPI_Comm / MPI_Win / MPI_Errhandler / + MPI_Session) in the matching representation. It is a whole-process + property set at init (MPI_Init / MPI_Init_thread / MPI_Session_init), + which also covers events -- such as ompi.mpi.initialization -- raised + before any tool registers a callback. Re-assert it here, ABI-conditioned + on which copy of this template is compiled (open-mpi/ompi#13280), so a + Standard-ABI tool registering after an intervening reinit is still seen + correctly; this is idempotent with the init-time set. */ +#if OMPI_ABI_SRC + ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; +#else ompi_mpit_callback_abi = OMPI_MPIT_ABI_OMPI; +#endif /* MPI_Info is an ompi_info_t whose first member is an opal_info_t. */ opal_info = (MPI_INFO_NULL == info) ? NULL : &info->super; diff --git a/ompi/runtime/ompi_mpit_events.h b/ompi/runtime/ompi_mpit_events.h index b5ac0d9a2b1..d9e5ed16af8 100644 --- a/ompi/runtime/ompi_mpit_events.h +++ b/ompi/runtime/ompi_mpit_events.h @@ -38,12 +38,35 @@ typedef enum { OMPI_MPIT_ABI_STANDARD = 1 /* MPI Standard ABI: handle == integer handle */ } ompi_mpit_abi_t; -/* Hard-coded to the Open MPI ABI for now. When the MPI Standard ABI lands - (open-mpi/ompi#13280), its MPI_T_event_register_callback entry point will set - this to OMPI_MPIT_ABI_STANDARD, and the producers' "else" branches (marked - "TODO ABI") will fill in the Standard-ABI handle values. */ +/* Defaults to the Open MPI ABI. The MPI Standard ABI variants of MPI_Init, + MPI_Init_thread, and MPI_Session_init set this to OMPI_MPIT_ABI_STANDARD + (see open-mpi/ompi#13280), which makes the producer raise sites publish + Standard-ABI integer handle values instead of internal object pointers. */ OMPI_DECLSPEC extern ompi_mpit_abi_t ompi_mpit_callback_abi; +/* Convert an internal MPI object handle to the value an MPI Standard ABI + MPI_T tool expects to see in an event payload. `object` is the internal + object pointer (ompi_communicator_t *, ompi_win_t *, ompi_instance_t *, + ompi_errhandler_t *, ompi_file_t *); `handle_kind` selects which object + class it is, using the public MPI_T_BIND_* binding constants + (MPI_T_BIND_MPI_COMM / _WIN / _SESSION / _ERRHANDLER / _FILE). Returns the + Standard-ABI integer handle widened to uint64_t. + + The intern->ABI converters live in libmpi_abi (the upper layer); the raise + sites live in libopen_mpi (the lower layer), which must not depend upward. + So the Standard-ABI init path installs this converter downward via + ompi_mpit_register_abi_handle_convert(), mirroring + ompi_mpi_instance_register_mpiext_init(). The raise sites call + ompi_mpit_abi_handle(), which forwards to the registered converter (or + returns 0 if none was registered -- the same fallback as the old stub). */ +typedef uint64_t (*ompi_mpit_abi_handle_convert_fn_t)(void *object, + int handle_kind); + +OMPI_DECLSPEC void ompi_mpit_register_abi_handle_convert( + ompi_mpit_abi_handle_convert_fn_t fn); + +OMPI_DECLSPEC uint64_t ompi_mpit_abi_handle(void *object, int handle_kind); + /* Event type handles for the in-tree producers. NULL until (and unless) the producers are registered, so a raise site must NULL-check before raising. */ OMPI_DECLSPEC extern mca_base_event_t *ompi_event_comm_created; diff --git a/ompi/runtime/ompi_mpit_register_events.c b/ompi/runtime/ompi_mpit_register_events.c index dd91b9118be..b72b5234574 100644 --- a/ompi/runtime/ompi_mpit_register_events.c +++ b/ompi/runtime/ompi_mpit_register_events.c @@ -23,10 +23,33 @@ #include "ompi/runtime/ompi_mpit_events.h" -/* The MPI ABI of the registering MPI_T tool (process-global; see the header). - Hard-coded to the Open MPI ABI until open-mpi/ompi#13280. */ +/* The MPI ABI of the running process (process-global; see the header). + Defaults to the Open MPI ABI; the MPI Standard ABI init entry points set it + to OMPI_MPIT_ABI_STANDARD (open-mpi/ompi#13280). */ ompi_mpit_abi_t ompi_mpit_callback_abi = OMPI_MPIT_ABI_OMPI; +/* Downward-installed converter from an internal object handle to the MPI + Standard ABI integer handle a Standard-ABI MPI_T tool expects in an event + payload. NULL under the Open MPI ABI (never consulted there); installed by + the Standard-ABI init path via ompi_mpit_register_abi_handle_convert(). See + the header for why this indirection is required (library layering). */ +static ompi_mpit_abi_handle_convert_fn_t ompi_mpit_abi_handle_convert_fn = NULL; + +void ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_fn_t fn) +{ + ompi_mpit_abi_handle_convert_fn = fn; +} + +uint64_t ompi_mpit_abi_handle(void *object, int handle_kind) +{ + if (NULL != ompi_mpit_abi_handle_convert_fn) { + return ompi_mpit_abi_handle_convert_fn(object, handle_kind); + } + /* No converter registered: fall back to 0 rather than publish an internal + pointer to a Standard-ABI tool (matches the old TODO-ABI stub). */ + return 0; +} + mca_base_event_t *ompi_event_comm_created = NULL; mca_base_event_t *ompi_event_comm_freed = NULL; mca_base_event_t *ompi_event_comm_name_set = NULL; diff --git a/ompi/test/mpi-abi/Makefile.am b/ompi/test/mpi-abi/Makefile.am index 0a586e6b37d..55338579ba6 100644 --- a/ompi/test/mpi-abi/Makefile.am +++ b/ompi/test/mpi-abi/Makefile.am @@ -68,6 +68,8 @@ C_CALLBACK_CASES = \ cases/c-callback/callback_comm_attr.prologue.in \ cases/c-callback/callback_datarep.cbody.in \ cases/c-callback/callback_datarep.prologue.in \ + cases/c-callback/callback_mpit_event_handle.cbody.in \ + cases/c-callback/callback_mpit_event_handle.prologue.in \ cases/c-callback/callback_file_errhandler.cbody.in \ cases/c-callback/callback_file_errhandler.prologue.in \ cases/c-callback/callback_grequest.cbody.in \ diff --git a/ompi/test/mpi-abi/_abi_tables.py b/ompi/test/mpi-abi/_abi_tables.py index 82f86f0e59b..3a0e4fa3b1c 100644 --- a/ompi/test/mpi-abi/_abi_tables.py +++ b/ompi/test/mpi-abi/_abi_tables.py @@ -1405,6 +1405,36 @@ "prologue_file": "cases/c-callback/callback_mpit_events.prologue.in", "body_file": "cases/c-callback/callback_mpit_events.cbody.in", }, + { + "name": "callback_mpit_event_handle", + "family": "callback_mpit", + "rank_count": 1, + "api_names": ( + "MPI_Comm_dup", + "MPI_Comm_free", + "MPI_T_event_handle_alloc", + "MPI_T_event_handle_free", + "MPI_T_event_read", + "MPI_T_event_register_callback", + ), + "support_api_names": ( + "MPI_Finalize", + "MPI_Info_free", + "MPI_Init", + "MPI_T_event_get_info", + "MPI_T_event_get_num", + "MPI_T_finalize", + "MPI_T_init_thread", + ), + "requires_feature": "mpit_events", + "skip_exit_codes": { + 77: SKIP_MPIT_EVENTS_UNAVAILABLE, + }, + "prologue_file": + "cases/c-callback/callback_mpit_event_handle.prologue.in", + "body_file": + "cases/c-callback/callback_mpit_event_handle.cbody.in", + }, { "name": "lifetime_nonblocking_collective_arrays", "family": "callback_lifetime", diff --git a/ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.cbody.in b/ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.cbody.in new file mode 100644 index 00000000000..f446da623f5 --- /dev/null +++ b/ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.cbody.in @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2026 Triad National Security, LLC. All rights + * reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + */ + +/* + * Test intent: + * Drive the ompi.mpi.communicator_created event under the MPI Standard + * ABI and confirm its payload carries the Standard-ABI integer MPI_Comm + * handle (open-mpi/ompi#13280). Skips stably (77) when the event APIs + * are unavailable, the event is not registered, or no registration can + * be allocated. + */ +int ret = MPI_Init(&argc, &argv); +if (MPI_SUCCESS != ret) { + return 1; +} + +int provided = 0; +ret = MPI_T_init_thread(MPI_THREAD_SINGLE, &provided); +if (MPI_T_ERR_NOT_SUPPORTED == ret) { + (void) MPI_Finalize(); + return 77; +} +if (MPI_SUCCESS != ret) { + (void) MPI_Finalize(); + return 1; +} + +/* Locate the communicator-created event by name. */ +int num_events = 0; +ret = MPI_T_event_get_num(&num_events); +if (MPI_SUCCESS != ret || num_events < 1) { + (void) MPI_T_finalize(); + (void) MPI_Finalize(); + return 77; +} + +int created_index = -1; +for (int event_index = 0; event_index < num_events; ++event_index) { + char name[MPI_MAX_OBJECT_NAME]; + int name_len = (int) sizeof(name); + int verbosity = 0; + MPI_Datatype datatypes[4]; + MPI_Aint displacements[4]; + int num_elements = 4; + MPI_T_enum enumtype = MPI_T_ENUM_NULL; + MPI_Info info = MPI_INFO_NULL; + char desc[MPI_MAX_INFO_VAL]; + int desc_len = (int) sizeof(desc); + int bind = MPI_T_BIND_NO_OBJECT; + + ret = MPI_T_event_get_info(event_index, name, &name_len, &verbosity, + datatypes, displacements, &num_elements, + &enumtype, &info, desc, &desc_len, &bind); + if (MPI_SUCCESS != ret) { + continue; + } + if (MPI_INFO_NULL != info) { + (void) MPI_Info_free(&info); + } + if (NULL != strstr(name, "communicator_created")) { + created_index = event_index; + break; + } +} + +if (created_index < 0) { + (void) MPI_T_finalize(); + (void) MPI_Finalize(); + return 77; +} + +mpit_created_state.registration = (MPI_T_event_registration) 0; +mpit_created_state.calls = 0; +mpit_created_state.failure_code = 0; +mpit_created_state.handle = 0; + +ret = MPI_T_event_handle_alloc(created_index, NULL, MPI_INFO_NULL, + &mpit_created_state.registration); +if (MPI_SUCCESS != ret || + (MPI_T_event_registration) 0 == mpit_created_state.registration) { + (void) MPI_T_finalize(); + (void) MPI_Finalize(); + return 77; +} + +ret = MPI_T_event_register_callback(mpit_created_state.registration, + MPI_T_CB_REQUIRE_NONE, MPI_INFO_NULL, + &mpit_created_state, + mpit_created_callback); +if (MPI_SUCCESS != ret) { + (void) MPI_T_event_handle_free(mpit_created_state.registration, NULL, + NULL); + (void) MPI_T_finalize(); + (void) MPI_Finalize(); + return 2; +} + +/* Create a communicator; this must raise communicator_created and deliver + the new communicator's Standard-ABI handle in payload element 1. */ +MPI_Comm dup = MPI_COMM_NULL; +ret = MPI_Comm_dup(MPI_COMM_SELF, &dup); +if (MPI_SUCCESS != ret || MPI_COMM_NULL == dup) { + (void) MPI_T_event_handle_free(mpit_created_state.registration, NULL, + NULL); + (void) MPI_T_finalize(); + (void) MPI_Finalize(); + return 3; +} + +int local_failure = 0; +if (0 != mpit_created_state.failure_code) { + local_failure = 4; +} else if (mpit_created_state.calls < 1) { + local_failure = 5; +} else if (mpit_created_state.handle != + (uint64_t) (uintptr_t) dup) { + /* The payload must carry the Standard-ABI integer handle the + application holds for the new communicator, not 0 or an internal + pointer. */ + fprintf(stderr, "ABI_FAIL:mpit:created_handle:%llu:%llu\n", + (unsigned long long) mpit_created_state.handle, + (unsigned long long) (uintptr_t) dup); + local_failure = 6; +} + +ret = MPI_Comm_free(&dup); +if (MPI_SUCCESS != ret && 0 == local_failure) { + local_failure = 7; +} + +ret = MPI_T_event_handle_free(mpit_created_state.registration, NULL, NULL); +if (MPI_SUCCESS != ret && 0 == local_failure) { + local_failure = 8; +} + +ret = MPI_T_finalize(); +if (MPI_SUCCESS != ret && 0 == local_failure) { + local_failure = 9; +} + +ret = MPI_Finalize(); +if (MPI_SUCCESS != ret && 0 == local_failure) { + local_failure = 10; +} + +if (0 != local_failure) { + return local_failure; +} \ No newline at end of file diff --git a/ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.prologue.in b/ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.prologue.in new file mode 100644 index 00000000000..e81db398ddc --- /dev/null +++ b/ompi/test/mpi-abi/cases/c-callback/callback_mpit_event_handle.prologue.in @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2026 Triad National Security, LLC. All rights + * reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + */ + +/* + * Test intent: + * Verify that Open MPI's MPI_T communicator-created event publishes the + * MPI *Standard ABI* integer handle in its payload when the process runs + * under the Standard ABI (open-mpi/ompi#13280), not an internal object + * pointer or the old zero stub. A Standard-ABI process links libmpi_abi, + * whose MPI_Init installs the intern->ABI handle converter downward into + * libopen_mpi; the producer raise site then converts the communicator to + * the Standard-ABI handle. + * + * For a freshly created (non-predefined) communicator, the Standard-ABI + * handle the application holds is exactly the value the converter yields, + * so the payload element must compare equal to the user's MPI_Comm handle. + * Open MPI may expose the event APIs with no source able to allocate a + * registration, or no communicator-created event; the body treats either + * as a stable skip. + */ +struct mpit_created_state { + MPI_T_event_registration registration; + int calls; + int failure_code; + uint64_t handle; +}; + +static struct mpit_created_state mpit_created_state; + +static void mpit_created_callback(MPI_T_event_instance event_instance, + MPI_T_event_registration event_registration, + MPI_T_cb_safety cb_safety, + void *user_data) +{ + struct mpit_created_state *state = + (struct mpit_created_state *) user_data; + if (&mpit_created_state != state) { + mpit_created_state.failure_code = 201; + } else if (event_registration != state->registration) { + state->failure_code = 202; + } else if (MPI_T_CB_REQUIRE_NONE != cb_safety) { + state->failure_code = 203; + } else { + ++state->calls; + /* Element 1 of the communicator_created payload is the MPI_Comm + handle (element 0 is the int32 size). */ + (void) MPI_T_event_read(event_instance, 1, &state->handle); + } +} \ No newline at end of file diff --git a/ompi/win/win.c b/ompi/win/win.c index 9efe6b16522..3d9981692da 100644 --- a/ompi/win/win.c +++ b/ompi/win/win.c @@ -271,9 +271,8 @@ config_window(void *base, size_t size, ptrdiff_t disp_unit, if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.handle = (uint64_t) (uintptr_t) win; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle value for the - window win. */ - payload.handle = 0; + /* MPI Standard ABI: publish the integer MPI_Win handle. */ + payload.handle = ompi_mpit_abi_handle(win, MPI_T_BIND_MPI_WIN); } mca_base_event_raise(ompi_event_win_created, NULL, &payload); } @@ -440,9 +439,8 @@ ompi_win_free(ompi_win_t *win) if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.handle = (uint64_t) (uintptr_t) win; } else { - /* TODO ABI (#13280): set the MPI Standard ABI handle value for - the window win. */ - payload.handle = 0; + /* MPI Standard ABI: publish the integer MPI_Win handle. */ + payload.handle = ompi_mpit_abi_handle(win, MPI_T_BIND_MPI_WIN); } mca_base_event_raise(ompi_event_win_freed, NULL, &payload); } diff --git a/specs/mpi-t-events/spec.md b/specs/mpi-t-events/spec.md index 91181f12347..95fb1030c32 100644 --- a/specs/mpi-t-events/spec.md +++ b/specs/mpi-t-events/spec.md @@ -501,12 +501,31 @@ bound-object resolution (sec. 6.1), because both must match the ABI of the tool that registered the callback: - A process-global flag `ompi_mpit_callback_abi` - (`OMPI_MPIT_ABI_OMPI` / `_STANDARD`) records which ABI the registering - tool uses. It is set by `MPI_T_event_register_callback` and is, for now, - hard-coded to the Open MPI ABI. + (`OMPI_MPIT_ABI_OMPI` / `_STANDARD`) records which ABI the process uses. + It defaults to the Open MPI ABI and is set to `OMPI_MPIT_ABI_STANDARD` + by the Standard-ABI variants of `MPI_Init`, `MPI_Init_thread`, and + `MPI_Session_init` (and re-asserted in the Standard-ABI copy of + `MPI_T_event_register_callback`). Keying off process init -- rather than + callback registration alone -- also covers the world-model + `ompi.mpi.initialization` event, which is raised during `MPI_Init` + before any tool can register a callback. - Each handle-emitting raise site branches on this flag. The Open MPI ABI - branch emits the internal pointer; the Standard ABI branch is a marked - `TODO ABI` stub (handle `0`) that #13280 will fill in. + branch emits the internal object pointer; the Standard ABI branch emits + the Standard-ABI integer handle. The intern->ABI converters + (`ompi_convert_comm_ompi_to_standard()` and friends, generated into + `abi_converters.h`) live in `libmpi_abi`, a *higher* layer than the + raise sites in `libopen_mpi`; a direct upward call would violate the + OPAL->OMPI linker boundary. The Standard-ABI init path therefore + installs a converter downward via + `ompi_mpit_register_abi_handle_convert()` (a function pointer stored in + `libopen_mpi`, mirroring `ompi_mpi_instance_register_mpiext_init()`), + and the raise sites call it through `ompi_mpit_abi_handle()`. The + converter itself is `ompi_mpit_abi_handle_convert_impl()` in + `ompi/mpi/c/mpit_abi_handle_convert.c` (compiled only into + `libmpi_abi`), which dispatches on the object's `MPI_T_BIND_*` kind. + When no converter is registered (e.g. the Open MPI ABI, which never + installs one), `ompi_mpit_abi_handle()` returns `0` rather than a wrong + pointer value. - Bound-object resolution (sec. 6.1) dereferences the tool's `obj_handle` to the internal identity; under the Open MPI ABI the MPI handle *is* that pointer, so a single deref suffices. The Standard ABI must convert @@ -535,6 +554,12 @@ producer's responsibility, hence the per-raise-site branch. `MPI_Comm_set_name` (`mpi_t_event_comm_name.c`, confirming bound delivery isolation, the payload handle, and name re-query); plus smoke/self/reinit/inert tests. +- **MPI Standard ABI coverage** (`ompi/test/mpi-abi/`, run by the ABI + harness): the `callback_mpit_event_handle` probe drives the + `ompi.mpi.communicator_created` event under a `libmpi_abi`-linked + process and asserts the payload handle is the Standard-ABI integer + handle of the new communicator (sec. 10), exercising the downward + converter registration and the raise-site conversion end to end. --- From a4e2a2b77ef44c7b3deed0d72d5f86273c8eb147 Mon Sep 17 00:00:00 2001 From: Howard Pritchard Date: Mon, 17 Aug 2026 11:56:05 -0600 Subject: [PATCH 2/3] pr feedback Signed-off-by: Howard Pritchard --- docs/release-notes/changelog/v6.1.x.rst | 7 ------- 1 file changed, 7 deletions(-) diff --git a/docs/release-notes/changelog/v6.1.x.rst b/docs/release-notes/changelog/v6.1.x.rst index 3190c89800d..b712b142145 100644 --- a/docs/release-notes/changelog/v6.1.x.rst +++ b/docs/release-notes/changelog/v6.1.x.rst @@ -36,13 +36,6 @@ Open MPI version v6.1.0 operation using the same communicator, peer, and tag, which then never completed. -- MPI_T event producers now emit the correct MPI object handle - representation for the tool's MPI ABI. When a process runs under the - MPI Standard ABI, the communicator, window, session, error-handler, - and file handles carried in MPI_T event payloads are now the Standard - ABI integer handles instead of a placeholder value; under the Open MPI - ABI they remain the internal object handle as before. - - Renamed the ``--enable-weak-symbols`` configure option to ``--enable-weak-aliases``, which more accurately reflects the linker feature (weak symbol *aliases*) that Open MPI actually tests for and From b26d9a1a8ee5fe5de5f8c926bbba2d1379231da6 Mon Sep 17 00:00:00 2001 From: Howard Pritchard Date: Tue, 18 Aug 2026 12:41:33 -0600 Subject: [PATCH 3/3] Convert all ABI-sensitive MPI_T event payload fields and close pre-init gaps The initial Standard-ABI MPI_T event work converted object handles in event payloads but left several gaps that a Standard-ABI MPI_T tool would observe as wrong values or missed deliveries. Address the review feedback on that work. Install converters before Session_init error exits. In the Standard-ABI copy of MPI_Session_init the process-ABI flag and the handle converter were installed after the MPI_PARAM_CHECK branches that jump to fn_exit. A failing check routes through OMPI_ERRHANDLER_RETURN on MPI_SESSION_NULL, which can invoke an error handler and raise ompi.mpi.errhandler_invoked before any converter is installed, publishing 0 for handles such as the predefined error handler. Move the whole registration block to the top of the function, ahead of all early error exits. Install converters in the Standard-ABI event_register_callback. MPI_T is init-independent: a tool may call MPI_T_init_thread and MPI_T_event_register_callback before MPI_Init, and events can be raised in that window. The re-assert of the process-ABI flag was already here, but the converters were installed only by the init entry points, so pre-init events fell back to the NULL-converter (handle 0). Install the converters here too, under the same OMPI_ABI_SRC guard; this Standard-ABI copy is compiled into libmpi_abi alongside the converter implementations, and the registration is idempotent with the init-time set. Convert the err_code and object_type payload fields, not just handles. The errhandler_invoked payload also carries an MPI error code and an MPI_T_BIND_* object-binding kind, whose numeric encodings differ between the Open MPI ABI and the MPI Standard ABI (roughly a third of the MPI_ERR_* space differs, and the Standard-ABI MPI_T_BIND_* values are the internal values + 1). These were left in internal representation, so a Standard-ABI tool misread them. The generated value converters live in libmpi_abi (the upper layer), so plumb them downward the same way as the handle converter: add ompi_mpit_register_abi_error_convert() / ompi_mpit_register_abi_bind_convert() function-pointer slots in libopen_mpi and ompi_mpit_abi_error() / ompi_mpit_abi_bind() forwarders (which return the value unchanged when unregistered, correct for the Open MPI ABI). The implementations wrap the generated ompi_convert_intern_error_abi_error() and ompi_convert_t_bind_ompi_to_standard(). All Standard-ABI init entry points and the event_register_callback copy install them, and the errhandler_invoke raise site converts both fields. Document the remaining binding-side gap. Object-bound delivery still lacks a Standard-ABI-to-internal conversion of the tool's obj_handle in event_handle_alloc.c.in, so predefined handles such as MPI_COMM_WORLD never match under the Standard ABI. Record this as deferred work in the MPI_T events spec and qualify the "end to end" test-coverage claim accordingly. Signed-off-by: Howard Pritchard --- ompi/errhandler/errhandler_invoke.c | 22 +++++++---- ompi/mpi/c/init.c.in | 2 + ompi/mpi/c/init_thread.c.in | 2 + ompi/mpi/c/mpit_abi_handle_convert.c | 14 ++++++- ompi/mpi/c/mpit_abi_handle_convert.h | 10 +++++ ompi/mpi/c/session_init.c.in | 43 +++++++++++++--------- ompi/mpi/tool/event_register_callback.c.in | 18 ++++++++- ompi/runtime/ompi_mpit_events.h | 20 ++++++++++ ompi/runtime/ompi_mpit_register_events.c | 38 +++++++++++++++++++ specs/mpi-t-events/spec.md | 40 ++++++++++++++++++-- 10 files changed, 179 insertions(+), 30 deletions(-) diff --git a/ompi/errhandler/errhandler_invoke.c b/ompi/errhandler/errhandler_invoke.c index 1b0e5bf3582..d18516a4c70 100644 --- a/ompi/errhandler/errhandler_invoke.c +++ b/ompi/errhandler/errhandler_invoke.c @@ -72,18 +72,24 @@ int ompi_errhandler_invoke(ompi_errhandler_t *errhandler, void *mpi_object, payload.err_code = (int32_t) err_code; payload.object_type = object_bind; /* The MPI_Errhandler handle and the invoking object's handle must match - the registering MPI_T tool's ABI (ompi_mpit_callback_abi). */ + the registering MPI_T tool's ABI (ompi_mpit_callback_abi). So must + the err_code and object_type integer values, whose encodings differ + between the two ABIs. */ if (OMPI_MPIT_ABI_OMPI == ompi_mpit_callback_abi) { payload.errhandler_handle = (uint64_t) (uintptr_t) errhandler; payload.object_handle = (uint64_t) (uintptr_t) mpi_object; } else { - /* MPI Standard ABI: publish integer handles. The errhandler is - converted as an MPI_Errhandler; the invoking object is converted - per its binding kind (MPI_Comm / MPI_Win / MPI_File / - MPI_Session). An object with no binding (e.g. a predefined - handler routed before MPI_INIT) has object_bind == NO_OBJECT and - a 0 object_handle, matching the Open MPI ABI's "0 when not - available" behavior. */ + /* MPI Standard ABI: publish integer handles and Standard-ABI + integer values. The errhandler is converted as an + MPI_Errhandler; the invoking object is converted per its binding + kind (MPI_Comm / MPI_Win / MPI_File / MPI_Session). An object + with no binding (e.g. a predefined handler routed before + MPI_INIT) has object_bind == NO_OBJECT and a 0 object_handle, + matching the Open MPI ABI's "0 when not available" behavior. The + err_code and object_type integer values are remapped to their + Standard-ABI encodings. */ + payload.err_code = ompi_mpit_abi_error((int32_t) err_code); + payload.object_type = ompi_mpit_abi_bind(object_bind); payload.errhandler_handle = ompi_mpit_abi_handle(errhandler, MPI_T_BIND_MPI_ERRHANDLER); payload.object_handle diff --git a/ompi/mpi/c/init.c.in b/ompi/mpi/c/init.c.in index f4920366626..11f7260904e 100644 --- a/ompi/mpi/c/init.c.in +++ b/ompi/mpi/c/init.c.in @@ -70,6 +70,8 @@ PROTOTYPE INT init(INT_OUT argc, ARGV argv) * registration above. */ ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); + ompi_mpit_register_abi_error_convert(ompi_mpit_abi_error_convert_impl); + ompi_mpit_register_abi_bind_convert(ompi_mpit_abi_bind_convert_impl); #endif /* Call the back-end initialization function (we need to put as diff --git a/ompi/mpi/c/init_thread.c.in b/ompi/mpi/c/init_thread.c.in index e5a5f82808d..d78ebcec3b4 100644 --- a/ompi/mpi/c/init_thread.c.in +++ b/ompi/mpi/c/init_thread.c.in @@ -90,6 +90,8 @@ PROTOTYPE ERROR_CLASS init_thread(INT_OUT argc, ARGV argv, TS_LEVEL required, * registration above. */ ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); + ompi_mpit_register_abi_error_convert(ompi_mpit_abi_error_convert_impl); + ompi_mpit_register_abi_bind_convert(ompi_mpit_abi_bind_convert_impl); #endif /* Call the back-end initialization function (we need to put as diff --git a/ompi/mpi/c/mpit_abi_handle_convert.c b/ompi/mpi/c/mpit_abi_handle_convert.c index 18979f9cb86..6f99d344490 100644 --- a/ompi/mpi/c/mpit_abi_handle_convert.c +++ b/ompi/mpi/c/mpit_abi_handle_convert.c @@ -74,4 +74,16 @@ uint64_t ompi_mpit_abi_handle_convert_impl(void *object, int handle_kind) default: return 0; } -} \ No newline at end of file +} + +int32_t ompi_mpit_abi_error_convert_impl(int32_t err_code) +{ + /* Map an internal MPI error code to its MPI Standard ABI value. */ + return (int32_t) ompi_convert_intern_error_abi_error((int) err_code); +} + +int32_t ompi_mpit_abi_bind_convert_impl(int32_t object_bind) +{ + /* Map an internal MPI_T_BIND_* value to its MPI Standard ABI value. */ + return (int32_t) ompi_convert_t_bind_ompi_to_standard((int) object_bind); +} diff --git a/ompi/mpi/c/mpit_abi_handle_convert.h b/ompi/mpi/c/mpit_abi_handle_convert.h index 75cdcd46633..dd764d0e30d 100644 --- a/ompi/mpi/c/mpit_abi_handle_convert.h +++ b/ompi/mpi/c/mpit_abi_handle_convert.h @@ -32,6 +32,16 @@ BEGIN_C_DECLS OMPI_DECLSPEC uint64_t ompi_mpit_abi_handle_convert_impl(void *object, int handle_kind); +/* Convert an internal MPI error code to its MPI Standard ABI value. This is + the ompi_mpit_abi_value_convert_fn_t installed via + ompi_mpit_register_abi_error_convert(). */ +OMPI_DECLSPEC int32_t ompi_mpit_abi_error_convert_impl(int32_t err_code); + +/* Convert an internal MPI_T_BIND_* binding kind to its MPI Standard ABI value. + This is the ompi_mpit_abi_value_convert_fn_t installed via + ompi_mpit_register_abi_bind_convert(). */ +OMPI_DECLSPEC int32_t ompi_mpit_abi_bind_convert_impl(int32_t object_bind); + END_C_DECLS #endif /* OMPI_MPI_C_MPIT_ABI_HANDLE_CONVERT_H */ \ No newline at end of file diff --git a/ompi/mpi/c/session_init.c.in b/ompi/mpi/c/session_init.c.in index 267731781c5..213e0af1a23 100644 --- a/ompi/mpi/c/session_init.c.in +++ b/ompi/mpi/c/session_init.c.in @@ -31,6 +31,32 @@ PROTOTYPE ERROR_CLASS session_init (INFO info, ERRHANDLER errhandler, SESSION_OU opal_cstring_t *info_value; const char ts_level_multi[] = "MPI_THREAD_MULTIPLE"; +#if !OMPI_ABI_SRC + /* Register the mpiext initialization function with libopen_mpi. + * This breaks the circular dependency: libopen_mpi calls ompi_mpiext_init indirectly + * through the registered function pointer, avoiding a direct symbol dependency on libmpi. + * MPI extensions are only supported in the OMPI ABI, not the MPI Forum ABI. */ + ompi_mpi_instance_register_mpiext_init(ompi_mpiext_init); +#else + /* This is the MPI Standard ABI entry point, so a tool observing MPI_T + * events from this process must see Standard-ABI integer handles and + * Standard-ABI integer values in event payloads. Record the process ABI + * and install the intern->ABI converters downward into libopen_mpi (the + * converters live here in libmpi_abi; see mpit_abi_handle_convert.c), + * mirroring the mpiext_init registration above. + * + * This is done up front, before the parameter-validation branches below + * that jump to fn_exit: a failing check routes through OMPI_ERRHANDLER_RETURN + * on MPI_SESSION_NULL, which can invoke an error handler and raise the + * ompi.mpi.errhandler_invoked event. Installing the converters first + * ensures that event publishes correct Standard-ABI values (e.g. for the + * predefined error handler) instead of the 0 / internal fallbacks. */ + ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; + ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); + ompi_mpit_register_abi_error_convert(ompi_mpit_abi_error_convert_impl); + ompi_mpit_register_abi_bind_convert(ompi_mpit_abi_bind_convert_impl); +#endif + if ( MPI_PARAM_CHECK ) { if (NULL == errhandler) { rc = MPI_ERR_ERRHANDLER; @@ -58,23 +84,6 @@ PROTOTYPE ERROR_CLASS session_init (INFO info, ERRHANDLER errhandler, SESSION_OU } } -#if !OMPI_ABI_SRC - /* Register the mpiext initialization function with libopen_mpi. - * This breaks the circular dependency: libopen_mpi calls ompi_mpiext_init indirectly - * through the registered function pointer, avoiding a direct symbol dependency on libmpi. - * MPI extensions are only supported in the OMPI ABI, not the MPI Forum ABI. */ - ompi_mpi_instance_register_mpiext_init(ompi_mpiext_init); -#else - /* This is the MPI Standard ABI entry point, so a tool observing MPI_T - * events from this process must see Standard-ABI integer handles in event - * payloads. Record the process ABI and install the intern->ABI handle - * converter downward into libopen_mpi (the converter lives here in - * libmpi_abi; see mpit_abi_handle_convert.c), mirroring the mpiext_init - * registration above. */ - ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; - ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); -#endif - rc = ompi_mpi_instance_init (ts_level, &info->super, errhandler, session, 0, NULL); /* if an error occurred raise it on the null session */ fn_exit: diff --git a/ompi/mpi/tool/event_register_callback.c.in b/ompi/mpi/tool/event_register_callback.c.in index 525127793ee..2affecd9352 100644 --- a/ompi/mpi/tool/event_register_callback.c.in +++ b/ompi/mpi/tool/event_register_callback.c.in @@ -19,6 +19,10 @@ #include "ompi/mpi/tool/mpit-internal.h" #include "ompi/info/info.h" #include "ompi/runtime/ompi_mpit_events.h" +/* Declares the intern->ABI converters installed below in the Standard-ABI + compile (guarded by OMPI_ABI_SRC). Included unconditionally because + OMPI_ABI_SRC is not yet defined this early in the generated file. */ +#include "ompi/mpi/c/mpit_abi_handle_convert.h" PROTOTYPE ERROR_CLASS event_register_callback (EVENT_REGISTRATION event_registration, CB_SAFETY cb_safety, INFO info, VOID user_data, @@ -42,9 +46,21 @@ PROTOTYPE ERROR_CLASS event_register_callback (EVENT_REGISTRATION event_registra before any tool registers a callback. Re-assert it here, ABI-conditioned on which copy of this template is compiled (open-mpi/ompi#13280), so a Standard-ABI tool registering after an intervening reinit is still seen - correctly; this is idempotent with the init-time set. */ + correctly; this is idempotent with the init-time set. + + MPI_T is init-independent: a Standard-ABI tool may call + MPI_T_init_thread + MPI_T_event_register_callback before MPI_Init, and + events (notably ompi.mpi.errhandler_invoked for an error routed to a + predefined handler) can be raised in that pre-init window. The init + entry points install the intern->ABI converters, but they have not run + yet here, so install them too (this Standard-ABI copy is compiled into + libmpi_abi alongside the converter implementations). This is idempotent + with the init-time registration. */ #if OMPI_ABI_SRC ompi_mpit_callback_abi = OMPI_MPIT_ABI_STANDARD; + ompi_mpit_register_abi_handle_convert(ompi_mpit_abi_handle_convert_impl); + ompi_mpit_register_abi_error_convert(ompi_mpit_abi_error_convert_impl); + ompi_mpit_register_abi_bind_convert(ompi_mpit_abi_bind_convert_impl); #else ompi_mpit_callback_abi = OMPI_MPIT_ABI_OMPI; #endif diff --git a/ompi/runtime/ompi_mpit_events.h b/ompi/runtime/ompi_mpit_events.h index d9e5ed16af8..5195e1a163c 100644 --- a/ompi/runtime/ompi_mpit_events.h +++ b/ompi/runtime/ompi_mpit_events.h @@ -67,6 +67,26 @@ OMPI_DECLSPEC void ompi_mpit_register_abi_handle_convert( OMPI_DECLSPEC uint64_t ompi_mpit_abi_handle(void *object, int handle_kind); +/* Some event payloads also carry integer values whose numeric encoding differs + between the Open MPI ABI and the MPI Standard ABI -- notably MPI error codes + (roughly a third of the MPI_ERR_* space differs) and the MPI_T_BIND_* object + binding kind (the Standard-ABI values are the internal values + 1). These + value converters live in libmpi_abi (the upper layer) just like the handle + converter, so they are installed downward the same way. The raise sites call + ompi_mpit_abi_error() / ompi_mpit_abi_bind(), which forward to the registered + converter, or return the value unchanged if none was registered (the Open MPI + ABI never installs one, and there the internal encoding is what the tool + expects). */ +typedef int32_t (*ompi_mpit_abi_value_convert_fn_t)(int32_t value); + +OMPI_DECLSPEC void ompi_mpit_register_abi_error_convert( + ompi_mpit_abi_value_convert_fn_t fn); +OMPI_DECLSPEC void ompi_mpit_register_abi_bind_convert( + ompi_mpit_abi_value_convert_fn_t fn); + +OMPI_DECLSPEC int32_t ompi_mpit_abi_error(int32_t err_code); +OMPI_DECLSPEC int32_t ompi_mpit_abi_bind(int32_t object_bind); + /* Event type handles for the in-tree producers. NULL until (and unless) the producers are registered, so a raise site must NULL-check before raising. */ OMPI_DECLSPEC extern mca_base_event_t *ompi_event_comm_created; diff --git a/ompi/runtime/ompi_mpit_register_events.c b/ompi/runtime/ompi_mpit_register_events.c index b72b5234574..732d8fec721 100644 --- a/ompi/runtime/ompi_mpit_register_events.c +++ b/ompi/runtime/ompi_mpit_register_events.c @@ -50,6 +50,44 @@ uint64_t ompi_mpit_abi_handle(void *object, int handle_kind) return 0; } +/* Downward-installed value converters for the payload elements whose numeric + encoding differs between the two ABIs (MPI error codes and MPI_T_BIND_* + binding kinds). NULL under the Open MPI ABI, where the internal encoding is + already what the tool expects; installed by the Standard-ABI init path. See + the header. */ +static ompi_mpit_abi_value_convert_fn_t ompi_mpit_abi_error_convert_fn = NULL; +static ompi_mpit_abi_value_convert_fn_t ompi_mpit_abi_bind_convert_fn = NULL; + +void ompi_mpit_register_abi_error_convert(ompi_mpit_abi_value_convert_fn_t fn) +{ + ompi_mpit_abi_error_convert_fn = fn; +} + +void ompi_mpit_register_abi_bind_convert(ompi_mpit_abi_value_convert_fn_t fn) +{ + ompi_mpit_abi_bind_convert_fn = fn; +} + +int32_t ompi_mpit_abi_error(int32_t err_code) +{ + if (NULL != ompi_mpit_abi_error_convert_fn) { + return ompi_mpit_abi_error_convert_fn(err_code); + } + /* No converter registered (Open MPI ABI): the internal encoding is what the + tool expects. */ + return err_code; +} + +int32_t ompi_mpit_abi_bind(int32_t object_bind) +{ + if (NULL != ompi_mpit_abi_bind_convert_fn) { + return ompi_mpit_abi_bind_convert_fn(object_bind); + } + /* No converter registered (Open MPI ABI): the internal encoding is what the + tool expects. */ + return object_bind; +} + mca_base_event_t *ompi_event_comm_created = NULL; mca_base_event_t *ompi_event_comm_freed = NULL; mca_base_event_t *ompi_event_comm_name_set = NULL; diff --git a/specs/mpi-t-events/spec.md b/specs/mpi-t-events/spec.md index 95fb1030c32..5e15c0505ac 100644 --- a/specs/mpi-t-events/spec.md +++ b/specs/mpi-t-events/spec.md @@ -526,10 +526,39 @@ the tool that registered the callback: When no converter is registered (e.g. the Open MPI ABI, which never installs one), `ompi_mpit_abi_handle()` returns `0` rather than a wrong pointer value. +- Some payloads also carry integer *values* whose numeric encoding + differs between the two ABIs: MPI error codes (roughly a third of the + `MPI_ERR_*` space) and the `MPI_T_BIND_*` object-binding kind (the + Standard-ABI values are the internal values + 1). The + `ompi.mpi.errhandler_invoked` payload carries both (`err_code`, + `object_type`). These are handled the same way as handles: the + Standard-ABI init path installs value converters downward via + `ompi_mpit_register_abi_error_convert()` / + `ompi_mpit_register_abi_bind_convert()`, and the raise site calls them + through `ompi_mpit_abi_error()` / `ompi_mpit_abi_bind()`. The converter + implementations (`ompi_mpit_abi_error_convert_impl()` / + `ompi_mpit_abi_bind_convert_impl()`, wrapping the generated + `ompi_convert_intern_error_abi_error()` / + `ompi_convert_t_bind_ompi_to_standard()`) live in `libmpi_abi`. When no + converter is registered (the Open MPI ABI), the value is returned + unchanged, which is the encoding a tool linked against that ABI expects. - Bound-object resolution (sec. 6.1) dereferences the tool's `obj_handle` to the internal identity; under the Open MPI ABI the MPI handle *is* - that pointer, so a single deref suffices. The Standard ABI must convert - its integer handle to the internal pointer there (marked `XXX ABI`). + that pointer, so a single deref suffices. Under the Standard ABI the + tool's `obj_handle` is the address of a Standard-ABI *integer* handle, + which must be converted to the internal pointer before it can be matched + against the object a raise site binds to. + **Deferred work (open-mpi/ompi#13280):** this binding-side conversion is + not yet implemented. `ompi/mpi/tool/event_handle_alloc.c.in` passes the + tool's `obj_handle` straight to `mca_base_event_handle_alloc()` with no + Standard-ABI->internal conversion. As a result, under the Standard ABI, + binding a registration to an object-bound event (currently only + `ompi.mpi.communicator_named`) works for user-created communicators only + by accident -- their Standard-ABI handle happens to equal the internal + pointer -- and silently never matches predefined handles such as + `MPI_COMM_WORLD`, whose Standard-ABI handle is a small reserved integer. + Closing this gap requires converting `obj_handle` in the Standard-ABI + copy of `event_handle_alloc.c.in` (marked `XXX ABI`). Because the engine's `mca_base_event_read` is a raw `memcpy` with no ABI translation, a tool always reads back the representation the producer @@ -559,7 +588,12 @@ producer's responsibility, hence the per-raise-site branch. `ompi.mpi.communicator_created` event under a `libmpi_abi`-linked process and asserts the payload handle is the Standard-ABI integer handle of the new communicator (sec. 10), exercising the downward - converter registration and the raise-site conversion end to end. + converter registration and the raise-site conversion. Note this probe + is *not* object-bound, so it does not exercise the binding side: under + the Standard ABI, converting the tool's `obj_handle` in + `event_handle_alloc.c.in` remains deferred work (see sec. 10), so + object-bound delivery for predefined handles is not yet covered + end-to-end. ---