global symbol name pollution

[markalle]

You can download testcase files here:
https://github.com/open-mpi/ompi-tests-public/blob/master/packaging/run_nmcheck.c
https://github.com/open-mpi/ompi-tests-public/blob/master/packaging/nmcheck_prefix.pl

With those in place and a build of OMPI available, running the test is just:

#!/bin/sh
export OPAL_PREFIX=/path/to/some/ompi_install
export LD_LIBRARY_PATH=${OPAL_PREFIX}/lib
$OPAL_PREFIX/bin/mpicc -o x run_nmcheck.c
$OPAL_PREFIX/bin/mpirun -np 1 ./x

And a big list of questionable global exports should get listed.

Or a lighter weight way to get similar output is simply

nm ompi_install/lib/libmpi.so | grep ' [TWBCDVR] ' | grep -v -e ' MPI_' -e ' mpi_' -e ' PMPI_' -e ' MPIX_' -e ' MPL_' -e ' ADIOI_' -e ' ADIO_' -e ' ompi_' -e ' OMPI_' -e ' mca_' | less

although I recommend the ompi-tests-public testcase since it checks more libraries and has a longer list of global exports it allows as "acceptable" prefixes. Those prefixes are a somewhat arbitrary decision based on pragmatism -- eg what symbol names are likely to collide with symbols from a user's application or possibly their other support libraries.

When I run the test on branch v5.0.x I get

*** checking /u/markalle/space/Projects/OMPIDev.symbol/install/lib/libopen-pal.so.0
    [error]   common_sm_mpool_create
    [error]   selected_component
    [error]   shmem_posix_shm_open
    [error]   wait_sync_global_wakeup_mt
    [error]   wait_sync_global_wakeup_st
    [error]   wait_sync_list
*** checking /u/markalle/space/Projects/OMPIDev.symbol/install/lib/libpmix.so.0
    [error]   file_caddy_t_class
    [error]   file_tracker_t_class
    [error]   heartbeat_caddy_t_class
*** checking /u/markalle/space/Projects/OMPIDev.symbol/install/lib/libmpi.so.0
    [error]   _dict_free
    [error]   _dict_malloc
    [error]   ad_get_env_vars
    [error]   adapt_module_cached_topology
    [error]   adapt_topology_cache_item_t_class
    [error]   add_edge_3
    [error]   add_to_bucket
    [error]   add_to_list
    [error]   adjacency_asc
    [error]   adjacency_dsc
    [error]   aggregate_obj_weight
    [error]   algo
    [error]   allocate_vertex
    [error]   allocate_vertex2
    [error]   append_frag_to_ordered_list
    [error]   available_components
    [error]   balancing
    [error]   bottom_up_build_tree_from_topology
    [error]   bucket_grouping
    [error]   bucket_id
    [error]   build_cost_matrix
    [error]   build_level_topology
    [error]   build_p_vector
    [error]   build_synthetic_proc_id
    [error]   built_pivot_tree
    [error]   check_bucket
    [error]   check_cantmatch_for_match
    [error]   check_constraints
    [error]   choose
    [error]   clone_tree
    [error]   common_monitoring_translation_ht
    [error]   complete_aff_mat
    [error]   complete_obj_weight
    [error]   complete_tab_node
    [error]   compute_gain
    [error]   compute_nb_leaves_from_level
    [error]   compute_weighted_degree
    [error]   create_dumb_tree
    [error]   create_work
    [error]   delete_group_list
    [error]   depth_first
    [error]   destroy_work
    [error]   destruction
    [error]   dfs
    [error]   dict_destroy
    [error]   dict_int_cmp
    [error]   dict_itor_destroy
    [error]   dict_long_cmp
    [error]   dict_ptr_cmp
    [error]   dict_set_free
    [error]   dict_set_malloc
    [error]   dict_str_cmp
    [error]   dict_uint_cmp
    [error]   dict_ulong_cmp
    [error]   display_bucket
    [error]   display_bucket_list
    [error]   display_grouping
    [error]   display_node
    [error]   display_pivots
    [error]   display_selection
    [error]   display_sol
    [error]   display_sol_sum_com
    [error]   display_tab
    [error]   display_tab_group
    [error]   distance
    [error]   era_agreement_info_t_class
    [error]   era_comm_agreement_specific_t_class
    [error]   era_iagree_request_t_class
    [error]   era_rank_item_t_class
    [error]   era_value_t_class
    [error]   eval_cost
    [error]   eval_cost2
    [error]   eval_grouping
    [error]   eval_sol
    [error]   exchange
    [error]   fast_group
    [error]   fast_grouping
    [error]   fbtl_posix_max_aio_active_reqs
    [error]   fiboTreeConsolidate
    [error]   fiboTreeDel
    [error]   fiboTreeExit
    [error]   fiboTreeFree
    [error]   fiboTreeInit
    [error]   fiboTreeMin
    [error]   file_stats
   ...
    [error]   zero_dte_mapping
*** checking /u/markalle/space/Projects/OMPIDev.symbol/install/lib/libmpi_mpifh.so
*** checking /u/markalle/space/Projects/OMPIDev.symbol/install/lib/libmpi_usempif08.so
*** checking /u/markalle/space/Projects/OMPIDev.symbol/install/lib/libmpi_usempi_ignore_tkr.so

Note, when it comes to fixing them sometimes you can make things static, but often the symbols will span multiple files so in that case a prefix can be added to the symbol. Also when symbols like adapt_topology_cache_item_t_class show up (things that end with _class) that means there's a line like

OBJ_CLASS_INSTANCE(adapt_topology_cache_item_t, ...)

and the occurrances of adapt_topology_cache_item_t need updated. Eg I'm just noting that you can't search the code for adapt_topology_cache_item_t_class, you have to just search for adapt_topology_cache_item_t to find the cause of the adapt_topology_cache_item_t_class export.

[gpaulsen]

I added Severity: critical for v5.0.0.
#8704 exposed many more incorrectly scoped symbols to the user in libmpi.so than were exposed in v4.x series. This may cause linking issues (or worse?) for our customers, and should be fixed before v5.0.0.

[markalle]

@gpaulsen asked me to post how many symbols there are that need changed, based on my run just now it's ~250.

Admittedly if we're only being pragmatic and non-strict we could ignore long names like bottom_up_build_tree_from_topology since even though it lacks any OMPI-reserved prefix, it seems extraordinarily unlikely a user app would just happen to use that same function name. Vs say create_work or distance that would be super easy for an app to also have.

The compromise solution I advocate though is to be strict about requiring some kind of prefix, but pragmatic about letting it be a large list of "acceptable" prefixes.

[awlauria]

Because some of these are so generic, such as "distance" and "dfs", changing this to a blocker.

[awlauria]

I've tried various different things to try and force an error - @markalle do you have a simple reproducer that will show a failure if a user defines one of these global symbols?

[markalle]

Ah, yes I did make an actual test that shows how redefining one of those global symbols in an MPI user applications breaks OMPI:

*** demonstrating why pollution is a problem:

I tried a few functions and not everything gets called in every run of course, but one example that I was able to get called was "append_frag_to_ordered_list". A user could have a function with that name in their user app, and if they do, maybe creating an app that looks like this:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <mpi.h>
static void
myprint(char *mesg)
{
    char str[4096];
    printf("*** %s -- in App's function\n", mesg);
    fflush(stdout);
    sprintf(str,
        "gdb -q -batch -ex bt -p %d < "
        "/dev/null 2>&1 | grep -v "
        "  -e '^Missing separate debuginfo for ' "
        "  -e '^Warning:.*No such file or directory' "
        "  -e 'Inferior .*process .*detached' "
        "  -e ' zypper install -C ' | "
        "sed -e 's/^/[p%d] /'", getpid(), getpid());
    system(str);
    fflush(stdout);
}
void append_frag_to_ordered_list() { myprint("append_frag_to_ordered_list"); }
int
main(int argc, char *argv[])
{
    int   myrank, nranks;
    char  myhost[MPI_MAX_PROCESSOR_NAME];
    int   len;
    char  *sbuf, *rbuf;
    MPI_Init(&argc, &argv);
    MPI_Comm_rank(MPI_COMM_WORLD, &myrank);
    MPI_Comm_size(MPI_COMM_WORLD, &nranks);
    MPI_Get_processor_name(myhost, &len);
    sbuf = malloc(10000000);
    rbuf = malloc(10000000);
    MPI_Bcast(sbuf, 100, MPI_CHAR, 0, MPI_COMM_WORLD);
    MPI_Bcast(sbuf, 1000, MPI_CHAR, 0, MPI_COMM_WORLD);
    MPI_Bcast(sbuf, 10000, MPI_CHAR, 0, MPI_COMM_WORLD);
    MPI_Bcast(sbuf, 100000, MPI_CHAR, 0, MPI_COMM_WORLD);
    MPI_Bcast(sbuf, 1000000, MPI_CHAR, 0, MPI_COMM_WORLD);
    MPI_Bcast(sbuf, 10000000, MPI_CHAR, 0, MPI_COMM_WORLD);
    MPI_Sendrecv(sbuf, 10, MPI_CHAR, (myrank+1)%nranks, 99,
                 rbuf, 10, MPI_CHAR, (myrank+nranks-1)%nranks, 99,
                 MPI_COMM_WORLD, MPI_STATUS_IGNORE);
    MPI_Sendrecv(sbuf, 1000, MPI_CHAR, (myrank+1)%nranks, 99,
                 rbuf, 1000, MPI_CHAR, (myrank+nranks-1)%nranks, 99,
                 MPI_COMM_WORLD, MPI_STATUS_IGNORE);
    MPI_Sendrecv(sbuf, 100000, MPI_CHAR, (myrank+1)%nranks, 99,
                 rbuf, 100000, MPI_CHAR, (myrank+nranks-1)%nranks, 99,
                 MPI_COMM_WORLD, MPI_STATUS_IGNORE);
    MPI_Sendrecv(sbuf, 10000000, MPI_CHAR, (myrank+1)%nranks, 99,
                 rbuf, 10000000, MPI_CHAR, (myrank+nranks-1)%nranks, 99,
                 MPI_COMM_WORLD, MPI_STATUS_IGNORE);
    MPI_Finalize();
    free(sbuf);
    free(rbuf);
    printf("past finalize at rank %d/%d (on %s)\n", myrank, nranks, myhost);
    exit(0);
}

And run with something like

export OPAL_PREFIX=/path/to/ompinstall
export LD_LIBRARY_PATH="${OPAL_PREFIX}/lib"
$OPAL_PREFIX/bin/mpicc -o x simplempi.c
hosts=hostA:1,hostB:1
$OPAL_PREFIX/bin/mpirun \
    --host $hosts \
    --mca coll ^hcoll \
    --mca pml ob1 --mca btl tcp,self \
    ./x

The stack trace comes up as deep in OMPI code where suddenly it thinks it's calling OMPI's "append_frag_to_ordered_list" but it's actually calling the user Application's redefinition.

[p295482] #3  0x0000000010000dc8 in myprint ()
[p295482] #4  0x0000000010000e34 in append_frag_to_ordered_list ()
[p295482] #5  0x000020000036b368 in mca_pml_ob1_recv_frag_match () from /myinstall/lib/libmpi.so.0
[p295482] #6  0x0000200000d2a658 in mca_btl_tcp_endpoint_recv_handler () from /myinstall/lib/libopen-pal.so.0
[p295482] #7  0x000020000112a58c in event_process_active_single_queue () from /lib64/libevent_core-2.1.so.6
[p295482] #8  0x000020000112b020 in event_base_loop () from /lib64/libevent_core-2.1.so.6
[p295482] #9  0x0000200000c736b8 in opal_progress_events () from /myinstall/lib/libopen-pal.so.0
[p295482] #10 0x0000200000c73838 in opal_progress () from /myinstall/lib/libopen-pal.so.0
[p295482] #11 0x000020000013d070 in ompi_request_default_wait () from /myinstall/lib/libmpi.so.0
[p295482] #12 0x00002000001a36e8 in PMPI_Sendrecv () from /myinstall/lib/libmpi.so.0
[p295482] #13 0x00000000100010f4 in main ()

[jsquyres]

See also #8262.

[jjhursey]

Per Open MPI teleconf Sept. 6:

• This ticket deals primarily with name prefixing.
• A question about the use of OPAL_DECLSPEC was noted:
  • We need to add something to the developer docs (maybe a paragraph) on its function and intended usage in the Open MPI codebase.
  • Scenario: Shared library DSO would need to use this attribute if they want a symbol to be usable outside of that library/DSO.
  • It exports this symbol outside of the immediate library scope
  • Related to Windows declspec

[naughtont3]

We need to add something to the developer docs (maybe a paragraph) on its function and intended usage in the Open MPI codebase.

See also #10770