Merge pull request #13187 from lrbison/coverity

Coverity Fixes for coll/tuned and coll/han/alltoallv

Author: lrbison

ompi/mca/coll/han/coll_han_alltoallv.c

@@ -804,6 +804,7 @@ int mca_coll_han_alltoallv_using_smsc(
     low_gather_out = malloc(sizeof(*low_gather_out) * low_size);
     struct peer_data *peers = malloc(sizeof(*peers) * low_size);
     opal_datatype_t *peer_send_types = malloc(sizeof(*peer_send_types) * low_size);
+    bool have_bufs_and_types = false;
 
     low_gather_in.serialization_buffer = serialization_buf;
     low_gather_in.sbuf = (void*)sbuf; // cast to discard the const
@@ -896,6 +897,7 @@ int mca_coll_han_alltoallv_using_smsc(
         peers[jrank].sendtype = &peer_send_types[jrank];
     }
 
+    have_bufs_and_types = true;
     send_from_addrs = malloc(sizeof(*send_from_addrs)*low_size);
     recv_to_addrs = malloc(sizeof(*recv_to_addrs)*low_size);
     send_counts = malloc(sizeof(*send_counts)*low_size);
@@ -964,14 +966,16 @@ int mca_coll_han_alltoallv_using_smsc(
         free(recv_types);
     }
 
-    for (int jlow=0; jlow<low_size; jlow++) {
-        if (jlow != low_rank) {
-            OBJ_DESTRUCT(&peer_send_types[jlow]);
-        }
+    if (have_bufs_and_types) {
+        for (int jlow=0; jlow<low_size; jlow++) {
+            if (jlow != low_rank) {
+                OBJ_DESTRUCT(&peer_send_types[jlow]);
+            }
 
-        for (int jbuf=0; jbuf<2; jbuf++) {
-            if (peers[jlow].map_ctx[jbuf]) {
-                mca_smsc->unmap_peer_region(peers[jlow].map_ctx[jbuf]);
+            for (int jbuf=0; jbuf<2; jbuf++) {
+                if (peers[jlow].map_ctx[jbuf]) {
+                    mca_smsc->unmap_peer_region(peers[jlow].map_ctx[jbuf]);
+                }
             }
         }
     }

ompi/mca/coll/tuned/coll_tuned_component.c

@@ -304,7 +304,7 @@ mca_coll_tuned_module_construct(mca_coll_tuned_module_t *module)
 
 int coll_tuned_alg_from_str(int collective_id, const char *alg_name, int *alg_value) {
     int rc;
-    if (collective_id > COLLCOUNT || collective_id < 0) { return OPAL_ERROR; };
+    if (collective_id >= COLLCOUNT || collective_id < 0) { return OPAL_ERROR; };
     rc = coll_tuned_algorithm_enums[collective_id]->value_from_string(
         coll_tuned_algorithm_enums[collective_id],
         alg_name, alg_value );
@@ -314,7 +314,7 @@ int coll_tuned_alg_from_str(int collective_id, const char *alg_name, int *alg_va
 /* return the enum's value and string.  caller's responsibility to free alg_string if NULL was not provided. */
 int coll_tuned_alg_to_str(int collective_id, int alg_value, char **alg_string) {
     int rc;
-    if (collective_id > COLLCOUNT || collective_id < 0) { return OPAL_ERROR; };
+    if (collective_id >= COLLCOUNT || collective_id < 0) { return OPAL_ERROR; };
     rc = coll_tuned_algorithm_enums[collective_id]->string_from_value(
         coll_tuned_algorithm_enums[collective_id],
         alg_value, alg_string );
@@ -326,7 +326,7 @@ int coll_tuned_alg_register_options(int collective_id, mca_base_var_enum_t *opti
     /* use the same enum used for mca parameters to allow tuning files to use
     algorithm names rather than just numbers.*/
     if (!options) { return OPAL_ERROR; }
-    if (collective_id > COLLCOUNT || collective_id < 0) {
+    if (collective_id >= COLLCOUNT || collective_id < 0) {
         return OPAL_ERROR;
     }
 

ompi/mca/coll/tuned/coll_tuned_dynamic_file.c

@@ -244,9 +244,11 @@ static int ompi_coll_tuned_read_rules_json (const opal_json_t *json_root, ompi_c
 
     const opal_json_t *collectives_obj = NULL;
     const opal_json_t *comm_rule_array = NULL;
+    const opal_json_t *comm_rule = NULL;
+    const opal_json_t *msg_size_array = NULL;
 
     size_t num_collectives = 0;
-    size_t num_comm_rules;
+    size_t num_comm_rules = 0;
     rc = opal_json_get_key(json_root, "collectives", &collectives_obj);
     if (rc != OPAL_SUCCESS) {
         opal_output_verbose(1, ompi_coll_tuned_stream,
@@ -269,15 +271,14 @@ static int ompi_coll_tuned_read_rules_json (const opal_json_t *json_root, ompi_c
             opal_output_verbose(1, ompi_coll_tuned_stream,
                 "Internal json error when attempting to parse the collective at index %ld\n",
                 jcol);
-            goto error_bad_coll;
+            goto error_cleanup;
         }
         coll_id = mca_coll_base_name_to_colltype(coll_name);
         if (coll_id < 0) {
             opal_output_verbose(1, ompi_coll_tuned_stream,
                 "Unrecognized collective: \"%s\". Use all lowercase such as \"allgather\"",
                 coll_name);
-            opal_json_free(&comm_rule_array);
-            goto error_bad_coll;
+            goto error_cleanup;
         }
 
         alg_p = &alg_rules[coll_id];
@@ -287,14 +288,12 @@ static int ompi_coll_tuned_read_rules_json (const opal_json_t *json_root, ompi_c
             opal_output_verbose(1, ompi_coll_tuned_stream,
                 "Problem parsing the collective at index %ld (for %s).  Expected an array of comm-related rules.",
                 jcol, mca_coll_base_colltype_to_str(coll_id) );
-            goto error_bad_coll;
+            goto error_cleanup;
         }
         alg_p->n_com_sizes = (int)num_comm_rules;
         alg_p->com_rules = ompi_coll_tuned_mk_com_rules (num_comm_rules, coll_id);
 
         for (jcomm_rule=0; jcomm_rule < num_comm_rules; jcomm_rule++) {
-            const opal_json_t *comm_rule;
-            const opal_json_t *msg_size_array;
             size_t num_msg_rules;
             rc = opal_json_get_index(comm_rule_array, jcomm_rule, &comm_rule);
             com_p = &(alg_p->com_rules[jcomm_rule]);
@@ -353,15 +352,16 @@ static int ompi_coll_tuned_read_rules_json (const opal_json_t *json_root, ompi_c
         "Problem occurred within collective %s, "
         "comm_size_min=%d, comm_size_max=%d (entry number %ld in the comm_size array), "
         "message size entry number %ld.", coll_name, com_p->mpi_comsize_min, com_p->mpi_comsize_max, 1+jcomm_rule, 1+jmsg_rule);
-    opal_json_free(&collectives_obj);
-    return OMPI_ERROR;
+    goto error_cleanup;
 error_bad_comm_rule:
     opal_output_verbose(1, ompi_coll_tuned_stream,
         "Problem occurred within collective %s, "
         "in entry number %ld of the comm_size array", coll_name, 1+jcomm_rule);
-    opal_json_free(&collectives_obj);
-    return OMPI_ERROR;
-error_bad_coll:
+    goto error_cleanup;
+error_cleanup:
+    opal_json_free(&msg_size_array);
+    opal_json_free(&comm_rule);
+    opal_json_free(&comm_rule_array);
     opal_json_free(&collectives_obj);
     return OMPI_ERROR;
 }
@@ -494,6 +494,11 @@ static int ompi_coll_tuned_read_rules_config_file_classic (char *fname, ompi_col
         }
         opal_output_verbose(25, ompi_coll_tuned_stream,
             "Read communicator count %ld for dynamic rule for collective ID %ld\n", NCOMSIZES, COLID);
+        if( NCOMSIZES > INT_MAX) {
+            opal_output_verbose(25, ompi_coll_tuned_stream,
+                "Refusing to proceed: suspiciously large number found for the number of communicator-based rules: %ld\n", NCOMSIZES);
+            goto on_file_error;
+        }
         alg_p->n_com_sizes = NCOMSIZES;
         alg_p->com_rules = ompi_coll_tuned_mk_com_rules (NCOMSIZES, COLID);
         if (NULL == alg_p->com_rules) {

opal/util/json/opal_json.c

@@ -203,7 +203,7 @@ int opal_json_get_key_by_index(const opal_json_t *json, const size_t index, cons
 
     json_object_entry entry = {0};
 
-    if (index < 0 || index >= in->value->u.object.length) {
+    if (index >= in->value->u.object.length) {
         opal_show_help("help-json.txt", "Index out of bound", true, index,
                        in->value->u.array.length);
         return ret;