From 4adad8afe6e280a153341407fd9c38750ff0675b Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Mon, 4 Dec 2023 19:55:55 -0500 Subject: [PATCH 1/2] Fixes for NAG dangling pointers. --- CHANGELOG.md | 2 ++ base/tests/Test_SimpleMAPLcomp.pf | 2 +- base/tests/Test_SphericalToCartesian.pf | 12 ++++----- pfio/StringVariableMap.F90 | 4 +-- pfunit/ESMF_TestCase.F90 | 33 ++++++++++++++++--------- pfunit/ESMF_TestMethod.F90 | 5 ++-- 6 files changed, 35 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39e7bc125bac..679ce4e05a3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/base/tests/Test_SimpleMAPLcomp.pf b/base/tests/Test_SimpleMAPLcomp.pf index f82c70158b71..6e0081f65644 100644 --- a/base/tests/Test_SimpleMAPLcomp.pf +++ b/base/tests/Test_SimpleMAPLcomp.pf @@ -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 diff --git a/base/tests/Test_SphericalToCartesian.pf b/base/tests/Test_SphericalToCartesian.pf index d0e5bc11af5d..077577fb92b6 100644 --- a/base/tests/Test_SphericalToCartesian.pf +++ b/base/tests/Test_SphericalToCartesian.pf @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/pfio/StringVariableMap.F90 b/pfio/StringVariableMap.F90 index bc41c318131e..cf3a67c332d3 100644 --- a/pfio/StringVariableMap.F90 +++ b/pfio/StringVariableMap.F90 @@ -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 diff --git a/pfunit/ESMF_TestCase.F90 b/pfunit/ESMF_TestCase.F90 index 22ea62fa4a8d..c618f2fc99a8 100644 --- a/pfunit/ESMF_TestCase.F90 +++ b/pfunit/ESMF_TestCase.F90 @@ -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()) @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/pfunit/ESMF_TestMethod.F90 b/pfunit/ESMF_TestMethod.F90 index 499e39d5d72a..2869bb9876fc 100644 --- a/pfunit/ESMF_TestMethod.F90 +++ b/pfunit/ESMF_TestMethod.F90 @@ -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() @@ -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 From 1e0940d34ec839de98633145dcaf111581fc97d4 Mon Sep 17 00:00:00 2001 From: Matthew Thompson Date: Tue, 5 Dec 2023 10:14:10 -0500 Subject: [PATCH 2/2] Update CI for coupled runs --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c8f8f3ee4be6..0cd5bd5971a5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -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: