VTK/SoftwareQuality/Valgrind/Fall2011: Difference between revisions

From KitwarePublic
< VTK
Jump to navigationJump to search
Line 27: Line 27:


==Improve==
==Improve==
* [http://review.source.kitware.com/#change,3320 TestContextImage] Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
* Minor defects
* [http://review.source.kitware.com/#change,3318 MeanValueCoordinatesInterpolator and Polyhedron] vtkPolyhedron::IntersectWithLine did not initialize tMin. In both Polyhedron and MeanValueCoordinatesInterpolator, the operator++ was accessing memory out of range when the end of the container was reached.
** [http://review.source.kitware.com/#change,3320 TestContextImage] Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
* [http://review.source.kitware.com/#change,3322 InteractorEventRecorder] If vtkInteractorEventRecorder::Play() was called more than once, the vtksys_ios::istringstream was leaked.
** [http://review.source.kitware.com/#change,3323 TestGlobeSource] Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
* [http://review.source.kitware.com/#change,3323 TestGlobeSource] Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
** [http://review.source.kitware.com/#change,3324 TestKCoreLayout] Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
* [http://review.source.kitware.com/#change,3324 TestKCoreLayout] Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
** [http://review.source.kitware.com/#change,3326 TestImageDataLIC*] Tests were not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
* [http://review.source.kitware.com/#change,3326 TestImageDataLIC*] Tests were not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
** [http://review.source.kitware.com/#change,3372 TestQuadRotationalExtrusionMultiBlock] Test was not releasing the two char * returned by vtkTestUtilities::ExpandDataFileName().
* [http://review.source.kitware.com/#change,3328 OpenGLProjectedTetrahedraMapper] this->Internals was allocated in the constructor, but not freed in the destructor. valgrind detected leaks in two tests:ProjectedTetrahedraZoomIn and TestProjectedHexahedra.
** [http://review.source.kitware.com/#change,3319 PCAStatistics] In both vtkPCAStatistics and TestPCAStatistics, a delete was used rather than a delete[].
* [http://review.source.kitware.com/#change,3330 OpenGLExtensionManager] OpenGLExtensionManager was replacing this->ExtensionsString without freeing the current contents. Valgrind detected the leaks in two tests: TestMultiTexturing and TestMultiTexturingTransform.
** [http://review.source.kitware.com/#change,3322 InteractorEventRecorder] If vtkInteractorEventRecorder::Play() was called more than once, the vtksys_ios::istringstream was leaked.
* [http://review.source.kitware.com/#change,3331 Coordinate] When a viewport was not specified, vtkCoordinate::GetComputedDoubleDisplayValue() was not setting ComputedDoubleDisplayValue. This patch sets the value to VTK_LARGE_INTEGER. Valgrind reported these defects for: vtkCaptionRepresentationTest1 and vtkTextRepresentationTest1.
** [http://review.source.kitware.com/#change,3331 Coordinate] When a viewport was not specified, vtkCoordinate::GetComputedDoubleDisplayValue() was not setting ComputedDoubleDisplayValue. This patch sets the value to VTK_LARGE_INTEGER. Valgrind reported these defects for: vtkCaptionRepresentationTest1 and vtkTextRepresentationTest1.
* [http://review.source.kitware.com/#change,3319 PCAStatistics] In both vtkPCAStatistics and TestPCAStatistics, a delete was used rather than a delete[].
** [http://review.source.kitware.com/#change,3388 vtkLinearExtractor] Get methods should not be used in constructors. The Set method compares the new value with the old value. Since the old value is not initialized it generates a valgrind defect.
* [http://review.source.kitware.com/#change,3345 vtkControlPointsItem] At least one compiler warned that a subscript was out of range. In vtkControlPointsItem::GetCenterOfMass, point[4] was being accessed. Looks like a cut and paste error.
** [http://review.source.kitware.com/#change,3396 TestHyperOctreeContourFilter] The 1D test needed mapper1d->SetScalarModeToUseCellData();
* [http://review.source.kitware.com/#change,3372 TestQuadRotationalExtrusionMultiBlock] Test was not releasing the two char * returned by vtkTestUtilities::ExpandDataFileName().
* Major defects
* [http://review.source.kitware.com/#change,3388 vtkLinearExtractor] et methods should not be used in constructors. The Set method compares the new value with the old value. Since the old value is not initialized it generates a valgrind defect.
** [http://review.source.kitware.com/#change,3318 MeanValueCoordinatesInterpolator and Polyhedron] vtkPolyhedron::IntersectWithLine did not initialize tMin. In both Polyhedron and MeanValueCoordinatesInterpolator, the operator++ was accessing memory out of range when the end of the container was reached.
* [http://review.source.kitware.com/#change,3387 ReebGraphSurfaceSkeletonFilter ReebGraphVolumeSkeletonFilter] Two logic errors caused valgrind to report "Conditional jump or move depends on uninitialised value(s)" for TestReebGraph. In vtkReebGraphSurfaceSkeletonFilter::RequestData() also vtkReebGraphVolumeSkeletonFilter::RequestData() 1) Storing the wrong point id in meshToSubMeshmap. This resulted in bad vertexIds in the subMesh. 2) The logic for the contourFilter setup was incorrect. contourFilter->SetNumberOfContours(1); but contourFilter->SetValue(i, ...)
** [http://review.source.kitware.com/#change,3328 OpenGLProjectedTetrahedraMapper] this->Internals was allocated in the constructor, but not freed in the destructor. valgrind detected leaks in two tests:ProjectedTetrahedraZoomIn and TestProjectedHexahedra.
* [http://review.source.kitware.com/#change,3396 TestHyperOctreeContourFilter] The 1D test needed mapper1d->SetScalarModeToUseCellData();
** [http://review.source.kitware.com/#change,3330 OpenGLExtensionManager] OpenGLExtensionManager was replacing this->ExtensionsString without freeing the current contents. Valgrind detected the leaks in two tests: TestMultiTexturing and TestMultiTexturingTransform.
* [http://review.source.kitware.com/#change,3403 vtkUnstructuredGridVolumeZSweepMapper] This valgrind defect has been present for years. There are instances where vtkDoubleScreenEdge::Case is not defined before RasterizeTriangle. The logic in vtkSimpleScreenEdge::Init is complex. Perhaps in the future, a more more extensive examination will uncover the real problem. For now, the patch fixes the valgrind defect and still passes the regression test.
** [http://review.source.kitware.com/#change,3345 vtkControlPointsItem] At least one compiler warned that a subscript was out of range. In vtkControlPointsItem::GetCenterOfMass, point[4] was being accessed. Looks like a cut and paste error.
** [http://review.source.kitware.com/#change,3387 ReebGraphSurfaceSkeletonFilter ReebGraphVolumeSkeletonFilter] Two logic errors caused valgrind to report "Conditional jump or move depends on uninitialised value(s)" for TestReebGraph. In vtkReebGraphSurfaceSkeletonFilter::RequestData() also vtkReebGraphVolumeSkeletonFilter::RequestData() 1) Storing the wrong point id in meshToSubMeshmap. This resulted in bad vertexIds in the subMesh. 2) The logic for the contourFilter setup was incorrect. contourFilter->SetNumberOfContours(1); but contourFilter->SetValue(i, ...)
** [http://review.source.kitware.com/#change,3403 vtkUnstructuredGridVolumeZSweepMapper] This valgrind defect has been present for years. There are instances where vtkDoubleScreenEdge::Case is not defined before RasterizeTriangle. The logic in vtkSimpleScreenEdge::Init is complex. Perhaps in the future, a more more extensive examination will uncover the real problem. For now, the patch fixes the valgrind defect and still passes the regression test.


==Control==
==Control==

Revision as of 17:36, 30 November 2011

Testing is critical to high quality software. There are a number of aspects to the VTK's software quality. One important aspect is valgrind. Valgrind detects a number of dynamic memory defects.

This experiment seeks to reduce VTK's valgrind defects to 0 and keep them at 0.

This experiment uses the DMAIC methodology of the Six Sigma management process to "Define", "Measure", "Analyze", "Improve" and "Control" valgrind defects in VTK.

The basic methodology (from Wikipedia) consists of the following five steps:

  • Define process goals that are consistent with customer demands and ITKv4's strategy.
  • Measure key aspects of the current process and collect relevant data.
  • Analyze the data to verify cause-and-effect relationships. Determine what the relationships are, and attempt to ensure that all factors have been considered.
  • Improve or optimize the process.
  • Control to ensure that any deviations from target are corrected before they result in defects. Set up pilot runs to establish software quality, move on to production, set up control mechanisms and continuously monitor the process.

Define

Keep the number of VTK valgrind defects to 0.

Measure

As of November 16, 2011, there were 175 valgrind defects.

  • 22 tests had defects
    • 12 tests had memory leaks
    • 6 tests had uninitialized memory conditionals
    • 4 tests had uninitialized memory reads
    • 2 test had potential memory leaks
    • 1 test has mismatched memory/deallocate

Analyze

Improve

  • Minor defects
    • TestContextImage Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
    • TestGlobeSource Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
    • TestKCoreLayout Test was not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
    • TestImageDataLIC* Tests were not releasing the char * returned by vtkTestUtilities::ExpandDataFileName().
    • TestQuadRotationalExtrusionMultiBlock Test was not releasing the two char * returned by vtkTestUtilities::ExpandDataFileName().
    • PCAStatistics In both vtkPCAStatistics and TestPCAStatistics, a delete was used rather than a delete[].
    • InteractorEventRecorder If vtkInteractorEventRecorder::Play() was called more than once, the vtksys_ios::istringstream was leaked.
    • Coordinate When a viewport was not specified, vtkCoordinate::GetComputedDoubleDisplayValue() was not setting ComputedDoubleDisplayValue. This patch sets the value to VTK_LARGE_INTEGER. Valgrind reported these defects for: vtkCaptionRepresentationTest1 and vtkTextRepresentationTest1.
    • vtkLinearExtractor Get methods should not be used in constructors. The Set method compares the new value with the old value. Since the old value is not initialized it generates a valgrind defect.
    • TestHyperOctreeContourFilter The 1D test needed mapper1d->SetScalarModeToUseCellData();
  • Major defects
    • MeanValueCoordinatesInterpolator and Polyhedron vtkPolyhedron::IntersectWithLine did not initialize tMin. In both Polyhedron and MeanValueCoordinatesInterpolator, the operator++ was accessing memory out of range when the end of the container was reached.
    • OpenGLProjectedTetrahedraMapper this->Internals was allocated in the constructor, but not freed in the destructor. valgrind detected leaks in two tests:ProjectedTetrahedraZoomIn and TestProjectedHexahedra.
    • OpenGLExtensionManager OpenGLExtensionManager was replacing this->ExtensionsString without freeing the current contents. Valgrind detected the leaks in two tests: TestMultiTexturing and TestMultiTexturingTransform.
    • vtkControlPointsItem At least one compiler warned that a subscript was out of range. In vtkControlPointsItem::GetCenterOfMass, point[4] was being accessed. Looks like a cut and paste error.
    • ReebGraphSurfaceSkeletonFilter ReebGraphVolumeSkeletonFilter Two logic errors caused valgrind to report "Conditional jump or move depends on uninitialised value(s)" for TestReebGraph. In vtkReebGraphSurfaceSkeletonFilter::RequestData() also vtkReebGraphVolumeSkeletonFilter::RequestData() 1) Storing the wrong point id in meshToSubMeshmap. This resulted in bad vertexIds in the subMesh. 2) The logic for the contourFilter setup was incorrect. contourFilter->SetNumberOfContours(1); but contourFilter->SetValue(i, ...)
    • vtkUnstructuredGridVolumeZSweepMapper This valgrind defect has been present for years. There are instances where vtkDoubleScreenEdge::Case is not defined before RasterizeTriangle. The logic in vtkSimpleScreenEdge::Init is complex. Perhaps in the future, a more more extensive examination will uncover the real problem. For now, the patch fixes the valgrind defect and still passes the regression test.

Control