Add SANITIZE CMake option to check for memory leaks and undefined behavior #24
Conversation
…m debug We have added -g to the list of default compiler options for both Intel Fortran and GNU Fortran. This will help users to run or view the gcclassic executable in a debugger without having to recompile with -DCMAKE_BUILD_TYPE=Debug. Also removed the -gdwarf-2 -gstrict-dwarf from the list of gfortran debugging compiler options. This forced using the DWARF-2 standard, which has now been replaced by more modern versions. Signed-off-by: Bob Yantosca <[email protected]>
CMakeScripts/GC-ConfigureClassic.cmake - Add new SANITIZE cache variable for GNU Fortran only. If activated it will set -fsanitize=address -fsanitize=leak -fsanitize=undefined compiler and linker flags. This will enable checking for memory leaks, memory address faults, and code producing undefined behavior according to the Fortran standard. (NOTE: This will make code run more slowly so it should not be used in production runs). - SANITIZE will throw an error when used with compilers other than GNU Fortran. This is because the sanitization options are only included in GNU Fortran but not in Intel Fortran. CMakeLists.txt - Updated CACHE STRING values to reflect the proper CMAKE_BUILD_TYPE values: Release, RelWithDebInfo, Debug. (Uses proper capitalization) Signed-off-by: Bob Yantosca <[email protected]>
yantosca
added
category: Feature Request
|
NOTE: I will open a new branch in geoschem/geos-chem where we attempt to fix the several memory leaks that were identified. This can all go into 14.1.0. |
|
Also tagging @kilicomu, from whom I found out about this feature. |
|
The sanitation will also flag runtime issues such as: /path/to/GCClassic/src/GEOS-Chem/Headers/dictionary_m.F90:79:31: runtime error: signed integer overflow: 193452388 * 33 cannot be represented in type 'integer(kind=4)'which can be fixed by using an |
CMakeScripts/GC-ConfigClassic.cmake - We now use target_link_libraries instead of link_libraries and target_compile_options instead of add_compile_options with the SANITIZE option. The target_* commands only apply to the target whereas the other commands will be applied to all directories in the build process (which might lead to side-effects). Signed-off-by: Bob Yantosca <[email protected]>
…ture/sanitize+14.0.1
|
I am currently running GCC and GCHP integration tests with the sanitize updates merged up to GC 14.0.1 and HEMCO 3.5.1. I can create a new PR, as @lizziel has requested that we merge these fixes into GC 14.0.2, if they prove to be zero-diff (which I believe they are). |
|
@yantosca In addition to doing diff tests to ensure zero diffs, could you also see if these updates have an impact on model performance? I'm wondering if the -g flag above slows down the model at all. A simple comparison of the timers printed out at the end of the log files should be sufficient. |
|
In theory,
Definitely worth a quick timing test! Also, @yantosca it might be worth switching from
|
|
I will revert the compilation flag updates. We can add them into a separate PR which will allow for more in-depth testing. I'll create a new feature request. |
CMakeLists.txt
- Remove -g from GEOSChem_Fortran_FLAGS_{GNU,Intel}
- Add -g to GEOSChem_Fortran_FLAGS_DEBUG_{GNU,Intel}
Signed-off-by: Bob Yantosca <[email protected]>
This commit informs the GCClassic superproject about the following commits that were pushed to the Github geoschem/HEMCO repository: 5be2f32 Bug fix: Nullify Inst%GFED_EMFAC instead of deallocating it 2d56ee9 Merge branch 'main' into feature/sanitize+3.5.1 7ed469e Prevent memory leaks in routine ConfigList_AddCont (hco_config_mod.F90) 04f3278 Remove deallocation statement in hco_datacont_mod.F90 d53a5b0 Remove memory leak in hco_datacont_mod.F90 and hco_filecont_mod.F90 a9a14fc Fix memory leaks in HEMCO core routines 5120eae Add -DSANITIZE=y option to CMakeLists.txt for HEMCO standalone 7e1e171 Clean up memory leaks in extension InstRemove routines #2 1efde62 Clean up memory leaks in extension "InstRemove" routines fd6f78f Free pointer fields of HcoState%Clock in routine HcoClock_Cleanup Signed-off-by: Bob Yantosca <[email protected]>
This commit informs the GCClassic superproject about the following commits that were pushed to the GitHub geoschem/geos-chem repository: 29ae8daf0 Merge branch 'main' into feature/sanitize+14.0.1 97eecb1d0 Remove extraneous USE statement reference to HistItem_Destroy bd28bafe5 Fix memory leaks in Registry and History routines ce9ea8118 Set TimeForEmis = .FALSE. for Emissions_Run phase 0 call 36547b875 Remove memory leaks in input_opt_mod.F90 Signed-off-by: Bob Yantosca <[email protected]>
yantosca
added
the
no-diff-to-benchmark
|
I have done a parallelization test (GEOS-Chem Classic) with 8 cores vs. 5 cores. The results show that the sanitize updates are zero-diff: Using configuration file rst.yml
... Printing totals and differences
... Only showing variables with |absolute difference| > 0.0
Variable Ref=5cores Dev=8cores Dev - Ref
``` |
|
@lizziel @msulprizio I have reverted the compilation option update... -g is only used with |
|
I've ported the -g back to debug-only. We can check this out in a separate PR. |
|
NOTE: These updates to add the -DSANITIZE=y configuration option (and compiler switches) can go into 14.1.0 as this is a new feature. The sanitized code can go into GEOS-Chem 14.0.2 and HEMCO 3.5.2. |
This commit informs the GCClassic superproject about the following commits that were pushed to the GitHub geoschem/geos-chem repository: 1951ea96e Declare PPN as photolyzing in the species database 40a078514 Merge PR #2424 (Support for Cloud-J_v8.0.0) 647c9f579 Merge PR #2426 (Store aerosol-specific optics files in new directory) and to the geoschem/Cloud-J repository: 291a161 Update Cloud-J standalone settings to use UV absorption by H2O by default 6a027b1 Merge pull request #24 from geoschem/feature/cloudj_v8 These PRs move aerosol optics configuration files to a separate folder (out of the CHEM_INPUTS/FAST-JX folder structure) and update Cloud-J to version 8.0.0. Signed-off-by: Bob Yantosca <[email protected]>
The GNU C and Fortran compilers allow you to build code sanitation functionality into an executable. This will add several library function calls to an executable that check for memory leaks and other such types of errors.
In
CMakeScripts/GC-ConfigureClassic.cmake, we have addedSANITIZEas a new cache variable. When configuring with-DSANITIZE=y, this will add-fsantize=address -fsantize=leak -fsanitize=undefinedto both the compiler and linker flags. This will compile the code sanitation functionality into the GEOS-Chem Classic executable.Using
-DSANITIZE=ywith Intel compilers will cause CMake to throw an error, as this functionality is not supported in Intel compilers (at least I think not).NOTE: The code sanitation causes GEOS-Chem Classic to run much more slowly. Therefore it is best to only use this new option on short debugging runs.
With
-DSANITIZE=y, you will get output similar to this to the stderr stream:you can then go to the line numbers that are cited and attempt to fix the memory leaks or other issues that are brought up.