From a0486e0d45b9bcd1f8598b77c5ba6a765602924e Mon Sep 17 00:00:00 2001 From: Howard Pritchard Date: Wed, 4 Dec 2024 12:20:45 -0700 Subject: [PATCH] comm: beef up args checking for some comm constructors The MPI_Comm_create_from_group and especially the MPI_Intercomm_create_from_groups functions are recent additions to the standard (MPI 4.0) and users may get confused easily trying to use them. So better parameter checking is needed. Related to #12906 where an incorrect code example showed up. Signed-off-by: Howard Pritchard --- ompi/mpi/c/comm_create_from_group.c | 18 ++++++++--- ompi/mpi/c/intercomm_create_from_groups.c | 39 ++++++++++++++++------- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/ompi/mpi/c/comm_create_from_group.c b/ompi/mpi/c/comm_create_from_group.c index e3347b6f72a..0101f84a1f8 100644 --- a/ompi/mpi/c/comm_create_from_group.c +++ b/ompi/mpi/c/comm_create_from_group.c @@ -15,7 +15,7 @@ * reserved. * Copyright (c) 2015 Research Organization for Information Science * and Technology (RIST). All rights reserved. - * Copyright (c) 2021 Triad National Security, LLC. All rights + * Copyright (c) 2021-2024 Triad National Security, LLC. All rights * reserved. * $COPYRIGHT$ * @@ -54,23 +54,31 @@ int MPI_Comm_create_from_group (MPI_Group group, const char *tag, MPI_Info info, if ( MPI_PARAM_CHECK ) { OMPI_ERR_INIT_FINALIZE(FUNC_NAME); + if (NULL == errhandler || + MPI_ERRHANDLER_NULL == errhandler || + ( OMPI_ERRHANDLER_TYPE_COMM != errhandler->eh_mpi_object_type && + OMPI_ERRHANDLER_TYPE_PREDEFINED != errhandler->eh_mpi_object_type) ) { + return ompi_errhandler_invoke (NULL, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, + MPI_ERR_ARG,FUNC_NAME); + } + if (NULL == tag) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, MPI_ERR_TAG, FUNC_NAME); } if (NULL == group) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, MPI_ERR_GROUP, FUNC_NAME); } if (NULL == info || ompi_info_is_freed(info)) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, MPI_ERR_INFO, FUNC_NAME); } if (NULL == newcomm) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, MPI_ERR_ARG, FUNC_NAME); } } diff --git a/ompi/mpi/c/intercomm_create_from_groups.c b/ompi/mpi/c/intercomm_create_from_groups.c index a11a936b7d9..ef509b1a500 100644 --- a/ompi/mpi/c/intercomm_create_from_groups.c +++ b/ompi/mpi/c/intercomm_create_from_groups.c @@ -17,7 +17,7 @@ * and Technology (RIST). All rights reserved. * Copyright (c) 2016 Los Alamos National Security, LLC. All rights * reserved. - * Copyright (c) 2018-2021 Triad National Security, LLC. All rights + * Copyright (c) 2018-2024 Triad National Security, LLC. All rights * reserved. * $COPYRIGHT$ * @@ -50,7 +50,7 @@ int MPI_Intercomm_create_from_groups (MPI_Group local_group, int local_leader, M int remote_leader, const char *tag, MPI_Info info, MPI_Errhandler errhandler, MPI_Comm *newintercomm) { - int rc; + int rc, my_grp_rank, remote_grp_size; MEMCHECKER( memchecker_comm(local_comm); @@ -60,26 +60,43 @@ int MPI_Intercomm_create_from_groups (MPI_Group local_group, int local_leader, M if (MPI_PARAM_CHECK) { OMPI_ERR_INIT_FINALIZE(FUNC_NAME); - if (NULL == errhandler) { - return MPI_ERR_ARG; - } + if (NULL == errhandler || + MPI_ERRHANDLER_NULL == errhandler || + ( OMPI_ERRHANDLER_TYPE_COMM != errhandler->eh_mpi_object_type && + OMPI_ERRHANDLER_TYPE_PREDEFINED != errhandler->eh_mpi_object_type) ) { + return ompi_errhandler_invoke (NULL, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, + MPI_ERR_ARG,FUNC_NAME); - if (NULL == local_group || NULL == remote_group) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, - MPI_ERR_GROUP, FUNC_NAME); } + if (NULL == info || ompi_info_is_freed(info)) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, MPI_ERR_INFO, FUNC_NAME); } if (NULL == tag) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, MPI_ERR_TAG, FUNC_NAME); } if (NULL == newintercomm) { - return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, errhandler->eh_mpi_object_type, + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, MPI_ERR_ARG, FUNC_NAME); } + + my_grp_rank = ompi_group_rank((ompi_group_t *)local_group); + if (local_leader == my_grp_rank) { + + if (NULL == local_group || NULL == remote_group) { + return ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, + MPI_ERR_GROUP, FUNC_NAME); + } + + remote_grp_size = ompi_group_size((ompi_group_t *)remote_group); + if (remote_leader >= remote_grp_size) { + rc = ompi_errhandler_invoke (errhandler, MPI_COMM_NULL, OMPI_ERRHANDLER_TYPE_COMM, + MPI_ERR_ARG, FUNC_NAME); + return rc; + } + } } rc = ompi_intercomm_create_from_groups (local_group, local_leader, remote_group, remote_leader, tag,