Skip to content

Commit

Permalink
Fixes #2283 - NAG and dangling pointers
Browse files Browse the repository at this point in the history
  • Loading branch information
tclune committed Aug 1, 2023
1 parent 3062ccf commit 761ab25
Show file tree
Hide file tree
Showing 21 changed files with 41 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion base/FileMetadataUtilities.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions base/MAPL_GridManager.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion griddedio/DataCollection.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion griddedio/FieldBundleRead.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion griddedio/GriddedIO.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pfio/AbstractDirectoryService.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pfio/AbstractServer.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pfio/AbstractSocket.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pfio/ClientThread.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion pfio/DirectoryService.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pfio/ExtDataCollection.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pfio/FileMetadata.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions pfio/HistoryCollection.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions pfio/MessageVisitor.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion pfio/MpiSocket.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pfio/MultiCommServer.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pfio/MultiGroupServer.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pfio/NetCDF4_FileFormatter.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions pfio/ServerThread.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pfio/SimpleSocket.F90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 761ab25

Please sign in to comment.