Skip to content

Fix some errors find by Address Sanitizer #1795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
foxtran wants to merge 4 commits into
base: main
Choose a base branch
from
+34 −23
Open

Fix some errors find by Address Sanitizer #1795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d849b8b
Do not use write in-place
foxtran Nov 20, 2025
94e70d2
missvalue accept r4 datatype
foxtran Nov 20, 2025
abad718
Do not use temporary arrays as c_loc targets
foxtran Nov 20, 2025
7cbdc23
Fix order of deallocation send_buffer: mpp_send is non-blocking, so d…
foxtran Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions mpp/include/mpp_domains_misc.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ end subroutine init_nonblock_type
! Force use of "scalar", integer pointer mpp interface
call mpp_send(ibounds(1), plen=4, to_pe=pelist2(1), tag=COMM_TAG_1)
call mpp_send(send_buffer(1),plen=im*jm, to_pe=pelist2(1), tag=COMM_TAG_2)
deallocate(send_buffer)

else if(pelist2(1) == pe) then ! receive data and compare
do p = pelist1(1), pelist1(size(pelist1(:)))
Expand Down Expand Up @@ -371,12 +370,13 @@ end subroutine init_nonblock_type
print*, trim(mesg)//": ", 'comparison between 1 pe and ', npes-1, ' pes is ok'
endif
! release memery
deallocate(field1, send_buffer)
deallocate(field1)
endif

deallocate(field2)

call mpp_sync()
if (allocated(send_buffer)) deallocate(send_buffer)

end subroutine mpp_check_field_2d_type1

Expand Down
14 changes: 7 additions & 7 deletions mpp/include/mpp_util_mpi.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,24 @@ subroutine mpp_error_basic( errortype, errormsg )
#endif
integer, intent(in) :: errortype
character(len=*), intent(in), optional :: errormsg
character(len=512) :: text
character(len=512) :: text, text_errortype
integer :: errunit

if( .NOT.module_is_initialized )call ABORT()

select case( errortype )
case(NOTE)
text = 'NOTE' !just FYI
text_errortype = 'NOTE' !just FYI
case(WARNING)
text = 'WARNING' !probable error
text_errortype = 'WARNING' !probable error
case(FATAL)
text = 'FATAL' !fatal error
text_errortype = 'FATAL' !fatal error
case default
text = 'WARNING: non-existent errortype (must be NOTE|WARNING|FATAL)'
text_errortype = 'WARNING: non-existent errortype (must be NOTE|WARNING|FATAL)'
end select

if( npes.GT.1 )write( text,'(a,i6)' )trim(text)//' from PE', pe !this is the mpp part
if( PRESENT(errormsg) )text = trim(text)//': '//trim(errormsg)
if( npes.GT.1 ) write( text,'(a,i6)' ) trim(text_errortype) // ' from PE', pe !this is the mpp part
if( PRESENT(errormsg) )text = trim(text) // ': ' // trim(errormsg)
!$OMP CRITICAL (MPP_ERROR_CRITICAL)
select case( errortype )
case(NOTE)
Expand Down
2 changes: 1 addition & 1 deletion test_fms/diag_manager/test_diag_update_buffer.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ program test_diag_update_buffer
diag_field_id = 1
sample = 1
weight = 1.0
missvalue = 1.0e-5
missvalue = 1.0e-5_r4_kind
Copy link
Copy Markdown
Contributor

@bensonr bensonr Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@foxtran foxtran Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@foxtran foxtran Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a shorter example how does it work: https://godbolt.org/z/5jW4P9oY4

pow_value = 1
phys_window = .false.
need_compute = .false.
Expand Down
37 changes: 24 additions & 13 deletions test_fms/exchange/test_xgrid.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,8 @@ program xgrid_test

subroutine test_unstruct_exchange()

real(r8_kind), allocatable :: atm_data_in(:,:), atm_data_sg(:,:)
real(r8_kind), allocatable :: atm_data_in_0(:,:), atm_data_in_1(:,:), atm_data_in_2(:,:), atm_data_in_3(:,:)
real(r8_kind), allocatable :: atm_data_sg(:,:)
real(r8_kind), allocatable :: atm_data_sg_1(:,:), atm_data_sg_2(:,:), atm_data_sg_3(:,:)
real(r8_kind), allocatable :: lnd_data_sg(:,:,:), ice_data_sg(:,:,:)
real(r8_kind), allocatable :: atm_data_ug(:,:), tmp_sg(:,:,:)
Expand Down Expand Up @@ -877,12 +878,18 @@ subroutine test_unstruct_exchange()
allocate(atm_data_ug_2(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_ug_3(isc_atm:iec_atm, jsc_atm:jec_atm ) )

allocate(atm_data_in(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_in_0(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_in_1(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_in_2(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_in_3(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_sg(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_sg_1(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_sg_2(isc_atm:iec_atm, jsc_atm:jec_atm ) )
allocate(atm_data_sg_3(isc_atm:iec_atm, jsc_atm:jec_atm ) )
atm_data_in = 0
atm_data_in_0 = 0
atm_data_in_1 = 0
atm_data_in_2 = 0
atm_data_in_3 = 0
atm_data_sg = 0
atm_data_sg_1 = 0
atm_data_sg_2 = 0
Expand Down Expand Up @@ -914,11 +921,14 @@ subroutine test_unstruct_exchange()
x_3 = 0
x_4 = 0

call random_number(atm_data_in)
call put_to_xgrid(atm_data_in, 'ATM', x_1, Xmap, remap_method=remap_method)
call put_to_xgrid(atm_data_in+1, 'ATM', x_2, Xmap, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in+2, 'ATM', x_3, Xmap, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in+3, 'ATM', x_4, Xmap, remap_method=remap_method, complete=.true.)
call random_number(atm_data_in_0)
atm_data_in_1 = atm_data_in_0 + 1
atm_data_in_2 = atm_data_in_0 + 2
atm_data_in_3 = atm_data_in_0 + 3
call put_to_xgrid(atm_data_in_0, 'ATM', x_1, Xmap, remap_method=remap_method)
call put_to_xgrid(atm_data_in_1, 'ATM', x_2, Xmap, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in_2, 'ATM', x_3, Xmap, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in_3, 'ATM', x_4, Xmap, remap_method=remap_method, complete=.true.)
call get_from_xgrid(lnd_data_sg, 'LND', x_1, xmap)
call get_from_xgrid(ice_data_sg, 'OCN', x_1, xmap)
call put_to_xgrid(lnd_data_sg, 'LND', x_2, xmap)
Expand All @@ -936,10 +946,10 @@ subroutine test_unstruct_exchange()
y_3 = 0
y_4 = 0

call put_to_xgrid(atm_data_in, 'ATM', y_1, Xmap_ug, remap_method=remap_method)
call put_to_xgrid(atm_data_in+1, 'ATM', y_2, Xmap_ug, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in+2, 'ATM', y_3, Xmap_ug, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in+3, 'ATM', y_4, Xmap_ug, remap_method=remap_method, complete=.true.)
call put_to_xgrid(atm_data_in_0, 'ATM', y_1, Xmap_ug, remap_method=remap_method)
call put_to_xgrid(atm_data_in_1, 'ATM', y_2, Xmap_ug, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in_2, 'ATM', y_3, Xmap_ug, remap_method=remap_method, complete=.false.)
call put_to_xgrid(atm_data_in_3, 'ATM', y_4, Xmap_ug, remap_method=remap_method, complete=.true.)
call get_from_xgrid_ug(lnd_data_ug, 'LND', y_1, xmap_ug)
call get_from_xgrid(ice_data_ug, 'OCN', y_1, xmap_ug)
call put_to_xgrid_ug(lnd_data_ug, 'LND', y_2, xmap_ug)
Expand Down Expand Up @@ -967,7 +977,8 @@ subroutine test_unstruct_exchange()

if(ice_pe) deallocate(ice_data_sg, ice_data_ug)
deallocate(tmp_sg, x_1, x_2, x_3, x_4, y_1, y_2, y_3, y_4)
deallocate(atm_data_in, atm_data_sg)
deallocate(atm_data_in_0, atm_data_in_1, atm_data_in_2, atm_data_in_3)
deallocate(atm_data_sg)
deallocate(atm_data_sg_1, atm_data_sg_2, atm_data_sg_3)
deallocate(atm_data_ug)
deallocate(atm_data_ug_1, atm_data_ug_2, atm_data_ug_3)
Expand Down
Loading