#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ---------------------+------------------------------------------------------ Reporter: sbrandt | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: development version Keywords: | ---------------------+------------------------------------------------------ I originally found the problem with clang, but have verified it with valgrind. A parameter is being freed by CCTK_SetString() in Misc.c, but is reused by CarpetIOHDF5. To reproduce, I build with clang, after commenting out the papi and hwloc thorns. Commenting out the call to free in CCTK_SetString() reveals alignment errors in LoopControl. I'm attaching the cfg file I used to build with clang.
#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ----------------------+----------------------------------------------------- Reporter: sbrandt | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: ----------------------+-----------------------------------------------------
Comment (by eschnett):
Where is this CCTK_SetString called?
#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ----------------------+----------------------------------------------------- Reporter: sbrandt | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: ----------------------+-----------------------------------------------------
Comment (by sbrandt):
My valgrind output is below. The line numbers may not be as helpful as the function names. Although I logged it as a CarpetIOHDF5 problem, it's possible that this is a Cactus problem.
{{{ ==16347== Invalid read of size 1 ==16347== at 0xCE1424: Util_StrCmpi (String.c:439) ==16347== by 0xCDC8A0: CCTK_Equals (Misc.c:128) ==16347== by 0x2EA5506: IOUtil_SetAllParameters (CheckpointRecovery.c:979) ==16347== by 0x246AFC6: CarpetIOHDF5::OpenFileSet(_cGH const*, std::string, char const*, int) (Input.cc:1025) ==16347== by 0x246BDF4: CarpetIOHDF5::Recover(_cGH*, char const*, int) (Input.cc:375) ==16347== by 0x2EA4F1E: IOUtil_RecoverParameters (CheckpointRecovery.c:751) ==16347== by 0x24619EF: CarpetIOHDF5_RecoverParameters (Input.cc:106) ==16347== by 0xF1474C: CCTKi_BindingsParameterRecovery_CarpetIOHDF5 (ScheduleCarpetIOHDF5.c:304) ==16347== by 0xEEEE14: CCTKi_BindingsParameterRecoveryInitialise (BindingsParameterRecovery.c:363) ==16347== by 0xCCB052: CCTKi_RecoverParameters.isra.0 (InitialiseCactus.c:219) ==16347== by 0xCCB1E9: CCTKi_InitialiseCactus (InitialiseCactus.c:104) ==16347== by 0xCA32F5: main (flesh.cc:64) ==16347== Address 0x82d09d0 is 0 bytes inside a block of size 9 free'd ==16347== at 0x59F24C4: free (in /usr/lib64/valgrind /vgpreload_memcheck-amd64-linux.so) ==16347== by 0xCDCD50: CCTK_SetString (Misc.c:1237) ==16347== by 0xCCC203: ParameterSetKeyword.isra.2 (Parameters.c:2072) ==16347== by 0xCCCF50: ParameterSetSimple (Parameters.c:2015) ==16347== by 0xCCE29C: ParameterSet (Parameters.c:1828) ==16347== by 0xCCE492: CCTK_ParameterSet (Parameters.c:660) ==16347== by 0x2EA5592: IOUtil_SetAllParameters (CheckpointRecovery.c:965) ==16347== by 0x246AFC6: CarpetIOHDF5::OpenFileSet(_cGH const*, std::string, char const*, int) (Input.cc:1025) ==16347== by 0x246BDF4: CarpetIOHDF5::Recover(_cGH*, char const*, int) (Input.cc:375) ==16347== by 0x2EA4F1E: IOUtil_RecoverParameters (CheckpointRecovery.c:751) ==16347== by 0x24619EF: CarpetIOHDF5_RecoverParameters (Input.cc:106) ==16347== by 0xF1474C: CCTKi_BindingsParameterRecovery_CarpetIOHDF5 (ScheduleCarpetIOHDF5.c:304)
}}}
#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ----------------------+----------------------------------------------------- Reporter: sbrandt | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: ----------------------+-----------------------------------------------------
Comment (by knarf):
It looks like IOUtil uses its own 'verbose' parameter (a keyword, and by that a string) during recovery of the parameters. String parameter pointers will/might get reset in that process, freeing their old pointers. The pointer in the GH is updated, but the copy made by CCTK_PARAMETERS inside the recovery routine itself is not. One of two solutions come to mind: either we make a copy of the whole string in this routine (and free it later), or we don't access parameters here directly, but always go through the GH.
#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ----------------------+----------------------------------------------------- Reporter: sbrandt | Owner: eschnett Type: defect | Status: review Priority: major | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: ----------------------+----------------------------------------------------- Changes (by knarf):
* status: new => review
Comment:
https://bitbucket.org/cactuscode/cactusbase/pull-request/1/ioutil-dont- use-own-parameter-after-it-has/diff
#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ----------------------+----------------------------------------------------- Reporter: sbrandt | Owner: eschnett Type: defect | Status: review Priority: major | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: ----------------------+-----------------------------------------------------
Comment (by knarf):
Replying to [comment:4 knarf]:
https://bitbucket.org/cactuscode/cactusbase/pull-request/1/ioutil-dont-
use-own-parameter-after-it-has/diff
I couldn't reproduce your valgrind output, and dind't try clang - but the pull request might fix it.
#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ----------------------+----------------------------------------------------- Reporter: sbrandt | Owner: eschnett Type: defect | Status: reviewed_ok Priority: major | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: ----------------------+----------------------------------------------------- Changes (by sbrandt):
* status: review => reviewed_ok
Comment:
It works for me using clang, and the code looks reasonable.
#1685: Access of freed memory in CarpetIOHDF5 during 1-Proc test of recoverML- EE.par ----------------------+----------------------------------------------------- Reporter: sbrandt | Owner: eschnett Type: defect | Status: closed Priority: major | Milestone: Component: Carpet | Version: development version Resolution: fixed | Keywords: ----------------------+----------------------------------------------------- Changes (by knarf):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
Merged in https://bitbucket.org/cactuscode/cactusbase/commits/fc20d98cf846e2eab152e7c8...
trac@lists.einsteintoolkit.org