Commits

Peter Brune committed f490541

DM: Fix for recursive DMDestroy() causing valgrind issues

runex26_2 in src/ts/examples/tutorials caused the following valgrind output:

> ==21447== Invalid read of size 8
> ==21447== at 0x599093E: DMDestroy (dm.c:401)
> ==21447== by 0x4F3F83F: PetscObjectDereference (inherit.c:612)
> ==21447== by 0x4F30C6A: PetscObjectListDestroy (olist.c:160)
> ==21447== by 0x4F3AA1F: PetscHeaderDestroy_Private (inherit.c:117)
> ==21447== by 0x5184D16: VecDestroy (vector.c:499)
> ==21447== by 0x5990EA5: DMDestroy (dm.c:425)
> ==21447== by 0x5B2833D: PCDestroy (precon.c:122)
> ==21447== by 0x5C0D6EF: KSPDestroy (itfunc.c:772)
> ==21447== by 0x5C93387: SNESDestroy (snes.c:2790)
> ==21447== by 0x5D35130: SNESReset_NASM (nasm.c:46)
> ==21447== by 0x5C92705: SNESReset (snes.c:2732)
> ==21447== by 0x5C92689: SNESReset (snes.c:2728)
> ==21447== by 0x5DA00E8: TSReset (ts.c:1872)
> ==21447== by 0x5DA09EB: TSDestroy (ts.c:1912)
> ==21447== by 0x402671: main (ex26.c:172)
> ==21447== Address 0x9b39778 is 24 bytes inside a block of size 32 free'd
> ==21447== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==21447== by 0x4F85AEB: PetscFreeAlign (mal.c:70)
> ==21447== by 0x5990F29: DMDestroy (dm.c:426)
> ==21447== by 0x5B2833D: PCDestroy (precon.c:122)
> ==21447== by 0x5C0D6EF: KSPDestroy (itfunc.c:772)
> ==21447== by 0x5C93387: SNESDestroy (snes.c:2790)
> ==21447== by 0x5D35130: SNESReset_NASM (nasm.c:46)
> ==21447== by 0x5C92705: SNESReset (snes.c:2732)
> ==21447== by 0x5C92689: SNESReset (snes.c:2728)
> ==21447== by 0x5DA00E8: TSReset (ts.c:1872)
> ==21447== by 0x5DA09EB: TSDestroy (ts.c:1912)
> ==21447== by 0x402671: main (ex26.c:172)

This was caused by recursive calls to DMDestroy(). When freeing the named local vector list, the linked list is freed
before being set to NULL. The recursive call of DMDestroy() by VecDestroy() then tries to access the value in the
structure. The fix to this is to zero out the DM member before the recursive call instead of after it.

Reported-by: Barry Smith <bsmith@mcs.anl.gov>

Comments (0)

Files changed (1)

src/dm/interface/dm.c

     if ((*dm)->localout[i]) SETERRQ(PETSC_COMM_SELF,PETSC_ERR_ARG_WRONGSTATE,"Destroying a DM that has a local vector obtained with DMGetLocalVector()");
     ierr = VecDestroy(&(*dm)->localin[i]);CHKERRQ(ierr);
   }
-  for (nlink=(*dm)->namedglobal; nlink; nlink=nnext) { /* Destroy the named vectors */
+  nnext=(*dm)->namedglobal;
+  (*dm)->namedglobal = NULL;
+  for (nlink=nnext; nlink; nlink=nnext) { /* Destroy the named vectors */
     nnext = nlink->next;
     if (nlink->status != DMVEC_STATUS_IN) SETERRQ1(((PetscObject)*dm)->comm,PETSC_ERR_ARG_WRONGSTATE,"DM still has Vec named '%s' checked out",nlink->name);
     ierr = PetscFree(nlink->name);CHKERRQ(ierr);
     ierr = VecDestroy(&nlink->X);CHKERRQ(ierr);
     ierr = PetscFree(nlink);CHKERRQ(ierr);
   }
-  (*dm)->namedglobal = NULL;
-
-  for (nlink=(*dm)->namedlocal; nlink; nlink=nnext) { /* Destroy the named local vectors */
+  nnext=(*dm)->namedlocal;
+  (*dm)->namedlocal = NULL;
+  for (nlink=nnext; nlink; nlink=nnext) { /* Destroy the named local vectors */
     nnext = nlink->next;
     if (nlink->status != DMVEC_STATUS_IN) SETERRQ1(((PetscObject)*dm)->comm,PETSC_ERR_ARG_WRONGSTATE,"DM still has Vec named '%s' checked out",nlink->name);
     ierr = PetscFree(nlink->name);CHKERRQ(ierr);
     ierr = VecDestroy(&nlink->X);CHKERRQ(ierr);
     ierr = PetscFree(nlink);CHKERRQ(ierr);
   }
-  (*dm)->namedlocal = NULL;
 
   /* Destroy the list of hooks */
   {