Skip to content

Commit 877ee24

Browse files
committed
Correctly access the communicator name is MSGQ.
We transformed the communicator c_name field from an array into a pointer and fialed to update the MSGQ accessors to the field. Thanks @david-edwards-linaro for the report and the initial patch. Fixes #12063. Signed-off-by: George Bosilca <bosilca@icl.utk.edu>
1 parent 1967454 commit 877ee24

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

ompi/debuggers/ompi_mpihandles_dll.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
* Copyright (c) 2012-2013 Inria. All rights reserved.
88
* Copyright (c) 2016 Research Organization for Information Science
99
* and Technology (RIST). All rights reserved.
10-
* Copyright (c) 2017 IBM Corp. All rights reserved.
10+
* Copyright (c) 2017 IBM Corp. All rights reserved.
11+
* Copyright (c) 2023 NVIDIA Corporation. All rights reserved.
12+
* Copyright (c) 2023 Jeffrey M. Squyres. All rights reserved.
1113
* $COPYRIGHT$
1214
*
1315
* Additional copyrights may follow
@@ -458,8 +460,21 @@ int mpidbg_comm_query(mqs_image *image, mqs_image_info *image_info,
458460
(*info)->comm_c_handle = c_comm;
459461

460462
printf("mpidbg_comm_query: %p\n", (void*) c_comm);
461-
mqs_fetch_data(process, c_comm + i_info->ompi_communicator_t.offset.c_name,
463+
mqs_taddr_t name_addr = ompi_fetch_pointer( process,
464+
c_comm + i_info->ompi_communicator_t.offset.c_name,
465+
p_info );
466+
mqs_fetch_data(process, name_addr,
462467
MPIDBG_MAX_OBJECT_NAME, (*info)->comm_name);
468+
(*info)->comm_name[MPIDBG_MAX_OBJECT_NAME-1] = '\0';
469+
/* Defensively zero anything beyond the actual name.
470+
471+
We know that MPIDBG_MAX_OBJECT_NAME == MPI_MAX_OBJECT_NAME (per
472+
mpihandles_interface.h), and OMPI *guarantees* that
473+
(*info)->comm_name will be both \0-terminated, and have a
474+
maximum of (MPI_MAX_OBJECT_NAME-1) non-NULL characters. So the
475+
memset length expression below is guaranted be >=0. */
476+
memset((*info)->comm_name + strlen((*info)->comm_name), 0,
477+
MPIDBG_MAX_OBJECT_NAME - 1 - strlen((*info)->comm_name));
463478

464479
/* Get this process' rank in the comm */
465480
(*info)->comm_rank = ompi_fetch_int(process,

ompi/debuggers/ompi_msgq_dll.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
* Copyright (c) 2016 Intel, Inc. All rights reserved.
1111
* Copyright (c) 2016 Research Organization for Information Science
1212
* and Technology (RIST). All rights reserved.
13+
* Copyright (c) 2023 NVIDIA Corporation. All rights reserved.
14+
* Copyright (c) 2023 Jeffrey M. Squyres. All rights reserved.
1315
* $COPYRIGHT$
1416
*
1517
* Additional copyrights may follow
@@ -682,8 +684,18 @@ static int rebuild_communicator_list (mqs_process *proc)
682684
p_info );
683685
old->group = find_or_create_group( proc, group_base );
684686
}
685-
mqs_fetch_data( proc, comm_ptr + i_info->ompi_communicator_t.offset.c_name,
686-
64, old->comm_info.name );
687+
mqs_taddr_t name_addr = ompi_fetch_pointer( proc,
688+
comm_ptr + i_info->ompi_communicator_t.offset.c_name,
689+
p_info );
690+
/* c_name can be up to MPI_MAX_OBJECT_NAME bytes, but we only
691+
* copy the first (sizeof(old->comm_info.name)-1) here. Make
692+
* sure the string is correctly terminated. */
693+
size_t target_size = sizeof(old->comm_info.name);
694+
mqs_fetch_data( proc, name_addr, target_size, old->comm_info.name );
695+
old->comm_info.name[target_size - 1] = '\0';
696+
/* Defensively zero anything beyond the actual name */
697+
size_t src_strlen = strlen(old->comm_info.name);
698+
memset(old->comm_info.name + src_strlen, 0, target_size - 1 - src_strlen);
687699

688700
if( NULL != old->group ) {
689701
old->comm_info.size = old->group->entries;
@@ -1156,8 +1168,9 @@ static int fetch_request( mqs_process *proc, mpi_process_info *p_info,
11561168
ompi_datatype + i_info->ompi_datatype_t.offset.size,
11571169
p_info );
11581170
/* Be user friendly, show the datatype name */
1171+
size_t data_name_size = sizeof(data_name);
11591172
mqs_fetch_data( proc, ompi_datatype + i_info->ompi_datatype_t.offset.name,
1160-
64, data_name );
1173+
data_name_size, data_name );
11611174
if( '\0' != data_name[0] ) {
11621175
// res->extra_text[x] is only 64 chars long -- same as
11631176
// data_name. If you try to snprintf it into
@@ -1171,6 +1184,11 @@ static int fetch_request( mqs_process *proc, mpi_process_info *p_info,
11711184
(int)res->desired_length);
11721185
snprintf( (char*)res->extra_text[2], 64, "%s",
11731186
data_name );
1187+
} else {
1188+
data_name[data_name_size - 1] = '\0';
1189+
/* Be nice and zero anything beyond the actual name */
1190+
size_t data_name_strlen = strlen(data_name);
1191+
memset(data_name + data_name_strlen, 0, data_name_size - 1 - data_name_strlen);
11741192
}
11751193
/* And now compute the real length as specified by the user */
11761194
res->desired_length *=

0 commit comments

Comments
 (0)