Merge pull request #8472 from awlauria/fix_mpiext_segv

Fix segv in OMPI_Affinity_str().

Author: awlauria

ompi/mpiext/affinity/c/mpiext_affinity_str.c

@@ -9,6 +9,7 @@
  * Copyright (c) 2015-2017 Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
  * Copyright (c) 2015-2019 Intel, Inc.  All rights reserved.
+ * Copyright (c) 2021      IBM Corporation. All rights reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -98,26 +99,24 @@ int OMPI_Affinity_str(ompi_affinity_fmt_t fmt_type,
  */
 static int get_rsrc_ompi_bound(char str[OMPI_AFFINITY_STRING_MAX])
 {
-    int ret;
-
     /* If OMPI did not bind, indicate that */
     if (!ompi_rte_proc_is_bound) {
         opal_string_copy(str, ompi_nobind_str, OMPI_AFFINITY_STRING_MAX);
         return OMPI_SUCCESS;
     }
 
-    if (NULL == ompi_proc_applied_binding) {
-        ret = OPAL_ERR_NOT_BOUND;
-    } else {
-        ret = opal_hwloc_base_cset2str(str, OMPI_AFFINITY_STRING_MAX,
-                                       opal_hwloc_topology,
-                                       ompi_proc_applied_binding);
+    hwloc_bitmap_t cpuset = hwloc_bitmap_alloc();
+    hwloc_bitmap_list_sscanf(cpuset, opal_process_info.cpuset);
+    if(OPAL_ERR_NOT_BOUND == opal_hwloc_base_cset2str(str,
+                                   OMPI_AFFINITY_STRING_MAX,
+                                   opal_hwloc_topology,
+                                   cpuset))
+    {
+       opal_string_copy(str, not_bound_str, OMPI_AFFINITY_STRING_MAX);
     }
-    if (OPAL_ERR_NOT_BOUND == ret) {
-        opal_string_copy(str, not_bound_str, OMPI_AFFINITY_STRING_MAX);
-        ret = OMPI_SUCCESS;
-    }
-    return ret;
+    hwloc_bitmap_free(cpuset);
+
+    return OMPI_SUCCESS;
 }
 
 
@@ -290,28 +289,23 @@ static int get_rsrc_exists(char str[OMPI_AFFINITY_STRING_MAX])
  */
 static int get_layout_ompi_bound(char str[OMPI_AFFINITY_STRING_MAX])
 {
-    int ret;
-
     /* If OMPI did not bind, indicate that */
     if (!ompi_rte_proc_is_bound) {
         opal_string_copy(str, ompi_nobind_str, OMPI_AFFINITY_STRING_MAX);
         return OMPI_SUCCESS;
     }
 
-    /* Find out what OMPI bound us to and prettyprint it */
-    if (NULL == ompi_proc_applied_binding) {
-        ret = OPAL_ERR_NOT_BOUND;
-    } else {
-        ret = opal_hwloc_base_cset2mapstr(str, OMPI_AFFINITY_STRING_MAX,
+    hwloc_bitmap_t cpuset = hwloc_bitmap_alloc();
+    hwloc_bitmap_list_sscanf(cpuset, opal_process_info.cpuset);
+    if(OPAL_ERR_NOT_BOUND == opal_hwloc_base_cset2mapstr(str,
+                                          OMPI_AFFINITY_STRING_MAX,
                                           opal_hwloc_topology,
-                                          ompi_proc_applied_binding);
-    }
-    if (OPAL_ERR_NOT_BOUND == ret) {
+                                          cpuset))
+    {
         opal_string_copy(str, not_bound_str, OMPI_AFFINITY_STRING_MAX);
-        ret = OMPI_SUCCESS;
     }
-
-    return ret;
+    hwloc_bitmap_free(cpuset);
+    return OMPI_SUCCESS;
 }
 
 /*

ompi/runtime/ompi_rte.c

@@ -15,6 +15,7 @@
  * Copyright (c) 2020      Amazon.com, Inc. or its affiliates.  All Rights
  *                         reserved.
  * Copyright (c) 2021      Nanook Consulting.  All rights reserved.
+ * Copyright (c) 2021      IBM Corporation. All rights reserved.
  * $COPYRIGHT$
  */
 #include "ompi_config.h"
@@ -62,7 +63,6 @@
 /* storage to support OMPI */
 opal_process_name_t pmix_name_wildcard = {UINT32_MAX-1, UINT32_MAX-1};
 opal_process_name_t pmix_name_invalid = {UINT32_MAX, UINT32_MAX};
-hwloc_cpuset_t ompi_proc_applied_binding = NULL;
 bool ompi_singleton = false;
 
 static int _setup_top_session_dir(char **sdir);

opal/mca/hwloc/base/hwloc_base_util.c

@@ -19,7 +19,7 @@
  * Copyright (C) 2018      Mellanox Technologies, Ltd.
  *                         All rights reserved.
  * Copyright (c) 2018      Amazon.com, Inc. or its affiliates.  All Rights reserved.
- * Copyright (c) 2019 IBM Corporation. All rights reserved.
+ * Copyright (c) 2019-2021 IBM Corporation. All rights reserved.
  * Copyright (c) 2019-2020 Inria.  All rights reserved.
  * $COPYRIGHT$
  *
@@ -294,6 +294,18 @@ int opal_hwloc_base_get_topology(void)
     wildcard_rank.jobid = OPAL_PROC_MY_NAME.jobid;
     wildcard_rank.vpid = OPAL_VPID_WILDCARD;
 
+    // Did the user ask for a topo file at the mca line?
+    // Check this first, before main methods.
+    if(NULL != opal_hwloc_base_topo_file) {
+        opal_output_verbose(1, opal_hwloc_base_framework.framework_output,
+                            "hwloc:base loading topology from file %s",
+                            opal_hwloc_base_topo_file);
+        if(OPAL_SUCCESS != (rc = opal_hwloc_base_set_topology(opal_hwloc_base_topo_file))) {
+            return rc;
+        }
+        goto done;
+    }
+
 #if HWLOC_API_VERSION >= 0x20000
     opal_output_verbose(2, opal_hwloc_base_framework.framework_output,
                          "hwloc:base: looking for topology in shared memory");
@@ -337,7 +349,7 @@ int opal_hwloc_base_get_topology(void)
             opal_output_verbose(2, opal_hwloc_base_framework.framework_output,
                                 "hwloc:base: topology in shared memory");
             topo_in_shmem = true;
-            return OPAL_SUCCESS;
+            goto done;
         }
     }
 #endif
@@ -394,7 +406,7 @@ int opal_hwloc_base_get_topology(void)
             hwloc_topology_destroy(opal_hwloc_topology);
             return rc;
         }
-    } else if (NULL == opal_hwloc_base_topo_file) {
+    } else {
         opal_output_verbose(1, opal_hwloc_base_framework.framework_output,
                             "hwloc:base discovering topology");
         if (0 != hwloc_topology_init(&opal_hwloc_topology) ||
@@ -408,15 +420,10 @@ int opal_hwloc_base_get_topology(void)
             hwloc_topology_destroy(opal_hwloc_topology);
             return rc;
         }
-    } else {
-        opal_output_verbose(1, opal_hwloc_base_framework.framework_output,
-                            "hwloc:base loading topology from file %s",
-                            opal_hwloc_base_topo_file);
-        if (OPAL_SUCCESS != (rc = opal_hwloc_base_set_topology(opal_hwloc_base_topo_file))) {
-            return rc;
-        }
     }
 
+    done:
+
     /* fill opal_cache_line_size global with the smallest L1 cache
        line size */
     fill_cache_line_size();
@@ -659,9 +666,11 @@ static hwloc_obj_t df_search(hwloc_topology_t topo,
 // available = opal_hwloc_base_get_available_cpus(topo, start)
 // and skipped objs that had hwloc_bitmap_iszero(available)
         hwloc_obj_t root;
-        opal_hwloc_topo_data_t *rdata;
+        opal_hwloc_topo_data_t *rdata = NULL;
         root = hwloc_get_root_obj(topo);
-        rdata = (opal_hwloc_topo_data_t*)root->userdata;
+        if(false == topo_in_shmem) {
+            rdata = (opal_hwloc_topo_data_t*)root->userdata;
+        }
         hwloc_cpuset_t constrained_cpuset;
 
         constrained_cpuset = hwloc_bitmap_alloc();
@@ -696,7 +705,7 @@ unsigned int opal_hwloc_base_get_nbobjs_by_type(hwloc_topology_t topo,
     unsigned int num_objs;
     hwloc_obj_t obj;
     opal_hwloc_summary_t *sum;
-    opal_hwloc_topo_data_t *data;
+    opal_hwloc_topo_data_t *data = NULL;
     int rc;
 
     /* bozo check */
@@ -728,10 +737,17 @@ unsigned int opal_hwloc_base_get_nbobjs_by_type(hwloc_topology_t topo,
     obj = hwloc_get_root_obj(topo);
 
     /* first see if the topology already has this summary */
-    data = (opal_hwloc_topo_data_t*)obj->userdata;
+    if(false == topo_in_shmem) {
+        data = (opal_hwloc_topo_data_t*)obj->userdata;
+    }
     if (NULL == data) {
         data = OBJ_NEW(opal_hwloc_topo_data_t);
-        obj->userdata = (void*)data;
+        if(false == topo_in_shmem) {
+            // Can't touch userdata if in read-only shmem!
+            // We have to protect here for the case where obj->userdata
+            // is in shmem and it is NULL.
+            obj->userdata = (void*) data;
+        }
     } else {
         OPAL_LIST_FOREACH(sum, &data->summaries, opal_hwloc_summary_t) {
             if (target == sum->type &&
@@ -1167,8 +1183,6 @@ int opal_hwloc_base_cset2str(char *str, int len,
     char tmp[BUFSIZ];
     const int stmp = sizeof(tmp) - 1;
     int **map=NULL;
-    hwloc_obj_t root;
-    opal_hwloc_topo_data_t *sum;
 
     str[0] = tmp[stmp] = '\0';
 
@@ -1177,18 +1191,6 @@ int opal_hwloc_base_cset2str(char *str, int len,
         return OPAL_ERR_NOT_BOUND;
     }
 
-    /* if the cpuset includes all available cpus, then we are unbound */
-    root = hwloc_get_root_obj(topo);
-    if (NULL != root->userdata) {
-        sum = (opal_hwloc_topo_data_t*)root->userdata;
-        if (NULL == sum->available) {
-           return OPAL_ERROR;
-        }
-        if (0 != hwloc_bitmap_isincluded(sum->available, cpuset)) {
-            return OPAL_ERR_NOT_BOUND;
-        }
-    }
-
     if (OPAL_SUCCESS != (ret = build_map(&num_sockets, &num_cores, cpuset, &map, topo))) {
         return ret;
     }
@@ -1235,8 +1237,6 @@ int opal_hwloc_base_cset2mapstr(char *str, int len,
     int core_index, pu_index;
     const int stmp = sizeof(tmp) - 1;
     hwloc_obj_t socket, core, pu;
-    hwloc_obj_t root;
-    opal_hwloc_topo_data_t *sum;
 
     str[0] = tmp[stmp] = '\0';
 
@@ -1245,18 +1245,6 @@ int opal_hwloc_base_cset2mapstr(char *str, int len,
         return OPAL_ERR_NOT_BOUND;
     }
 
-    /* if the cpuset includes all available cpus, then we are unbound */
-    root = hwloc_get_root_obj(topo);
-    if (NULL != root->userdata) {
-        sum = (opal_hwloc_topo_data_t*)root->userdata;
-        if (NULL == sum->available) {
-           return OPAL_ERROR;
-        }
-        if (0 != hwloc_bitmap_isincluded(sum->available, cpuset)) {
-            return OPAL_ERR_NOT_BOUND;
-        }
-    }
-
     /* Iterate over all existing sockets */
     for (socket = hwloc_get_obj_by_type(topo, HWLOC_OBJ_SOCKET, 0);
          NULL != socket;