From 761ab252d07a3058fe8a7cd8c9ff25ec9f39bb68 Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 1 Aug 2023 11:52:49 -0400 Subject: [PATCH 1/2] Fixes #2283 - NAG and dangling pointers --- CHANGELOG.md | 1 + base/FileMetadataUtilities.F90 | 2 +- base/MAPL_GridManager.F90 | 6 +++--- griddedio/DataCollection.F90 | 2 +- griddedio/FieldBundleRead.F90 | 2 +- griddedio/GriddedIO.F90 | 4 +++- pfio/AbstractDirectoryService.F90 | 2 +- pfio/AbstractServer.F90 | 2 +- pfio/AbstractSocket.F90 | 2 +- pfio/ClientThread.F90 | 4 ++-- pfio/DirectoryService.F90 | 2 +- pfio/ExtDataCollection.F90 | 2 +- pfio/FileMetadata.F90 | 2 +- pfio/HistoryCollection.F90 | 6 +++--- pfio/MessageVisitor.F90 | 12 ++++++------ pfio/MpiSocket.F90 | 2 +- pfio/MultiCommServer.F90 | 2 +- pfio/MultiGroupServer.F90 | 2 +- pfio/NetCDF4_FileFormatter.F90 | 8 ++++---- pfio/ServerThread.F90 | 12 ++++++------ pfio/SimpleSocket.F90 | 2 +- 21 files changed, 41 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8048e73e60fc..e68c1ea153e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed missing TARGET attribute on dummy argument. NAG aggressively uses copy-in/copy-out which exposes these missing attributes. This fix probably did not find all - just the ones exercised by one failing test. - Workaround for NAG which prevents reading values from ESMF Config files that have been set using `SetAttribute()`. The immediate issue appears to be due to a wrong CPP conditional on `ESMF_HAS_ACHAR_BUG', but it is not immediately clear if this is due to recent changes in ESMF or some change in NAG. Probably ESMF though. Once the ESMF core team analyzes we will potentially update this fix. ## [2.40.0] - 2023-07-29 diff --git a/base/FileMetadataUtilities.F90 b/base/FileMetadataUtilities.F90 index 0c2b5a9f650b..ea8c858f7ccb 100644 --- a/base/FileMetadataUtilities.F90 +++ b/base/FileMetadataUtilities.F90 @@ -595,7 +595,7 @@ subroutine get_coordinate_info(this,coordinate_name,coordSize,coordUnits,long_na end subroutine get_coordinate_info function get_level_name(this,rc) result(lev_name) - class (FileMetadataUtils), intent(inout) :: this + class (FileMetadataUtils), target, intent(inout) :: this integer, optional, intent(out) :: rc character(len=:), pointer :: units diff --git a/base/MAPL_GridManager.F90 b/base/MAPL_GridManager.F90 index ddee627d4297..5808d12ac5bb 100644 --- a/base/MAPL_GridManager.F90 +++ b/base/MAPL_GridManager.F90 @@ -196,7 +196,7 @@ end function make_clone subroutine add_factory(this, factory, id) - class (GridManager), intent(inout) :: this + class (GridManager), target, intent(inout) :: this class (AbstractGridFactory), intent(in) :: factory integer(kind=ESMF_KIND_I8), optional, intent(out) :: id @@ -240,7 +240,7 @@ function make_grid_from_factory(this, factory, unusable, rc) result(grid) use ESMF type (ESMF_Grid) :: grid - class (GridManager), intent(inout) :: this + class (GridManager), target, intent(inout) :: this class (AbstractGridFactory), intent(in) :: factory class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: rc @@ -430,7 +430,7 @@ end subroutine delete function get_factory(this, grid, unusable, rc) result(factory) class (AbstractGridFactory), pointer :: factory - class (GridManager), intent(in) :: this + class (GridManager), target, intent(in) :: this type (ESMF_Grid), intent(in) :: grid class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: rc diff --git a/griddedio/DataCollection.F90 b/griddedio/DataCollection.F90 index c881a4f8bd04..7100792fc5e3 100644 --- a/griddedio/DataCollection.F90 +++ b/griddedio/DataCollection.F90 @@ -51,7 +51,7 @@ end function new_MAPLDataCollection function find(this, file_name, rc) result(metadata) type (FileMetadataUtils), pointer :: metadata - class (MAPLDataCollection), intent(inout) :: this + class (MAPLDataCollection), target, intent(inout) :: this character(len=*), intent(in) :: file_name integer, optional, intent(out) :: rc diff --git a/griddedio/FieldBundleRead.F90 b/griddedio/FieldBundleRead.F90 index 177d6d0f6cac..220058566215 100644 --- a/griddedio/FieldBundleRead.F90 +++ b/griddedio/FieldBundleRead.F90 @@ -192,7 +192,7 @@ subroutine MAPL_read_bundle(bundle,file_tmpl,time,only_vars,regrid_method,noread exit end if end do - _ASSERT(time_index/=-1,"Time not found on file"//trim(file_name)) + _ASSERT(time_index/=-1,"Time not found on file "//trim(file_name)) deallocate(time_series) call ESMF_FieldBundleGet(bundle,fieldCount=num_fields,rc=status) diff --git a/griddedio/GriddedIO.F90 b/griddedio/GriddedIO.F90 index 682ff6733a4b..3b2bfb3c6a52 100644 --- a/griddedio/GriddedIO.F90 +++ b/griddedio/GriddedIO.F90 @@ -455,7 +455,7 @@ subroutine modifyTimeIncrement(this, frequency, rc) end subroutine modifyTimeIncrement subroutine bundlepost(this,filename,oClients,rc) - class (MAPL_GriddedIO), intent(inout) :: this + class (MAPL_GriddedIO), target, intent(inout) :: this character(len=*), intent(in) :: filename type (ClientManager), optional, intent(inout) :: oClients integer, optional, intent(out) :: rc @@ -546,6 +546,8 @@ subroutine RegridScalar(this,itemName,rc) type(ESMF_Grid) :: gridIn,gridOut logical :: hasDE_in, hasDE_out + ptr3d => null() + call ESMF_FieldBundleGet(this%output_bundle,itemName,field=outField,rc=status) _VERIFY(status) call ESMF_FieldBundleGet(this%input_bundle,grid=gridIn,rc=status) diff --git a/pfio/AbstractDirectoryService.F90 b/pfio/AbstractDirectoryService.F90 index 28fa1302a949..2eceb9d4c121 100644 --- a/pfio/AbstractDirectoryService.F90 +++ b/pfio/AbstractDirectoryService.F90 @@ -39,7 +39,7 @@ subroutine connect_to_server(this, port_name, client, client_comm, unusable, ser import KeywordEnforcer class (AbstractDirectoryService), target, intent(inout) :: this character(len=*), intent(in) :: port_name - class (ClientThread), intent(inout) :: client + class (ClientThread), target, intent(inout) :: client integer, intent(in) :: client_comm class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: server_size diff --git a/pfio/AbstractServer.F90 b/pfio/AbstractServer.F90 index daebdf4dadba..693ca9161e1c 100644 --- a/pfio/AbstractServer.F90 +++ b/pfio/AbstractServer.F90 @@ -252,7 +252,7 @@ subroutine update_status(this, rc) end subroutine update_status subroutine clean_up(this, rc) - class(AbstractServer),intent(inout) :: this + class(AbstractServer),target, intent(inout) :: this integer, optional, intent(out) :: rc type(StringInteger64MapIterator) :: iter diff --git a/pfio/AbstractSocket.F90 b/pfio/AbstractSocket.F90 index e92d40447c87..b3812597077f 100644 --- a/pfio/AbstractSocket.F90 +++ b/pfio/AbstractSocket.F90 @@ -31,7 +31,7 @@ subroutine send(this, message, rc) use pFIO_AbstractMessageMod import AbstractSocket implicit none - class (AbstractSocket), intent(inout) :: this + class (AbstractSocket), target, intent(inout) :: this class (AbstractMessage), intent(in) :: message integer, optional, intent(out) :: rc end subroutine send diff --git a/pfio/ClientThread.F90 b/pfio/ClientThread.F90 index f8e3c0491c6b..9d9c17b76df3 100644 --- a/pfio/ClientThread.F90 +++ b/pfio/ClientThread.F90 @@ -131,14 +131,14 @@ end function add_ext_collection function add_hist_collection(this, fmd, unusable, mode, rc) result(hist_collection_id) integer :: hist_collection_id - class (ClientThread), intent(inout) :: this + class (ClientThread), target, intent(inout) :: this type(FileMetadata),intent(in) :: fmd class (KeywordEnforcer), optional, intent(out) :: unusable integer, optional, intent(in) :: mode integer, optional, intent(out) :: rc class (AbstractMessage), pointer :: message - class(AbstractSocket),pointer :: connection + class(AbstractSocket), pointer :: connection integer :: status connection=>this%get_connection() diff --git a/pfio/DirectoryService.F90 b/pfio/DirectoryService.F90 index ee940fec6075..c8970d181e57 100644 --- a/pfio/DirectoryService.F90 +++ b/pfio/DirectoryService.F90 @@ -147,7 +147,7 @@ subroutine connect_to_server(this, port_name, client, client_comm, unusable, ser use pFIO_ClientThreadMod class (DirectoryService), target, intent(inout) :: this character(*), intent(in) :: port_name - class (ClientThread), intent(inout) :: client + class (ClientThread), target, intent(inout) :: client integer, intent(in) :: client_comm class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: server_size diff --git a/pfio/ExtDataCollection.F90 b/pfio/ExtDataCollection.F90 index fc2f7371f456..29552439476b 100644 --- a/pfio/ExtDataCollection.F90 +++ b/pfio/ExtDataCollection.F90 @@ -45,7 +45,7 @@ end function new_ExtDataCollection function find(this, file_name, rc) result(formatter) type (NetCDF4_FileFormatter), pointer :: formatter - class (ExtDataCollection), intent(inout) :: this + class (ExtDataCollection), target, intent(inout) :: this character(len=*), intent(in) :: file_name integer, optional, intent(out) :: rc diff --git a/pfio/FileMetadata.F90 b/pfio/FileMetadata.F90 index 634617bb6c79..9a6c59fb2b90 100644 --- a/pfio/FileMetadata.F90 +++ b/pfio/FileMetadata.F90 @@ -370,7 +370,7 @@ end subroutine set_order subroutine add_variable(this, var_name, var, unusable, rc) class (FileMetadata), target, intent(inout) :: this character(len=*), intent(in) :: var_name - class (Variable), intent(in) :: var + class (Variable), target, intent(in) :: var class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: rc diff --git a/pfio/HistoryCollection.F90 b/pfio/HistoryCollection.F90 index 22ea616b274b..e191a19922fb 100644 --- a/pfio/HistoryCollection.F90 +++ b/pfio/HistoryCollection.F90 @@ -42,7 +42,7 @@ function new_HistoryCollection(fmd) result(collection) end function new_HistoryCollection function find(this, file_name,rc) result(formatter) - class (HistoryCollection), intent(inout) :: this + class (HistoryCollection), target, intent(inout) :: this character(len=*), intent(in) :: file_name integer,optional,intent(out) :: rc @@ -73,7 +73,7 @@ function find(this, file_name,rc) result(formatter) end function find subroutine ModifyMetadata(this,var_map,rc) - class (HistoryCollection), intent(inout) :: this + class (HistoryCollection), target, intent(inout) :: this type (StringVariableMap), intent(in) :: var_map integer, optional, intent(out) :: rc @@ -105,7 +105,7 @@ subroutine ReplaceMetadata(this, fmd,rc) end subroutine ReplaceMetadata subroutine clear(this, rc) - class (HistoryCollection), intent(inout) :: this + class (HistoryCollection), target, intent(inout) :: this integer, optional, intent(out) :: rc type(NetCDF4_FileFormatter), pointer :: f_ptr diff --git a/pfio/MessageVisitor.F90 b/pfio/MessageVisitor.F90 index a61123adc191..21f5da6b1dbb 100644 --- a/pfio/MessageVisitor.F90 +++ b/pfio/MessageVisitor.F90 @@ -138,7 +138,7 @@ recursive subroutine handle(this, message, rc) end subroutine handle subroutine handle_CollectivePrefetchData(this, message, rc) - class (MessageVisitor), intent(inout) :: this + class (MessageVisitor), target, intent(inout) :: this type (CollectivePrefetchDataMessage), intent(in) :: message integer, optional, intent(out) :: rc _FAIL( "Warning : dummy handle_CollectivePrefetchData should not be called") @@ -147,7 +147,7 @@ subroutine handle_CollectivePrefetchData(this, message, rc) end subroutine handle_CollectivePrefetchData subroutine handle_CollectiveStageData(this, message, rc) - class (MessageVisitor), intent(inout) :: this + class (MessageVisitor), target, intent(inout) :: this type (CollectiveStageDataMessage), intent(in) :: message integer, optional, intent(out) :: rc _FAIL( "Warning : dummy handle_CollectiveStageData should not be called") @@ -237,7 +237,7 @@ subroutine handle_Id(this, message, rc) end subroutine handle_Id subroutine handle_PrefetchData(this, message, rc) - class (MessageVisitor), intent(inout) :: this + class (MessageVisitor), target, intent(inout) :: this type (PrefetchDataMessage), intent(in) :: message integer, optional, intent(out) :: rc _FAIL( "Warning : dummy handle_PrefetchData should not be called") @@ -246,7 +246,7 @@ subroutine handle_PrefetchData(this, message, rc) end subroutine handle_PrefetchData subroutine handle_StageData(this, message, rc) - class (MessageVisitor), intent(inout) :: this + class (MessageVisitor), target, intent(inout) :: this type (StageDataMessage), intent(in) :: message integer, optional, intent(out) :: rc _FAIL( "Warning : dummy handle_StageData should not be called") @@ -255,7 +255,7 @@ subroutine handle_StageData(this, message, rc) end subroutine handle_StageData subroutine handle_ModifyMetadata(this, message, rc) - class (MessageVisitor), intent(inout) :: this + class (MessageVisitor), target, intent(inout) :: this type (ModifyMetadataMessage), intent(in) :: message integer, optional, intent(out) :: rc _FAIL( "Warning : dummy handle_ModifyMetadata should not be called") @@ -264,7 +264,7 @@ subroutine handle_ModifyMetadata(this, message, rc) end subroutine handle_ModifyMetadata subroutine handle_ReplaceMetadata(this, message, rc) - class (MessageVisitor), intent(inout) :: this + class (MessageVisitor), target, intent(inout) :: this type (ReplaceMetadataMessage), intent(in) :: message integer, optional, intent(out) :: rc _FAIL( "Warning : dummy handle_ReplaceMetadata should not be called") diff --git a/pfio/MpiSocket.F90 b/pfio/MpiSocket.F90 index 6f29569021c6..9fc75f1b1db0 100644 --- a/pfio/MpiSocket.F90 +++ b/pfio/MpiSocket.F90 @@ -120,7 +120,7 @@ function receive(this, rc) result(message) end function receive subroutine send(this, message, rc) - class (MpiSocket), intent(inout) :: this + class (MpiSocket), target, intent(inout) :: this class (AbstractMessage), intent(in) :: message integer, optional, intent(out) :: rc diff --git a/pfio/MultiCommServer.F90 b/pfio/MultiCommServer.F90 index 99cd49a4c034..48b55de61553 100644 --- a/pfio/MultiCommServer.F90 +++ b/pfio/MultiCommServer.F90 @@ -432,7 +432,7 @@ subroutine put_DataToFile(this, rc) end subroutine put_DataToFile subroutine clean_up(this, rc) - class(MultiCommServer),intent(inout) :: this + class(MultiCommServer), target, intent(inout) :: this integer, optional, intent(out) :: rc class (ServerThread),pointer :: threadPtr class (AbstractMessage),pointer :: msg diff --git a/pfio/MultiGroupServer.F90 b/pfio/MultiGroupServer.F90 index 45fd8d5722b4..457fcc395c7d 100644 --- a/pfio/MultiGroupServer.F90 +++ b/pfio/MultiGroupServer.F90 @@ -272,7 +272,7 @@ subroutine put_DataToFile(this, rc) end subroutine put_DataToFile subroutine clean_up(this, rc) - class(MultiGroupServer),intent(inout) :: this + class(MultiGroupServer),target, intent(inout) :: this integer, optional, intent(out) :: rc type(StringInteger64MapIterator) :: iter integer :: num_clients, n diff --git a/pfio/NetCDF4_FileFormatter.F90 b/pfio/NetCDF4_FileFormatter.F90 index dc73decc40ae..923241c43b08 100644 --- a/pfio/NetCDF4_FileFormatter.F90 +++ b/pfio/NetCDF4_FileFormatter.F90 @@ -350,7 +350,7 @@ end subroutine write subroutine def_dimensions(this, cf, unusable, rc) class (NetCDF4_FileFormatter), intent(inout) :: this - type (FileMetadata), intent(in) :: cf + type (FileMetadata), target, intent(in) :: cf class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: rc @@ -389,7 +389,7 @@ end subroutine def_dimensions subroutine put_attributes(this, cf, varid, unusable, rc) class (NetCDF4_FileFormatter), intent(inout) :: this - type (FileMetadata), intent(in) :: cf + type (FileMetadata), target, intent(in) :: cf integer, intent(in) :: varid class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: rc @@ -480,7 +480,7 @@ end subroutine put_attributes subroutine write_const_variables(this, cf, unusable, rc) class (NetCDF4_FileFormatter), intent(inout) :: this - type (FileMetadata), intent(in) :: cf + type (FileMetadata), target, intent(in) :: cf class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: rc @@ -534,7 +534,7 @@ end subroutine write_const_variables subroutine write_coordinate_variables(this, cf, unusable, rc) class (NetCDF4_FileFormatter), intent(inout) :: this - type (FileMetadata), intent(in) :: cf + type (FileMetadata), target, intent(in) :: cf class (KeywordEnforcer), optional, intent(in) :: unusable integer, optional, intent(out) :: rc diff --git a/pfio/ServerThread.F90 b/pfio/ServerThread.F90 index 58a3aa5f2b58..93c04a9a3e33 100644 --- a/pfio/ServerThread.F90 +++ b/pfio/ServerThread.F90 @@ -545,7 +545,7 @@ subroutine handle_AddHistCollection(this, message, rc) end subroutine handle_AddHistCollection subroutine handle_PrefetchData(this, message, rc) - class (ServerThread), intent(inout) :: this + class (ServerThread), target, intent(inout) :: this type (PrefetchDataMessage), intent(in) :: message integer, optional, intent(out) :: rc @@ -561,7 +561,7 @@ subroutine handle_PrefetchData(this, message, rc) end subroutine handle_PrefetchData subroutine handle_CollectivePrefetchData(this, message, rc) - class (ServerThread), intent(inout) :: this + class (ServerThread), target, intent(inout) :: this type (CollectivePrefetchDataMessage), intent(in) :: message integer, optional, intent(out) :: rc @@ -577,7 +577,7 @@ subroutine handle_CollectivePrefetchData(this, message, rc) end subroutine handle_CollectivePrefetchData subroutine handle_ModifyMetadata(this, message, rc) - class (ServerThread), intent(inout) :: this + class (ServerThread), target, intent(inout) :: this type (ModifyMetadataMessage), intent(in) :: message integer, optional, intent(out) :: rc @@ -596,7 +596,7 @@ subroutine handle_ModifyMetadata(this, message, rc) end subroutine handle_ModifyMetadata subroutine handle_ReplaceMetadata(this, message, rc) - class (ServerThread), intent(inout) :: this + class (ServerThread), target, intent(inout) :: this type (ReplaceMetadataMessage), intent(in) :: message integer, optional, intent(out) :: rc @@ -739,7 +739,7 @@ subroutine get_DataFromFile(this,message,address, rc) end subroutine get_DataFromFile subroutine handle_StageData(this, message, rc) - class (ServerThread), intent(inout) :: this + class (ServerThread), target, intent(inout) :: this type (StageDataMessage), intent(in) :: message integer, optional, intent(out) :: rc @@ -761,7 +761,7 @@ subroutine handle_StageData(this, message, rc) end subroutine handle_StageData subroutine handle_CollectiveStageData(this, message, rc) - class (ServerThread), intent(inout) :: this + class (ServerThread), target, intent(inout) :: this type (CollectiveStageDataMessage), intent(in) :: message integer, optional, intent(out) :: rc diff --git a/pfio/SimpleSocket.F90 b/pfio/SimpleSocket.F90 index 65344ab9d9d9..5b08ff625d46 100644 --- a/pfio/SimpleSocket.F90 +++ b/pfio/SimpleSocket.F90 @@ -92,7 +92,7 @@ function receive(this, rc) result(message) end function receive recursive subroutine send(this, message, rc) - class (SimpleSocket), intent(inout) :: this + class (SimpleSocket), target, intent(inout) :: this class (AbstractMessage), intent(in) :: message class (AbstractSocket),pointer :: connection integer, optional, intent(out) :: rc From 63b7e30f497e8280b3110bd04a0942b9e0a3c5ef Mon Sep 17 00:00:00 2001 From: Matthew Thompson Date: Tue, 1 Aug 2023 11:57:35 -0400 Subject: [PATCH 2/2] Update for release --- CHANGELOG.md | 7 ++++++- CMakeLists.txt | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e68c1ea153e4..fc9bdd1a8b83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,11 +17,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Deprecated -## [2.40.1] - 2023-08-01 +## [2.40.2] - 2023-08-01 ### Fixed - Fixed missing TARGET attribute on dummy argument. NAG aggressively uses copy-in/copy-out which exposes these missing attributes. This fix probably did not find all - just the ones exercised by one failing test. + +## [2.40.1] - 2023-08-01 + +### Fixed + - Workaround for NAG which prevents reading values from ESMF Config files that have been set using `SetAttribute()`. The immediate issue appears to be due to a wrong CPP conditional on `ESMF_HAS_ACHAR_BUG', but it is not immediately clear if this is due to recent changes in ESMF or some change in NAG. Probably ESMF though. Once the ESMF core team analyzes we will potentially update this fix. ## [2.40.0] - 2023-07-29 diff --git a/CMakeLists.txt b/CMakeLists.txt index fdcb220875ba..825bba06abca 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ endif () project ( MAPL - VERSION 2.40.1 + VERSION 2.40.2 LANGUAGES Fortran CXX C) # Note - CXX is required for ESMF # Set the possible values of build type for cmake-gui