Error in MPI_Error_string after failed call to MPI_T_pvar_get_info

[devreal]

I'm trying to run the varlist developed by LLNL that is supposed to list all MPI_T variables available. The code is available at https://github.com/LLNL/mpi-tools.

Running the benchmark ends in an error:

Found 20 performance variables
Found 20 performance variables with verbosity <= D/A-9

Variable                        VRB   Class   Type   Bind     R/O CNT ATM
-------------------------------------------------------------------------
mpool_hugepage_bytes_allocated  U/A-3 SIZE    ULONG  n/a      YES YES  NO
ERROR: PVARINFO: MPI error code -18: 

Thread 1 "varlist" hit Breakpoint 1, PMPI_Error_string (errorcode=-18, string=0x55555575a080 <errMsg> "MPI_ERR_OTHER: known error not in list", 
    resultlen=0x55555575a468 <errMsgLen>) at perror_string.c:44
44          OPAL_CR_NOOP_PROGRESS();
(gdb) bt
#0  PMPI_Error_string (errorcode=-18, string=0x55555575a080 <errMsg> "MPI_ERR_OTHER: known error not in list", resultlen=0x55555575a468 <errMsgLen>) at perror_string.c:44
#1  0x00005555555562a1 in list_pvars () at /home/joseph/src/mpi-tools/mpi_t/varlist/varlist.c:410
#2  0x0000555555557ca3 in main (argc=1, argv=0x7fffffffdcf8) at /home/joseph/src/mpi-tools/mpi_t/varlist/varlist.c:899
(gdb) f 1
#1  0x00005555555562a1 in list_pvars () at /home/joseph/src/mpi-tools/mpi_t/varlist/varlist.c:410
410                     CHECKERR("PVARINFO",err);
(gdb) print err
$4 = -18
(gdb) list
405             for (i=0; i<num; i++)
406             {
407                     namelen=maxnamelen;
408                     desclen=maxdesclen;
409                     err=MPI_T_pvar_get_info(i,name,&namelen,&verbos,&vc,&dt,&et,desc,&desclen,&bind,&ro,&ct,&at);
410                     CHECKERR("PVARINFO",err);
411                     if (verbos<=verbosity)
412                     {
413                             if (!longlist)
414                             {
(gdb) print name
$5 = 0x55555593a630 "mpool_hugepage_bytes_allocated"
(gdb) c
Continuing.
[beryl:18517] *** An error occurred in MPI_Error_string
[beryl:18517] *** reported by process [3787063297,0]
[beryl:18517] *** on communicator MPI_COMM_WORLD
[beryl:18517] *** MPI_ERR_ARG: invalid argument of some other kind
[beryl:18517] *** MPI_ERRORS_ARE_FATAL (processes in this communicator will now abort,
[beryl:18517] ***    and potentially your MPI job)
[Thread 0x7fffed921700 (LWP 18523) exited]
[Thread 0x7ffff4f6c700 (LWP 18522) exited]
[Inferior 1 (process 18517) exited with code 015]

The definition of the CHECKERR macro is:

#define CHECKERR(errstr,err) if (err!=MPI_SUCCESS) { printf("ERROR: %s: MPI error code %i: \n",errstr,err); MPI_Error_string(err, errMsg, &errMsgLen); errMsg[errMsgLen]=    0; printf("%s\n", errMsg); /*usage(1);*/ }

It checks the error and calls MPI_Error_string on the value returned by the previous call, MPI_T_pvar_get_info in this case. The value is -18 in this case. The call then causes a fatal error inside Open MPI.

I tried running the varlist tool using MPICH 3.2.1, which runs fine (although it reports that there are no performance variables in MPICH).

I tested with both the v4.0.x and master branches, both showing the same error.

[bosilca]

The error code -18 that we are propagating to MPI_Error_string is not a valid MPI error, but instead is our internal value for OPAL_ERR_VALUE_OUT_OF_BOUNDS (corresponding to the MPI MPI_T_ERR_INVALID_INDEX error code). We missed a conversion from OPAL to MPI error codes in ompi/mpi/tool/pvar_get_info.c.

Try the following patch:

diff --git a/ompi/mpi/tool/pvar_get_info.c b/ompi/mpi/tool/pvar_get_info.c
index 8121558f49..343ec0a16a 100644
--- a/ompi/mpi/tool/pvar_get_info.c
+++ b/ompi/mpi/tool/pvar_get_info.c
@@ -41,6 +41,7 @@ int MPI_T_pvar_get_info(int pvar_index, char *name, int *name_len,
            bounds checking. */
         ret = mca_base_pvar_get (pvar_index, &pvar);
         if (OMPI_SUCCESS != ret) {
+            ret = MPI_T_ERR_INVALID_INDEX;
             break;
         }

However, I found an error in the varlist.c file, when copying an internal string of possible length (4k) into an external string of length 257. Line 652 should read

snprintf(value, sizeof(value)-1, "%s",v_char);

[devreal]

@bosilca Thanks, the patch solves the error. I'm working on a patch that prevents the tool from querying invalid pvars in the first place.

Something seems fishy with MPI_T_CVAR_READ. The tool already skips invalid indices but some calls to MPI_T_CVAR_READ return MPI_ERR_OTHER:

ERROR: CVARREAD: MPI error code 16: MPI_ERR_OTHER: known error not in list

I tried tracking down what happens. It seems that the variables where this happens are synonyms: the call to MPI_T_CVAR_GET_INFO returns MPI_T_ERR_INVALID_INDEX for btl_openib_device_param_files but not for btl_openib_hca_param_files (the latter is a synonym for the former). This happens because the original flag passed to var_get in mca_base_var.c is false here. However, when reading the value for btl_openib_hca_param_files in MPI_T_CVAR_READ refers to the original variable (original is true), which is marked as invalid and thus the error happens. Maybe the synonyms are not correctly marked as invalid when the openib component is deregistered?

---

However, I found an error in the varlist.c file, when copying an internal string of possible length (4k) into an external string of length 257. Line 652 should read

snprintf(value, sizeof(value)-1, "%s",v_char);

Good point, I will include this in my PR. Just out of curiosity:

1. Why sizeof(value)-1? The man page states that that is the maximum number of characters written, including the terminating \0.
2. The 4k buffer size seems arbitrary but there does not seem to be a way to properly query the expected size of a string value provided by MPI_T_CVAR_READ. Is that functionality that is missing in the standard?

[jsquyres]

@hjelmn MPI_T is your baby. 😄 Can you comment?

[jsquyres]

@bosilca @devreal Can one of you make relevant PR's for that 1-line patch @bosilca posted above?

[bosilca]

we're not done here yet.

[bosilca]

@devreal I think the root cause arise from trying to get the value from an invalid CVAR. I have a patch but I don't like it because as a user I would have preferred to be told a CVAR is invalid earlier, preferably before reaching the point where trying to access it's value.

Another contentious point is that this CVARs behavior differs from the PVARs. Invalid PVARs are reported very early as the pvar search function (mca_base_pvar_get) return an error that is then propagated back to the MPI level. At the opposite, CVARs uses mca_base_var_get which is more lenient on checking the status of the variable. My personal opinion is that both should behave similarly, but let's see what the MPI_T experts and @hjelmn have to say.

Meanwhile, here is my patch.

diff --git a/ompi/mpi/tool/cvar_read.c b/ompi/mpi/tool/cvar_read.c
index 2246c5f88b..9225b7fddc 100644
--- a/ompi/mpi/tool/cvar_read.c
+++ b/ompi/mpi/tool/cvar_read.c
@@ -41,8 +41,8 @@ int MPI_T_cvar_read (MPI_T_cvar_handle handle, void *buf)
     do {
         rc = mca_base_var_get_value(handle->var->mbv_index, &value, NULL, NULL);
         if (OPAL_SUCCESS != rc || NULL == value) {
-            /* shouldn't happen */
-            rc = MPI_ERR_OTHER;
+            /* invalid or discarded cvar, ignore */
+            rc = MPI_T_ERR_INVALID_INDEX;
             break;
         }

diff --git a/ompi/mpi/tool/pvar_get_info.c b/ompi/mpi/tool/pvar_get_info.c
index 8121558f49..343ec0a16a 100644
--- a/ompi/mpi/tool/pvar_get_info.c
+++ b/ompi/mpi/tool/pvar_get_info.c
@@ -41,6 +41,7 @@ int MPI_T_pvar_get_info(int pvar_index, char *name, int *name_len,
            bounds checking. */
         ret = mca_base_pvar_get (pvar_index, &pvar);
         if (OMPI_SUCCESS != ret) {
+            ret = MPI_T_ERR_INVALID_INDEX;
             break;
         }

[devreal]

I think the root cause arise from trying to get the value from an invalid CVAR. I have a patch

Yes, that fixes the symptom and is definitely needed to correctly signal to the user that they are trying to query an invalid CVAR.

but I don't like it because as a user I would have preferred to be told a CVAR is invalid earlier, preferably before reaching the point where trying to access it's value.

For most invalid CVARs a call to MPI_T_CVAR_GET_INFO already returns MPI_T_ERR_INVALID_INDEX. The problematic CVARs are the ones that are synonyms of other CVARs because they appear to not be marked as invalid (as opposed to the CVARs they refer to). Maybe a call to MPI_T_CVAR_GET_INFO on a synonym CVAR just needs to query the state of the original CVAR just like a call to MPI_T_CVAR_READ does?

[awlauria]

Has the above patch been posted? I assume not - wondering if it's still needed/desired.