Skip to content

Commit

Permalink
Merge pull request #2469 from GEOS-ESM/hotfix/nag-dangling-pointers
Browse files Browse the repository at this point in the history
Fixes for NAG dangling pointers.
  • Loading branch information
mathomp4 committed Dec 5, 2023
2 parents 646d85b + 1e0940d commit 1e204be
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 24 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ parameters:
# Anchors to prevent forgetting to update a version
os_version: &os_version ubuntu20
baselibs_version: &baselibs_version v7.14.0
bcs_version: &bcs_version v11.2.0
bcs_version: &bcs_version v11.3.0
tag_build_arg_name: &tag_build_arg_name maplversion

orbs:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Corrected some unit tests (and test utilities) to fix dangling pointers detected by NAG. Most (possibly all) of these changes are already on release/MAPL-v3, but it was getting annoying to have NAG fail unit tests with develop branch.

### Removed

### Deprecated
Expand Down
2 changes: 1 addition & 1 deletion base/tests/Test_SimpleMAPLcomp.pf
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module Test_SimpleMAPLcomp

contains

@test(npes=[1,2,0],type=newESMF_TestMethod)
@test(npes=[1,2,0],type=ESMF_TestMethod)
subroutine test_one(this)
class (ESMF_TestMethod), intent(inout) :: this

Expand Down
12 changes: 6 additions & 6 deletions base/tests/Test_SphericalToCartesian.pf
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module Test_SphericalToCartesian
contains
@test(npes=[1],type=newESMF_TestMethod)
@test(npes=[1],type=ESMF_TestMethod)
subroutine test_spherical_to_cartesian_east_wind(this)
class (ESMF_TestMethod), intent(inout) :: this
type (LatLonGridFactory) :: factory
Expand Down Expand Up @@ -55,7 +55,7 @@ contains
end subroutine test_spherical_to_cartesian_east_wind
@test(npes=[1],type=newESMF_TestMethod)
@test(npes=[1],type=ESMF_TestMethod)
subroutine test_spherical_to_cartesian_north_wind(this)
class (ESMF_TestMethod), intent(inout) :: this
type (LatLonGridFactory) :: factory
Expand Down Expand Up @@ -93,7 +93,7 @@ contains
end subroutine test_spherical_to_cartesian_north_wind
@test(npes=[1],type=newESMF_TestMethod)
@test(npes=[1],type=ESMF_TestMethod)
subroutine test_cartesian_to_spherical_X(this)
class (ESMF_TestMethod), intent(inout) :: this
type (LatLonGridFactory) :: factory
Expand Down Expand Up @@ -132,7 +132,7 @@ contains
end subroutine test_cartesian_to_spherical_X
@test(npes=[1],type=newESMF_TestMethod)
@test(npes=[1],type=ESMF_TestMethod)
subroutine test_cartesian_to_spherical_Y(this)
class (ESMF_TestMethod), intent(inout) :: this
type (LatLonGridFactory) :: factory
Expand Down Expand Up @@ -172,7 +172,7 @@ contains
end subroutine test_cartesian_to_spherical_Y
@test(npes=[1],type=newESMF_TestMethod)
@test(npes=[1],type=ESMF_TestMethod)
subroutine test_cartesian_to_spherical_Z(this)
class (ESMF_TestMethod), intent(inout) :: this
type (LatLonGridFactory) :: factory
Expand Down Expand Up @@ -215,7 +215,7 @@ contains
! No good place to put this test, so putting it here for now.
! Testing a static method on abstract class (AbstractGridFactory)
@test(npes=[1,2,3,4,6],type=newESMF_TestMethod)
@test(npes=[1,2,3,4,6],type=ESMF_TestMethod)
subroutine test_make_arbitrary_decomposition(this)
class (ESMF_TestMethod), intent(inout) :: this
type (LatLonGridFactory) :: factory
Expand Down
4 changes: 2 additions & 2 deletions pfio/StringVariableMap.F90
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ integer function StringVariableMap_get_length(this) result(length)
end function StringVariableMap_get_length

subroutine StringVariableMap_serialize(map, buffer, rc)
type (StringVariableMap) ,intent(in):: map
integer, allocatable,intent(inout) :: buffer(:)
type (StringVariableMap), target, intent(in):: map
integer, allocatable, intent(inout) :: buffer(:)
integer, optional, intent(out) :: rc

type (StringVariableMapIterator) :: iter
Expand Down
33 changes: 22 additions & 11 deletions pfunit/ESMF_TestCase.F90
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,31 @@ module ESMF_TestCase_mod
recursive subroutine runBare(this)
class (ESMF_TestCase), intent(inout) :: this

! We need an inner procedure to get the TARGET attribute
! added to the TestCase object so that it can be called back from inside the ESMF
! gridcomp. Inelegant but it works around the issue where NAG debug flags do
! a copy-in/copy-out which leaves a dangling pointer in the self reference.
call runbare_inner(this)
end subroutine runBare

subroutine runbare_inner(this)
class (ESMF_TestCase), target, intent(inout) :: this

logical :: discard
type (ESMF_GridComp), target :: gc
integer :: rc, userRc
integer :: pet


! Gridded component
gc = ESMF_GridCompCreate(petList=[(pet,pet=0,this%getNumPETsRequested()-1)], rc=rc)
if (rc /= ESMF_SUCCESS) call throw('Insufficient PETs for request')

this%gc => gc
this%val = 4

call this%setInternalState(gc,rc=rc)
if (rc /= ESMF_SUCCESS) call throw('Insufficient PETs for request')

! create subcommunicator
this%context = this%parentContext%makeSubcontext(this%getNumPETsRequested())

Expand Down Expand Up @@ -86,9 +96,9 @@ recursive subroutine runBare(this)
call gatherExceptions(this%parentContext)

call this%clearInternalState(gc, rc=rc)
if (rc /= ESMF_SUCCESS) call throw('Failure in ESMF_GridCompFinalize()')
if (rc /= ESMF_SUCCESS) call throw('Failure clearing internal state')

end subroutine runBare
end subroutine runbare_inner

subroutine setInternalState(this, gc, rc)
class (ESMF_TestCase), target, intent(inout) :: this
Expand Down Expand Up @@ -127,11 +137,11 @@ subroutine clearInternalState(this, gc, rc)
deallocate(this%wrapped%wrapped)
deallocate(this%wrapped)

call ESMF_GridCompDestroy(gc, rc=status)
if (status /= ESMF_SUCCESS) then
rc = status
return
end if
!!$ call ESMF_GridCompDestroy(gc, rc=status)
!!$ if (status /= ESMF_SUCCESS) then
!!$ rc = status
!!$ return
!!$ end if
rc = ESMF_SUCCESS

end subroutine clearInternalState
Expand Down Expand Up @@ -161,7 +171,8 @@ subroutine initialize(comp, importState, exportState, clock, rc)
end if

! Access private data block and verify data
testPtr => wrap%wrapped%testPtr
testPtr => wrap%wrapped%testPtr

call testPtr%setUp()

rc = finalrc
Expand Down Expand Up @@ -236,7 +247,7 @@ subroutine finalize(comp, importState, exportState, clock, rc)

end subroutine finalize

subroutine setServices(comp, rc)
subroutine setServices(comp, rc)
type(ESMF_GridComp) :: comp ! must not be optional
integer, intent(out) :: rc ! must not be optional

Expand Down
5 changes: 2 additions & 3 deletions pfunit/ESMF_TestMethod.F90
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ module ESMF_TestMethod_mod
private

public :: ESMF_TestMethod
public :: newESMF_TestMethod

type, extends(ESMF_TestCase) :: ESMF_TestMethod
procedure(esmfMethod), pointer :: userMethod => null()
Expand All @@ -26,10 +25,10 @@ subroutine esmfMethod(this)
end subroutine esmfMethod
end interface

interface newEsmf_TestMethod
interface Esmf_TestMethod
module procedure newEsmf_TestMethod_basic
module procedure newEsmf_TestMethod_setUpTearDown
end interface newEsmf_TestMethod
end interface Esmf_TestMethod

contains

Expand Down

0 comments on commit 1e204be

Please sign in to comment.