Fix memory leaks and other issues identified by the code sanitizer

CHANGELOG.md

@@ -5,10 +5,20 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [3.5.1] - 2022-11-03
+## Unreleased [3.5.2]
+### Added
+  - Added sanitizer option for detecting memory leaks in HEMCO
+    standalone during build
+
+### Changed
+  - Remove unused, commented-out code in `src/Extensions/hcox_dustdead_mod.F`
+
 ### Fixed
-- Changed Inst%NP to Inst%NumP in HCOX_Seasalt_Mod for CESM compatibility
+  - Removed memory leaks that were identified by the code sanitizer
 
+## [3.5.1] - 2022-11-03
+### Fixed
+  - Changed Inst%NP to Inst%NumP in HCOX_Seasalt_Mod for CESM compatibility
 
 ## [3.5.0] - 2022-09-19
 ### Added
@@ -341,7 +351,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
   - Various updates to PARANOX:
     - Bug fix in calculation of H2O ambient air concentration'
-	- Bug fix in computation of solar zenith angle for the current date
+ - Bug fix in computation of solar zenith angle for the current date
       calculation of the current date SZA;
     - Loss fluxes of O3 and HNO3 are now passed in kg/m2/s via the
         HEMCO diagnostics instead of converting them to a deposition
@@ -354,12 +364,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Various updates to the HEMCO standalone code
   - Modifications to on/off switches in `HEMCO_Config.rc`:
     - Extension names can be used as switches
-	- Multiple switches can be combined with `.or`
+ - Multiple switches can be combined with `.or`
 
 ### Changed
   - HEMCO has now two run phases:
-      - Phase 1 reads the HEMCO list
-	  - Phase 2 calculates emissions.
+  - Phase 1 reads the HEMCO list
+  - Phase 2 calculates emissions.
   - Environmental fields used by HEMCO (stored in the `ExtState`
     object) can now be read directly from disk.
   - Various updates to the HEMCO standalone code
@@ -373,11 +383,11 @@ Initial HEMCO release
   - Bug fixes for the BIOGENIC_OCPI diagnostic
   - Bug fixes in the computation of alkalinity
   - PARANOx updates:
-    - Can now read the lookup table from netCDF or ASCII format
-	- Wind speed is now accounted for in the parameterization
-	- Dry deposition of N is included va loss of HNO3.
-	- Total tropospheric column mass is used to calculate dry
-      deposition frequencies.
+  - Can now read the lookup table from netCDF or ASCII format
+  - Wind speed is now accounted for in the parameterization
+  - Dry deposition of N is included va loss of HNO3.
+  - Total tropospheric column mass is used to calculate dry
+    deposition frequencies.
   - Local times can now be calculated based on a time zone map (at 1x1
     degree resolution).
   - Non-emissions data may now be specified in `HEMCO_Config.rc` by

CMakeLists.txt

@@ -263,6 +263,36 @@ if(NOT HEMCO_EXTERNAL_CONFIG)
         CACHE STRING "Executable targets that get built as a part of \"all\""
     )
 
+    # Add code sanitization options (GNU Fortran only) to HEMCO standalone
+    # We need to add these options to both compiler & linker.
+    set(SANITIZE OFF CACHE BOOL
+        "Switch to turn on code sanitation (i.e. identify memory leaks and similar conditions)"
+    )
+    if(${SANITIZE})
+      if(CMAKE_Fortran_COMPILER_ID STREQUAL "GNU")
+	target_compile_options(HEMCOBuildProperties
+          INTERFACE "-fsanitize=address"
+	)
+	target_link_libraries(HEMCOBuildProperties
+          INTERFACE "-fsanitize=address"
+	)
+	target_compile_options(HEMCOBuildProperties
+          INTERFACE "-fsanitize=leak"
+	)
+	target_link_libraries(HEMCOBuildProperties
+          INTERFACE "-fsanitize=leak"
+	)
+	target_compile_options(HEMCOBuildProperties
+          INTERFACE "-fsanitize=undefined"
+	)
+	target_link_libraries(HEMCOBuildProperties
+          INTERFACE "-fsanitize=undefined"
+	)
+      else()
+        message( FATAL_ERROR "The SANITIZE option is only defined for GNU Fortran.")
+      endif()
+    endif()
+
 endif()
 
 #-----------------------------------------------------------------------------
@@ -405,6 +435,7 @@ if(NOT HEMCO_EXTERNAL_CONFIG)
     hco_pretty_print(SECTION "Settings")
     hco_pretty_print(VARIABLE OMP IS_BOOLEAN)
     hco_pretty_print(VARIABLE USE_REAL8 IS_BOOLEAN)
+    hco_pretty_print(VARIABLE SANITIZE IS_BOOLEAN)
 endif()
 
 #-----------------------------------------------------------------------------

src/Core/hco_clock_mod.F90

@@ -1297,10 +1297,36 @@ SUBROUTINE HcoClock_Cleanup ( Clock )
     !======================================================================
     ! HcoClock_Cleanup begins here!
     !======================================================================
-
     IF ( ASSOCIATED( Clock ) ) THEN
-       ! Make sure TIMEZONES array does not point to any content any more.
-       CALL HCO_ArrCleanup( Clock%TIMEZONES, DeepClean=.FALSE.)
+
+       ! Make sure TimeZones does not point to any content any more.
+       CALL HCO_ArrCleanup( Clock%TimeZones, DeepClean=.FALSE. )
+
+       ! We also need to free the pointer fields in the Clock object
+       IF ( ASSOCIATED( Clock%ThisLocYear ) ) THEN
+          DEALLOCATE( Clock%ThisLocYear )
+       ENDIF
+       Clock%ThisLocYear => NULL()
+
+       IF ( ASSOCIATED( Clock%ThisLocMonth ) ) THEN
+          DEALLOCATE( Clock%ThisLocMonth )
+       ENDIF
+       Clock%ThisLocMonth => NULL()
+
+       IF ( ASSOCIATED( Clock%ThisLocDay ) ) THEN
+          DEALLOCATE( Clock%ThisLocDay )
+       ENDIF
+       Clock%ThisLocDay => NULL()
+
+       IF ( ASSOCIATED( Clock%ThisLocWD ) ) THEN
+          DEALLOCATE( Clock%ThisLocWD )
+       ENDIF
+       Clock%ThisLocWD => NULL()
+
+       IF ( ASSOCIATED( Clock%ThisLocHour ) ) THEN
+          DEALLOCATE( Clock%ThisLocHour )
+       ENDIF
+       Clock%ThisLocHour => NULL()
 
        DEALLOCATE ( Clock )
     ENDIF

src/Core/hco_config_mod.F90

@@ -921,8 +921,9 @@ SUBROUTINE Config_ReadCont( HcoConfig, IU_HCO,  CFDIR,          &
                 ENDIF
                 Lct%Dct%DtaHome = Lct%Dct%DtaHome - 1
              ELSE
-                Dta => NULL()
-                CALL FileData_Init ( Dta )
+                ! NOTE: FileData_Init now nullifies the Dta object
+                ! befire allocation.  -- Bob Yantosca (22 Aug 2022)
+                CALL FileData_Init( Dta )
 
                 ! Set source file name. Check if the read file name starts
                 ! with the configuration file token '$CFDIR', in which case
@@ -1119,8 +1120,9 @@ SUBROUTINE Config_ReadCont( HcoConfig, IU_HCO,  CFDIR,          &
              ENDIF
              Lct%Dct%DtaHome = Lct%Dct%DtaHome - 1
           ELSE
-             Dta => NULL()
-             CALL FileData_Init ( Dta )
+             ! NOTE: FileData_Init now nullifies the Dta object
+             ! before allocation. -- Bob Yantosca (22 Aug 2022)
+             CALL FileData_Init( Dta )
 
              ! Set source file name. Check if the read file name starts
              ! with the configuration file token '$CFDIR', in which case
@@ -3591,8 +3593,7 @@ SUBROUTINE ConfigList_AddCont( Lct, List )
 !
 ! !LOCAL VARIABLES:
 !
-    TYPE(ListCont), POINTER :: NewLct
-    INTEGER                 :: cID
+    INTEGER :: cID
 
     !======================================================================
     ! ConfigList_AddCont begins here
@@ -3602,22 +3603,20 @@ SUBROUTINE ConfigList_AddCont( Lct, List )
     ! The DataCont_Init call creates a new data container (type DataCont)
     ! All HEMCO lists (ConfigList, ReadList, EmisList) point to this
     ! container!
-    ALLOCATE ( NewLct )
-    NewLct%Dct      => NULL()
-    NewLct%NextCont => NULL()
+    ALLOCATE ( Lct )
+    Lct%Dct      => NULL()
+    Lct%NextCont => NULL()
 
     ! Get # of containers in list. Set new container ID (cID) to # of
     ! containers + 1.
     cID = ListCont_Length( List )
     cID = cID + 1
-    CALL DataCont_Init ( NewLct%Dct, cID )
+    CALL DataCont_Init ( Lct%Dct, cID )
 
     ! Connect blank container with ConfigList list.
-    NewLct%NextCont => List
-    List            => NewLct
+    Lct%NextCont => List
+    List         => Lct
 
-    ! Output pointer points to the new container
-    Lct => NewLct
 
   END SUBROUTINE ConfigList_AddCont
 !EOC

src/Core/hco_datacont_mod.F90

@@ -264,8 +264,10 @@ SUBROUTINE DataCont_Cleanup( Dct, ArrOnly )
        ! Clean up data container if DeepClean option is enabled.
        IF ( DeepClean ) THEN
           Dct%Dta => NULL()
-          IF(ASSOCIATED(Dct%Scal_cID)) DEALLOCATE(Dct%Scal_cID)
-          DEALLOCATE ( Dct )
+          IF( ASSOCIATED( Dct%Scal_cID ) ) DEALLOCATE( Dct%Scal_cID )
+          Dct%Scal_cID => NULL()
+          DEALLOCATE( Dct )
+          Dct => NULL()
        ENDIF
 
     ENDIF

src/Core/hco_diagn_mod.F90

@@ -3867,7 +3867,7 @@ SUBROUTINE DiagnCollection_Create ( Diagn,    NX, NY, NZ,    &
 !
 ! !ARGUMENTS:
 !
-    TYPE(DiagnCollection), POINTER :: NewCollection => NULL()
+    TYPE(DiagnCollection), POINTER :: NewCollection
     INTEGER                        :: PS
     CHARACTER(LEN=255)             :: MSG
     CHARACTER(LEN=255)             :: LOC = 'DiagnCollection_Create (hco_diagn_mod.F90)'
@@ -3994,7 +3994,13 @@ SUBROUTINE DiagnCollection_Cleanup ( Diagn )
        ! Advance
        NextColl                => ThisColl%NextCollection
        ThisColl%NextCollection => NULL()
+
+       ! Free the memory given to ThisColl to avoid memory leaks
+       DEALLOCATE( ThisColl )
+
+       ! Point to the next collection in the list for the next iteration
        ThisColl                => NextColl
+
     ENDDO
 
   END SUBROUTINE DiagnCollection_Cleanup

src/Core/hco_extlist_mod.F90

@@ -1430,9 +1430,9 @@ SUBROUTINE HCO_CleanupOpt ( OptList )
        ! Archive next option in list
        NextOpt => ThisOpt%NextOpt
 
-       ! Cleanup option
+       ! Free the memory allocated to ThisOpt (this avoids memory leaks)
        ThisOpt%NextOpt => NULL()
-       NULLIFY(ThisOpt)
+       DEALLOCATE( ThisOpt )
 
        ! Go to next option in list (previously archived)
        ThisOpt => NextOpt

src/Core/hco_filedata_mod.F90

@@ -166,50 +166,47 @@ SUBROUTINE FileData_Init( FileDta )
 !
 ! !LOCAL VARIABLES:
 !
-    TYPE(FileData), POINTER  :: NewFDta
-
     !======================================================================
     ! FileData_Init begins here!
     !======================================================================
 
-    ! Allocate the new file data object
-    ALLOCATE( NewFDta )
+    FileDta => NULL()
+    ALLOCATE( FileDta )
 
     ! Nullify all pointers and initialize variables
-    NewFDta%V3          => NULL()
-    NewFDta%V2          => NULL()
-    NewFDta%tIDx        => NULL()
-    NewFDta%ncFile       = ''
-    NewFDta%ncPara       = ''
-    NewFDta%ncYrs(:)     = -999
-    NewFDta%ncMts(:)     = -999
-    NewFDta%ncDys(:)     = -999
-    NewFDta%ncHrs(:)     = -999
-    NewFDta%tShift(:)    = 0
-    NewFDta%CycleFlag    = HCO_CFLAG_CYCLE
-    NewFDta%UpdtFlag     = HCO_UFLAG_FROMFILE
-    NewFDta%MustFind     = .FALSE.
-    NewFDta%ncRead       = .TRUE.
-    NewFDta%Cover        = -999
-    NewFDta%DeltaT       = 0
-    NewFDta%nt           = 0
-    NewFDta%SpaceDim     = -1
-    NewFDta%Levels       = 0
-    NewFDta%EmisL1       = 1.0_hp
-    NewFDta%EmisL2       = 1.0_hp
-    NewFDta%EmisL1Unit   = HCO_EMISL_LEV
-    NewFDta%EmisL2Unit   = HCO_EMISL_LEV
-    NewFDta%OrigUnit     = ''
-    NewFDta%ArbDimName   = 'none'
-    NewFDta%ArbDimVal    = ''
-    NewFDta%IsLocTime    = .FALSE.
-    NewFDta%IsConc       = .FALSE.
-    NewFDta%DoShare      = .FALSE.
-    NewFDta%IsInList     = .FALSE.
-    NewFDta%IsTouched    = .FALSE.
-
-    ! Return
-    FileDta => NewFDta
+    ! NOTE: Avoid memory leaks by working on the argument FileDta instead
+    ! of pointing to a local object NewFDta (Bob Yantosca, 22 Aug 2022)
+    FileDta%V3          => NULL()
+    FileDta%V2          => NULL()
+    FileDta%tIDx        => NULL()
+    FileDta%ncFile       = ''
+    FileDta%ncPara       = ''
+    FileDta%ncYrs(:)     = -999
+    FileDta%ncMts(:)     = -999
+    FileDta%ncDys(:)     = -999
+    FileDta%ncHrs(:)     = -999
+    FileDta%tShift(:)    = 0
+    FileDta%CycleFlag    = HCO_CFLAG_CYCLE
+    FileDta%UpdtFlag     = HCO_UFLAG_FROMFILE
+    FileDta%MustFind     = .FALSE.
+    FileDta%ncRead       = .TRUE.
+    FileDta%Cover        = -999
+    FileDta%DeltaT       = 0
+    FileDta%nt           = 0
+    FileDta%SpaceDim     = -1
+    FileDta%Levels       = 0
+    FileDta%EmisL1       = 1.0_hp
+    FileDta%EmisL2       = 1.0_hp
+    FileDta%EmisL1Unit   = HCO_EMISL_LEV
+    FileDta%EmisL2Unit   = HCO_EMISL_LEV
+    FileDta%OrigUnit     = ''
+    FileDta%ArbDimName   = 'none'
+    FileDta%ArbDimVal    = ''
+    FileDta%IsLocTime    = .FALSE.
+    FileDta%IsConc       = .FALSE.
+    FileDta%DoShare      = .FALSE.
+    FileDta%IsInList     = .FALSE.
+    FileDta%IsTouched    = .FALSE.
 
   END SUBROUTINE FileData_Init
 !EOC
@@ -255,10 +252,11 @@ SUBROUTINE FileData_Cleanup( FileDta, DeepClean )
        CALL HCO_ArrCleanup( FileDta%V3, DeepClean )
        CALL HCO_ArrCleanup( FileDta%V2, DeepClean )
        FileDta%nt = 0
-
+       
        IF ( DeepClean ) THEN
           FileDta%tIDx => NULL()
           DEALLOCATE ( FileDta )
+          FileDta => NULL()
        ENDIF
     ENDIF
 

src/Core/hco_vertgrid_mod.F90

@@ -299,15 +299,20 @@ SUBROUTINE HCO_VertGrid_Cleanup( zGrid )
 !------------------------------------------------------------------------------
 !BOC
 !
-    ! Eventually deallocate Ap/Bp vectors
-    IF( ASSOCIATED(zGrid%Ap) ) THEN
-       DEALLOCATE(zGrid%Ap)
-       zGrid%Ap => NULL()
+    IF( ASSOCIATED( zGrid%Ap ) ) THEN
+       DEALLOCATE( zGrid%Ap )
     ENDIF
-    IF( ASSOCIATED(zGrid%Bp) ) THEN
-       DEALLOCATE(zGrid%Bp)
-       zGrid%Bp => NULL()
+    zGrid%Ap => NULL()
+
+    IF( ASSOCIATED( zGrid%Bp ) ) THEN
+       DEALLOCATE( zGrid%Bp )
+    ENDIF
+    zGrid%Bp => NULL()
+
+    IF ( ASSOCIATED( zGrid ) ) THEN
+       DEALLOCATE( zGrid )
     ENDIF
+    zGrid => NULL()
 
   END SUBROUTINE HCO_VertGrid_Cleanup
 !EOC