Opal coverity fixes #608
Merged
Opal coverity fixes #608
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sent from
|
Refer to this link for build results (access rights to CI server needed): Build Log Test FAILed. |
|
|
Refer to this link for build results (access rights to CI server needed): Build Log Test FAILed. |
|
d'oh. i messed up the dead code fix. now fixed. |
|
|
Refer to this link for build results (access rights to CI server needed): Build Log Test FAILed. |
|
Refer to this link for build results (access rights to CI server needed): Build Log Test FAILed. |
|
|
Refer to this link for build results (access rights to CI server needed): Build Log Test FAILed. |
|
Fixed for real now. |
|
|
Refer to this link for build results (access rights to CI server needed): |
hjelmn
force-pushed
the
opal_coverity
branch
2 times, most recently
from
2e59b0a to
dcb4767
Compare
|
|
Refer to this link for build results (access rights to CI server needed): |
|
|
Refer to this link for build results (access rights to CI server needed): |
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>
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 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 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 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 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 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 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 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>
|
|
Refer to this link for build results (access rights to CI server needed): |
jsquyres
added a commit
to jsquyres/ompi
that referenced
this pull request
Nov 10, 2015
Now that we have an "isolated" PLM component, we cannot just let rsh …
markalle
pushed a commit
to markalle/ompi
that referenced
this pull request
Sep 12, 2020
only generate one binding for MPI_IN_PLACE etc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Quieting coverity issues in opal.