From 403b3b20d77d7218778ce366a59d74d44c8d0799 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Thu, 21 May 2015 10:39:09 -0600 Subject: [PATCH 1/2] Handle ompi error codes in java code This commit also adds protection against negative error codes in ompi error code functions. There is one outstanding issue. There is a negative MPI error code defined in mpi.h. This will need to be fixed separetely. This commit fixes coverity IDs 1271533 and 1270156. Signed-off-by: Nathan Hjelm --- ompi/errhandler/errcode.h | 30 +++++++++++++++++++++--------- ompi/mpi/java/c/mpi_MPI.c | 15 +++++++++++++-- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/ompi/errhandler/errcode.h b/ompi/errhandler/errcode.h index b1246a1556..f6111069c2 100644 --- a/ompi/errhandler/errcode.h +++ b/ompi/errhandler/errcode.h @@ -1,4 +1,4 @@ -/* -*- Mode: C; c-basic-offset:4 ; -*- */ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ /* * Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana * University Research and Technology @@ -12,6 +12,8 @@ * All rights reserved. * Copyright (c) 2006 University of Houston. All rights reserved. * Copyright (c) 2007 Cisco Systems, Inc. All rights reserved. + * Copyright (c) 2015 Los Alamos National Security, LLC. All rights + * reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -69,10 +71,13 @@ static inline bool ompi_mpi_errcode_is_invalid(int errcode) */ static inline int ompi_mpi_errcode_get_class (int errcode) { - ompi_mpi_errcode_t *err; + ompi_mpi_errcode_t *err = NULL; + + if (errcode >= 0) { + err = (ompi_mpi_errcode_t *)opal_pointer_array_get_item(&ompi_mpi_errcodes, errcode); + /* If we get a bogus errcode, return MPI_ERR_UNKNOWN */ + } - err = (ompi_mpi_errcode_t *)opal_pointer_array_get_item(&ompi_mpi_errcodes, errcode); - /* If we get a bogus errcode, return MPI_ERR_UNKNOWN */ if (NULL != err) { if ( err->code != MPI_UNDEFINED ) { return err->cls; @@ -93,6 +98,10 @@ static inline int ompi_mpi_errnum_is_class ( int errnum ) { ompi_mpi_errcode_t *err; + if (errno < 0) { + return false; + } + if ( errnum <= ompi_mpi_errcode_lastpredefined ) { /* Predefined error values represent an error code and an error class at the same time */ @@ -117,11 +126,14 @@ static inline int ompi_mpi_errnum_is_class ( int errnum ) */ static inline char* ompi_mpi_errnum_get_string (int errnum) { - ompi_mpi_errcode_t *err; - - err = (ompi_mpi_errcode_t *)opal_pointer_array_get_item(&ompi_mpi_errcodes, errnum); - /* If we get a bogus errcode, return a string indicating that this - truly should not happen */ + ompi_mpi_errcode_t *err = NULL; + + if (errnum >= 0) { + err = (ompi_mpi_errcode_t *)opal_pointer_array_get_item(&ompi_mpi_errcodes, errnum); + /* If we get a bogus errcode, return a string indicating that this + truly should not happen */ + } + if (NULL != err) { return err->errstring; } else { diff --git a/ompi/mpi/java/c/mpi_MPI.c b/ompi/mpi/java/c/mpi_MPI.c index 7036c19e46..761704a731 100644 --- a/ompi/mpi/java/c/mpi_MPI.c +++ b/ompi/mpi/java/c/mpi_MPI.c @@ -531,8 +531,11 @@ static void* getBuffer(JNIEnv *env, ompi_java_buffer_t **item, int size) opal_free_list_item_t *freeListItem; freeListItem = opal_free_list_get (&ompi_java_buffers); - ompi_java_exceptionCheck(env, NULL == freeListItem ? OMPI_ERROR : - OMPI_SUCCESS); + ompi_java_exceptionCheck(env, NULL == freeListItem ? MPI_ERR_NO_MEM : + MPI_SUCCESS); + if (NULL == freeListItem) { + return NULL; + } *item = (ompi_java_buffer_t*)freeListItem; return (*item)->buffer; @@ -1049,6 +1052,14 @@ void ompi_java_releasePtrArray(JNIEnv *env, jlongArray array, jboolean ompi_java_exceptionCheck(JNIEnv *env, int rc) { + if (rc < 0) { + /* handle ompi error code */ + rc = ompi_errcode_get_mpi_code (rc); + /* ompi_mpi_errcode_get_class CAN NOT handle negative error codes. + * all Open MPI MPI error codes should be > 0. */ + assert (rc >= 0); + } + if(MPI_SUCCESS == rc) { return JNI_FALSE; From 163a1b4505497f9801c56cb8979312b2750b6a43 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Fri, 22 May 2015 19:59:37 -0600 Subject: [PATCH 2/2] Remove non-standard MPI_ERR_SYSRESOURCE error code Replaced internal usage with OMPI_ERR_OUT_OF_RESOURCE. Signed-off-by: Nathan Hjelm --- ompi/attribute/attribute.c | 14 +++++++------- ompi/attribute/attribute.h | 4 ++-- ompi/include/mpi.h.in | 3 --- ompi/include/mpif-values.pl | 2 -- ompi/mpi/java/c/mpi_Constant.c | 1 - 5 files changed, 9 insertions(+), 15 deletions(-) diff --git a/ompi/attribute/attribute.c b/ompi/attribute/attribute.c index 86c016f375..418ab80a57 100644 --- a/ompi/attribute/attribute.c +++ b/ompi/attribute/attribute.c @@ -479,7 +479,7 @@ int ompi_attr_init(void) keyval_hash = OBJ_NEW(opal_hash_table_t); if (NULL == keyval_hash) { - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } key_bitmap = OBJ_NEW(opal_bitmap_t); /* @@ -487,7 +487,7 @@ int ompi_attr_init(void) */ opal_bitmap_set_max_size (key_bitmap, OMPI_FORTRAN_HANDLE_MAX); if (0 != opal_bitmap_init(key_bitmap, 32)) { - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } for (int_pos = 0; int_pos < (sizeof(void*) / sizeof(MPI_Fint)); @@ -540,7 +540,7 @@ static int ompi_attr_create_keyval_impl(ompi_attribute_type_t type, /* Allocate space for the list item */ keyval = OBJ_NEW(ompi_attribute_keyval_t); if (NULL == keyval) { - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } /* Fill in the list item (must be done before we set the keyval @@ -667,7 +667,7 @@ int ompi_attr_set_c(ompi_attribute_type_t type, void *object, int ret; attribute_value_t *new_attr = OBJ_NEW(attribute_value_t); if (NULL == new_attr) { - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } OPAL_THREAD_LOCK(&attribute_lock); @@ -698,7 +698,7 @@ int ompi_attr_set_fortran_mpi1(ompi_attribute_type_t type, void *object, int ret; attribute_value_t *new_attr = OBJ_NEW(attribute_value_t); if (NULL == new_attr) { - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } OPAL_THREAD_LOCK(&attribute_lock); @@ -730,7 +730,7 @@ int ompi_attr_set_fortran_mpi2(ompi_attribute_type_t type, void *object, int ret; attribute_value_t *new_attr = OBJ_NEW(attribute_value_t); if (NULL == new_attr) { - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } OPAL_THREAD_LOCK(&attribute_lock); @@ -1056,7 +1056,7 @@ int ompi_attr_delete_all(ompi_attribute_type_t type, void *object, attrs = malloc(sizeof(attribute_value_t *) * num_attrs); if (NULL == attrs) { OPAL_THREAD_UNLOCK(&attribute_lock); - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } ret = opal_hash_table_get_first_key_uint32(attr_hash, &key, &attr, &node); diff --git a/ompi/attribute/attribute.h b/ompi/attribute/attribute.h index d3a60c8f07..b0dc9ecab8 100644 --- a/ompi/attribute/attribute.h +++ b/ompi/attribute/attribute.h @@ -189,10 +189,10 @@ int ompi_attr_hash_init(opal_hash_table_t **hash) *hash = OBJ_NEW(opal_hash_table_t); if (NULL == *hash) { fprintf(stderr, "Error while creating the local attribute list\n"); - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } if (OMPI_SUCCESS != opal_hash_table_init(*hash, ATTR_HASH_SIZE)) { - return MPI_ERR_SYSRESOURCE; + return OMPI_ERR_OUT_OF_RESOURCE; } return MPI_SUCCESS; diff --git a/ompi/include/mpi.h.in b/ompi/include/mpi.h.in index f50f110ba1..31996d978b 100644 --- a/ompi/include/mpi.h.in +++ b/ompi/include/mpi.h.in @@ -610,9 +610,6 @@ enum { error codes without breaking ABI. */ #define MPI_ERR_LASTCODE 92 -#define MPI_ERR_SYSRESOURCE -2 - - /* * Comparison results. Don't change the order of these, the group * comparison functions rely on it. diff --git a/ompi/include/mpif-values.pl b/ompi/include/mpif-values.pl index 2c26bda427..3dcb40e70c 100755 --- a/ompi/include/mpif-values.pl +++ b/ompi/include/mpif-values.pl @@ -328,8 +328,6 @@ $constants->{MPI_ERR_RMA_SHARED} = 71; $constants->{MPI_T_ERR_INVALID} = 72; $constants->{MPI_ERR_LASTCODE} = 92; -$constants->{MPI_ERR_SYSRESOURCE} = -2; - $constants->{MPI_IDENT} = 0; $constants->{MPI_CONGRUENT} = 1; $constants->{MPI_SIMILAR} = 2; diff --git a/ompi/mpi/java/c/mpi_Constant.c b/ompi/mpi/java/c/mpi_Constant.c index aa52b9186f..7e80e542ef 100644 --- a/ompi/mpi/java/c/mpi_Constant.c +++ b/ompi/mpi/java/c/mpi_Constant.c @@ -176,5 +176,4 @@ JNIEXPORT void JNICALL Java_mpi_Constant_setConstant(JNIEnv *env, jobject obj) ompi_java_setIntField(env, c, obj, "ERR_WIN", MPI_ERR_WIN); ompi_java_setIntField(env, c, obj, "ERR_LASTCODE", MPI_ERR_LASTCODE); - ompi_java_setIntField(env, c, obj, "ERR_SYSRESOURCE", MPI_ERR_SYSRESOURCE); }