MPI_Win_lock returns incorrect error code for rank<0

[jeffhammond]

Summary

MPI_Win_lock should return MPI_ERR_RANK instead of MPI_ERR_RMA_SYNC when the rank argument is invalid. Currently, Open-MPI does this for invalid positive ranks but not negative ones.

Version

I built 7e57075 from source.

Root Cause and Proposed Solution

ompi_win_peer_invalid needs to check for negative ranks.

Before

static inline int ompi_win_peer_invalid(ompi_win_t *win, int peer) {
    if (win->w_group->grp_proc_count <= peer) return true;
    return false;
}

After

static inline int ompi_win_peer_invalid(ompi_win_t *win, int peer) {
    if (win->w_group->grp_proc_count <= peer || peer < 0) return true;
    return false;
}

Obviously, this is but one of many trivial ways to fix this.

Unexpected Result

$ /opt/ompi/dev/gcc/bin/mpirun -n 2 ./ompi_err_codes.x -1
[blah:44302] *** An error occurred in MPI_Win_lock
[blah:44302] *** reported by process [2748121089,0]
[blah:44302] *** on win 
[blah:44302] *** MPI_ERR_RMA_SYNC: error executing rma sync
[blah:44302] *** MPI_ERRORS_ARE_FATAL (processes in this win will now abort,
[blah:44302] ***    and potentially your MPI job)

Expected Results

$ /opt/ompi/dev/gcc/bin/mpirun -n 2 ./ompi_err_codes.x 0
$ /opt/ompi/dev/gcc/bin/mpirun -n 2 ./ompi_err_codes.x 1
$ /opt/ompi/dev/gcc/bin/mpirun -n 2 ./ompi_err_codes.x 2 
[blah:44298] *** An error occurred in MPI_Win_lock
[blah:44298] *** reported by process [2747858945,0]
[blah:44298] *** on win 
[blah:44298] *** MPI_ERR_RANK: invalid rank
[blah:44298] *** MPI_ERRORS_ARE_FATAL (processes in this win will now abort,
[blah:44298] ***    and potentially your MPI job)
[blah.ra.intel.com:44296] 1 more process has sent help message help-mpi-errors.txt / mpi_errors_are_fatal
[blah.ra.intel.com:44296] Set MCA parameter "orte_base_help_aggregate" to 0 to see all help / error messages

Source

#include <stdio.h>
#include <stdlib.h>

#include <mpi.h>

int main(int argc, char* argv[])
{
    MPI_Init(&argc, &argv);
    char * base;
    MPI_Win win;
    MPI_Win_allocate(1, 1, MPI_INFO_NULL, MPI_COMM_WORLD, &base, &win);
    {
        int rank = (argc>1) ? atoi(argv[1]) : 0;
        MPI_Win_lock(MPI_LOCK_SHARED, rank, 0, win);
        MPI_Win_unlock(rank, win);
    }
    MPI_Win_free(&win);
    MPI_Finalize();
}

Compile

$ /opt/ompi/dev/gcc/bin/mpicc -g -Wall ompi_err_codes.c -o ompi_err_codes.x

Cross-reference

pmodels/casper#22 (comment)

[jsquyres]

Thanks for the bug report and corresponding PR!

[ggouaillardet]

fwiw, here is the revamped test case to fit the Open MPI test framework

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

#include <mpi.h>
#include "ompitest_error.h"
#include "ompitest_mpit.h"

int main(int argc, char* argv[])
{
    char * base;
    MPI_Win win;
    bool checking_params;
    int err, rank = -1;

    MPI_Init(&argc, &argv);
    if ( !ompitest_checking_mpi_params() ) {
        // we do not check params, skipping this test
        MPI_Abort(MPI_COMM_WORLD, 77);
    }

    MPI_Win_allocate(1, 1, MPI_INFO_NULL, MPI_COMM_WORLD, &base, &win);
    MPI_Win_set_errhandler(win,MPI_ERRORS_RETURN);
    err = MPI_Win_lock(MPI_LOCK_SHARED, rank, 0, win);
    if (MPI_ERR_RANK != err) {
        ompitest_error(__FILE__, __LINE__, "MPI_Win_lock(rank<0) did not fail with MPI_ERR_RANK\n");
    }
    MPI_Finalize();
    return 0;
}