Fix some errors find by Address Sanitizer #1795
Fix some errors find by Address Sanitizer #1795foxtran wants to merge 4 commits intoNOAA-GFDL:mainfrom
Conversation
uramirez8707
left a comment
There was a problem hiding this comment.
Thank you for fixing these.
|
@foxtran Can you open an issue that describes what you are fixing and why and link it to this PR. It helps us with tracking. Also, just out of curiosity, do you have a use case for FMS that we don't know about. We are interested to hear how you are using FMS. |
I'm playing with flang frontend :-) |
|
@thomas-robinson, I filed 2 of 4 bugs. Ping me, please, tomorrow, if I will not put other 2 until tomorrow. |
|
@uramirez8707, can it be merged? |
…eallocation must happen after sync point
|
@rem1776, could you please have a look on it? |
|
@rem1776, @uramirez8707, any chance that it will be merged? :D |
|
@bensonr Could you take a look at this when you get a chance? |
bensonr
left a comment
There was a problem hiding this comment.
My apologies. While I performed the review shortly after the PR was entered, I see I never formally submitted it. Please see my comment regarding the chosen missing value.
| sample = 1 | ||
| weight = 1.0 | ||
| missvalue = 1.0e-5 | ||
| missvalue = 1.0e-5_r4_kind |
There was a problem hiding this comment.
I know you aren't the original author here, but I see a deeper problem with the chosen assignment value. At line 150, there is the possibility for missvalue to be typed as an i8 and associated with an i8 pointer leading to an invalid value.
There was a problem hiding this comment.
As I understood, missvalue can be both real(4) or integer(8). So, this assignment can change type of missvalue that invalidates missval_i8_ptr, defined at line 150, although data still be accessible via missval_i8_ptr.
There was a problem hiding this comment.
There is a shorter example how does it work: https://godbolt.org/z/5jW4P9oY4
Description
AddressSanitizer detected some errors which I've fixed here:
mpp_domain_misc:mpp_sendis non-blocking operation so,send_buffermust be alive at least until sync point. (Closesmpp_check_field_2d_type1has use after free #1803)mpp_utils_mpi: in-place writing may give errors (not sure that it really happens here, but anyway)test_diag_update_buffeer:-fdefault-real-8generates1.0e-5asreal(8). Then,real(8)constructor ofmissvalueis called while it isreal(4). So, 4 extra bytes were written. (Closestest_diag_update_manager: wrong datatype is set with-fdefault-real-8#1805)test_xgrid:put_to_xgridkept address of local/temporary variables generated byarray + xexpressions. Now, lifetimes ofarray + xarrays are expanded. That is why after Remove cray pointers fromexchange/xgrid.f90#1778 there is a lot failures of CI. (Closestest_xgridkeeps local temporaries (use after free) #1804)How Has This Been Tested?
Tested in CI with AddressSanitizer. Unfortunately, not all Address Sanitizer errors are fixed.
Checklist:
make distcheckpasses