CID 1269730 Dereference after null check (FORWARD_NULL)
The code checked for cb == NULL before checking for a callback
function but did not have the same protection around the
OBJ_RELEASE(cb).
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269821 Dereference null return value (NULL_RETURNS)
This is another false positive that can be silenced by looping on
opal_list_remove_first instead of using both opal_list_is_empty and
opal_list_remove_first.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1301390 Dereference before null check (REVERSE_INULL)
endpoint can not be NULL here. Remove NULL check.
CID 1269836 Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
CID 1301388 Bad bit shift operation (BAD_SHIFT)
Add ull to integer constants to ensure the math is done in 64-bits not
32.
CID 715749 Explicit null dereferenced (FORWARD_NULL)
As far as I can tell this parser function does not accept a line that
does match key = value. If that is the case then value should never be
NULL. If it is it is a parse error. Updated the code to reflect
this. Also modified the intify function to do something more sane
(strtol vs atoi with hex detection).
CID 1269820 Dereference null return value (NULL_RETURNS)
This is a false positive as strchr will never return NULL here. It
makes sense, though, to quiet the warning by changing the do {} while
() loop to a while () loop.
CID 1269780 Dereference after null check (FORWARD_NULL)
Just return an error if the endpoint's cpc data is NULL.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269674 Ignoring number of bytes read (CHECKED_RETURN)
Check that we read enough bytes to get a complete async command.
CID 1269793 Missing break in switch (MISSING_BREAK)
Added comment to indicate fall through was intentional.
CID 1269702: Constant variable guards dead code (DEADCODE)
Remove an unused argument to opal_show_help. This will quiet the
coverity issue.
CID 1269675 Ignoring number of bytes read (CHECKED_RETURN)
Check that at least sizeof(int) bytes are read. If this is not the
case then it is an error.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1196720 Resource leak (RESOURCE_LEAK)
CID 1196721 Resource leak (RESOURCE_LEAK)
The code in question does leak loc_token and loc_value. Cleaned up the
code a bit and plugged the leak.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 996175 Dereference before null check (REVERSE_NULL)
If lims is NULL then we ran out of memory. Return an error and remove
the NULL check at cleanup.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269864 Resource leak (RESOURCE_LEAK)
CID 1269865 Resource leak (RESOURCE_LEAK)
Slightly refactored the code to remove extra goto statements and
ensure the if_include_list and if_exclude_list are actually released
on success.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269931 Uninitialized scalar variable (UNINIT)
Initialize complete async message. This was not a bug but the fix
contributes to valgrind cleanness (uninitialed write).
CID 1269915 Unintended sign extension (SIGN_EXTENSION)
Should never happen. Quieting this by explicitly casting to uint64_t.
CID 1269824 Dereference null return value (NULL_RETURNS)
It is impossible for opal_list_remove_first to return NULL if
opal_list_is_empty returns false. I refactored the code in question to
not use opal_list_is_empty but loop until NULL is returned by
opal_list_remove_first. That will quiet the issue.
CID 1269913 Dereference before null check (REVERSE_INULL)
The storage parameter should never be NULL. The check intended to
check if *storage was NULL not storage.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269988 Use after free (USE_AFTER_FREE)
CID 1269987 Use after free (USE_AFTER_FREE)
Both are false positives as convert is always overwritten by the call
to opal_dss_unpack_string(). Set convert to prevent this issue from
re-appearing.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1269933 Uninitialized scalar variable (UNINIT)
This CID isn't really an error but it is best for both valgrind and
coverity cleanness to not write uninitialized data. Added an
initializer for async_command in btl_openib_component_close.
CID 1269930 Uninitialized scalar variable (UNINIT)
Same as above. Best not to write uninitialized data. Added an
initializer for async_command.
CID 1269701 Logically dead code (DEADCODE)
Coverity is correct. The smallest_pp_qp will always be 0. Changed the
initial value so that the smallest_pp_qp is set as intended. If no
per-per queue pair exists then use the last shared queue pair. This
queue pair should have the smallest message size. This will reduce
buffer waste.
CID 1269713 Logically dead code (DEADCODE)
False positive but easy to silence. The two check are meaningless if
HAVE_XRC is 0 so protect them with #if HAVE_XRC.
CID 1269726 Division or modulo by zero (DIVIDE_BY_ZERO)
Indeed an issue. If we get an invalid value for rd_win then this will
cause a divide-by-zero exception. Added a check to ensure rd_win is >
0. Also updated the help message to reflect this requirement.
CID 1269672 Ignoring number of bytes read (CHECKED_RETURN)
This error was somewhat intentional. Linux parameter files are
probably not empty but it is safer to check the return code of read to
make sure we got something. If 0 bytes are read this code could SEGV
whe running strtoull.
CID 1269836 Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
Add a range check to read_module_param to ensure we do not
overflow. In the future it might be worthwhile to report an error
because these parameters should never cause overflow in this
calculation.
CID 1269692 Calling risky function (DC.WEAK_CRYPTO)
??? This call was added in 2006 but I see no calls to the rest of the
rand48 family of functions. Anyway, we SHOULD NEVER be calling seed48,
srand, etc because it messes with user code. Removed the call to
seed48.
CID 1269823 Dereference null return value (NULL_RETURNS)
This is likely a false positive. The endpoint lock is being held so no
other thread should be able to remove fragments from the list. Also,
mca_btl_openib_endpoint_post_send should not be removing items from
the list. If a NULL fragment is ever returned it will likely be a
coding error on the part of an Open MPI developer. Added an assert()
to catch this and quiet the coverity error.
CID 1269671 Unchecked return value (CHECKED_RETURN)
Added a check for the return code of mca_btl_openib_endpoint_post_send
to quiet the coverity error. It is unlikely this error path will be
traversed.
CID 1270229 Missing break in switch (MISSING_BREAK)
Add a comment to indicate that the fall-through is intentional.
CID 1269735 Dereference after null check (FORWARD_NULL)
There should always be an endpoint when handling a work
completion. The endpoint is either stored on the fragment or can be
looked up using the immediate data. Move the immediate data code up
and add an assert for a NULL endpoint.
CID 1269740 Dereference after null check (FORWARD_NULL)
CID 1269741 Explicit null dereferenced (FORWARD_NULL)
Similar to CID 1269735 fix.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1292483 Uninitialized pointer read (UNINIT)
Initialize the method and credential members of the opal_sec_cred_t to
avoid possible invalid read when calling cleanup_cred.
CID 1292484 Double free (USE_AFTER_FREE)
Set method and credential members to NULL after freeing in
cleanup_cred.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
CID 1292738 Dereference after null check (FORWARD_NULL)
It is an error if NULL is passed for val in add_to_env_str. Removed
the NULL-check @ keyval_parse.c:253 and added a NULL check and an
error return.
CID 1292737 Logically dead code (DEADCODE)
Coverity is correct, the error code at the end of parse_line_new is
never reached. This means we fail to report parsing errors when
parsing -x and -mca lines in keyval files. I moved the error code into
the loop and removed the checks @ keyval_parse.c:314.
I also named the parse state enum type and updated parse_line_new to
use this type.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
Followup to open-mpi/ompi@65b66ab: if we're not debugging, then #if
out an entire block so that the compiler doesn't warn about variables
that are assigned and not used.
CID 70622 Dereference before null check (REVERSE_INULL)
CID 70459 Logically dead code (DEADCODE)
Cleanup some cludgy code which (among other things) reimplemented
strcat, strdup, and strchr. In the process this resolved two
outstanding coverity issues.
CID 70631 Dereference before null check (REVERSE_INULL)
best_module can not be NULL in this code path. Remove NULL check and
unnecessary goto statements.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
long double internal representation is arch specific,
and no conversion is done (yet), so MPI_LONG_DOUBLE should not
be used (yet) in heterogeneous mode
CID 1047278 Unchecked return value
Updated check for mca_base_var_generate_full_name4 to match other
checks. Logically equivalent to the old check. Not a bug.
CID 1196685 Dereference null return
Added check for NULL when looking up the original variable for a
synonym.
CID 1269705 Logically dead code
Removed code that set the project to NULL. Code was intended to be
removed with an earlier commit that added the project name into the
component structure. Added code to actually support searching for a
group with a wildcard ('*').
CID 1292739 Dereference null return
CID 1269819 Dereference null return
Removed unnecessary string duplication and strchr.
CID 1287030 Logically dead code
Refactored fixup_files code and confirmed that the code in question is
not reachable. Removed the dead code.
CID 1292740 Use of untrusted string
Use strdup to silence coverity warning.
CID 1294413 Free of address-of expression
Reset mitem to NULL after the OPAL_LIST_FOREACH loop to ensure we
never try to free the list sentinel.
CID 1294414 Unchecked return value
Use (void) to indicate we do not care about the return code in this
instance.
CID 1294415 Resource leak
On error free all the base pointer.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
When using an external libfabric (or really any libfabric newer than
libfabric commit 607e863), we must use fi_getname to determine the local
port of our endpoint. Without this fix, OMPI will hang endlessly
while retransmitting packets to port 0 on the remote host.
Code for setting proc node locality
was absent after the removal of Cray
PMI KVS usage. This commit puts that
functionality back in place.
Signed-off-by: Howard Pritchard <howardp@lanl.gov>
This commit fixes some issues with the cuda support parameters. There
were a couple of duplicate registrations and an incorrect synonym (one
variable was made a synonym of mpi_preconnect_mpi).
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
This commit fixes synonyms so the source file is correctly printed out
by ompi_info. This commit also adds support for printing out the line
number where the variable is set.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
Only install the fake usnic libibverbs driver when there are actually
usnic kernel devices present. This prevents some run-time weirdness
on the Cray verbs emulation environment, where apparently
ibv_register_driver() either is not implemented or does not work
properly.
In days past, some implementations of Portals4 could not cover all
of memory with a single Memory Descriptor so multiple large
overlapping Memory Descriptors were created. Because none of the
current implementations have this limitation (and no future
implementations should either), this commit removes the overlapping
Memory Descriptors code.
A few uninitialized common symbols are remaining:
common symbols generated by flex :
* opal/util/keyval/keyval_lex.l: opal_util_keyval_yyleng
* opal/util/keyval/keyval_lex.o: opal_util_keyval_yytext
* opal/util/show_help_lex.l: opal_show_help_yyleng
* opal/util/show_help_lex.l: opal_show_help_yytext
common symbol generated by "external" hwloc library:
* opal/mca/hwloc/hwloc191/hwloc/src/components.o: component_map
This is likely short-lived: now that libfabric has a 1.0.0 release
available, the embedded libfabric may disappear from the OMPI tree
sometime soon. However, we still need it for the time being...
The ompi libfabric/Makefile.am to build the libmca_component_libfabric
lib was missing a recently added psmx_eq.c in the list of source
files for the psm provider.
Fixes#569
Signed-off-by: Howard Pritchard <howardp@lanl.gov>
When a libibverbs driver returns NULL for its context, it's the Open
MPI libibverbs fake driver. Hence, this device is simply not
supported -- ignore it.
libibverbs will complain to stderr if it sees device entries in
/sys/class/infiniband for which it has no userspace plugins.
The Cisco usNIC device no longer exports a verbs interface, thereby
causing libibverbs to emit this annoying stderr warning.
To avoid this, use the public ibv API to register a "fake" libibverbs
driver at run-time (right after we call ibv_fork_init(), but --
critically -- *before* we call ibv_get_device_list()). The purpose of
this driver is solely to convince libibverbs that there *is* a driver
for /sys/class/infininband/usnic_verbs devices. ...although this
driver will never return a valid ibv context (and therefore will never
be used).
Defer initializing the CPCs until we know that we have devices/ports
to use. This both prevents some useless work at startup when there
are no devices/ports to use, and also prevents librdmacm complaining
that there are no verbs-capable RDMA devices available (e.g., if a
Cisco usNIC device is present, but does not present a verbs RDMA
interface).
Defer initializing the CPCs until we know that we have devices/ports
to use. This both prevents some useless work at startup when there
are no devices/ports to use, and also prevents librdmacm complaining
that there are no verbs-capable RDMA devices available (e.g., if a
Cisco usNIC device is present, but does not present a verbs RDMA
interface).
This commit fixes an assert when trying to cleanup a module we failed
to initialize. There is no protection around the OBJ_DESTRUCT calls so
they will always be called so similarly we should always call
OBJ_CONSTRUCT at init.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
This commit also fixes a problem with the lazy opening of topo
components. The topo framework incorrectly: 1) checked if the topo
framework was open by checking the length of the components list, and
2) called the framework open directly instead of using
mca_base_framework_open.
fixes#544
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
The Portals4 spec requires that PtlSetMap() be called before any
Portals4 call other than PtlNIInit(), PtlGetMap() and
PtlGetPhysId(). To satisfy this requirement, this commit delays
interface initialization until the end add_procs() at which time
PtlSetMap() has been called.
The Portals4 BTL is registered with the PML as an RDMA BTL, so
prepare_src() is only used in limited cases. This commit removes
the code path from prepare_src() for unbuffered contiguous buffers
with no PML reserve. This is now handled in register_mem().
In the large message case, the sender issues a PtlMEAppend() in
order to generate events when the receiver issues a PtlGet(). This
commit moves the PtlMEAppend() from mca_btl_portals4_prepare_src()
to mca_btl_portals4_register_mem() which is the way it's done in
BTL 3.0.
This commit is related to an RFC from June 2014. Disscussion can be
found at:
http://www.open-mpi.org/community/lists/devel/2014/07/15140.php
The finalize function is set using either the linker option -fini or
__attribute__((destructor)) depending on compiler support. I have
confirmed that this hybrid approach works with all the major
compilers. The attribute is supported by gcc, clang, llvm, xlc, and
icc. The fini function will support pgi. If a compiler/linker
combination does not support either the destructor or fini function a
message will be printed on re-init indicating it is not supported (an
improvement over the current behavior-- SEGV).
I moved the following to the destructor function:
- Class system finalize. This solves a bug when MPI_T_finalize is
called before MPI_Init. The only downside to this change is we will
leave the footprint of the opal class system after
MPI_Finalize. This footprint should be relatively small.
This is an alternative to #517 but the two PRs are not
mutually-exclusive (with some modifications). This commit should also
be safe for 1.8.x as it does not change internal or external ABI (#517
changes internal ABI).
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
This commit is a rework of the component repository. The changes
included in this commit are:
- Remove the component dependency code based off .ompi_info
files. This code is legacy code dating back 10 years that and is no
longer used.
- Move the plugin scanning code to the component repository. New
calls have been added to add new scanning paths, query available
components, and dlopen/load components.
- Pass the framework down to mca_base_component_find/filter. Eventually
the framework structure will be used to further validate components
before they are used.
- Add support to the MCA framework system to disable scanning for
dlopened components on open (support already existed in
register). This is really only relevant to installdirs as it has no
register function and no DSO components.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
This commit fixes a typo in mca_btl_vader_progress_endpoints where
OPAL_THREAD_LOCK was used when OPAL_THREAD_UNLOCK was intended.
Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
