BUG: QuickSurfaceMesh preflight crashes when the grid geometry has no Cell Attribute Matrix

Abierto Apto para principiantes
  • ¿Algo no está bien?
#1,843 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 3 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
76/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
cpp
Área
backend, testing

Línea de trabajo

Lee QuickSurfaceMeshFilter.cpp, especialmente QuickSurfaceMeshFilter::preflightImpl() cerca de la consulta del recuento de celdas, e inspecciona QuickSurfaceMeshFilterTest.cpp. Sustituye la fuente del recuento de celdas por el recuento de celdas de la geometría y añade la prueba proporcionada para Image Geometry, además de un caso para Rectilinear Grid. Ejecuta el filtro ctest indicado; el trabajo estará terminado cuando ambos casos completen el preflight correctamente sin una Cell Attribute Matrix.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

bug Filter

Problem

QuickSurfaceMesh crashes during preflight when the selected grid geometry has no Cell Attribute Matrix. Preflight reads the cell count from the geometry's Cell Attribute Matrix without checking that one exists, so it dereferences a null pointer and the process ends with a segmentation fault.

  • Expected: preflight returns a result. With Attempt to Fix Problem Voxels off, Feature Ids that live outside the geometry are allowed, so a consistent input (one Feature Id per cell) should preflight cleanly.
  • Actual: the application or test process crashes (SIGSEGV).
  • Commit: develop at 2eb6b878f6ff408d1454754d4c0e63edd676d7e0.

Steps to Reproduce

  1. Create an Image Geometry with dimensions 4 x 1 x 1 and no Cell Attribute Matrix.
  2. Create an int32 DataArray with 4 tuples and 1 component in a DataGroup outside the geometry, for example Elsewhere/Feature Ids.
  3. Preflight Quick Surface Mesh with:
    • Grid Geometry: the Image Geometry from step 1
    • Cell Feature Ids: the array from step 2
    • Attempt to Fix Problem Voxels: off
    • every other parameter at its default
  4. Preflight crashes.

Cause

QuickSurfaceMeshFilter::preflightImpl() takes the number of cells from the Cell Attribute Matrix:

https://github.com/BlueQuartzSoftware/simplnx/blob/2eb6b878f6ff408d1454754d4c0e63edd676d7e0/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/QuickSurfaceMeshFilter.cpp#L151-L153

  const auto& gridGeom = dataStructure.getDataRefAs<IGridGeometry>(pGridGeomDataPath);

  const usize elementTupleCount = gridGeom.getCellData()->getNumberOfTuples();

IGridGeometry::getCellData() returns nullptr when no Cell Attribute Matrix is assigned. Nothing before this line checks for that:

  • The Grid Geometry parameter only checks the geometry type (Image or Rectilinear Grid).
  • The Cell Feature Ids parameter only checks the data type and component shape. It does not require the array to be in the geometry.

The value is used only to check the tuple count of the Cell Attribute Arrays to Transfer (error -76531):

https://github.com/BlueQuartzSoftware/simplnx/blob/2eb6b878f6ff408d1454754d4c0e63edd676d7e0/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/QuickSurfaceMeshFilter.cpp#L174-L181

Verification Unit Test

Add to src/Plugins/SimplnxCore/test/QuickSurfaceMeshFilterTest.cpp, with #include "simplnx/DataStructure/DataGroup.hpp" next to the other DataStructure includes:

TEST_CASE("SimplnxCore::QuickSurfaceMeshFilter: Geometry without a Cell Attribute Matrix", "[SimplnxCore][QuickSurfaceMeshFilter]")
{
  UnitTest::LoadPlugins();

  // The Image Geometry has 4 cells but no Cell Attribute Matrix. Feature Ids lives in a separate DataGroup
  // and has one tuple per cell, so the geometry and the Feature Ids array are consistent.
  DataStructure dataStructure;
  auto* imageGeomPtr = ImageGeom::Create(dataStructure, "Image Geometry");
  REQUIRE(imageGeomPtr != nullptr);
  imageGeomPtr->setDimensions({4, 1, 1});
  imageGeomPtr->setOrigin({0, 0, 0});
  imageGeomPtr->setSpacing({1, 1, 1});
  REQUIRE(imageGeomPtr->getCellData() == nullptr);
  auto* elsewherePtr = DataGroup::Create(dataStructure, "Elsewhere");
  REQUIRE(elsewherePtr != nullptr);
  auto* featureIdsPtr = Int32Array::CreateWithStore<DataStore<int32>>(dataStructure, "Feature Ids", {4}, {1}, elsewherePtr->getId());
  REQUIRE(featureIdsPtr != nullptr);
  (*featureIdsPtr)[0] = 1;
  (*featureIdsPtr)[1] = 1;
  (*featureIdsPtr)[2] = 2;
  (*featureIdsPtr)[3] = 2;

  QuickSurfaceMeshFilter filter;
  Arguments args;
  args.insertOrAssign(QuickSurfaceMeshFilter::k_GridGeometryDataPath_Key, std::make_any<DataPath>(DataPath({"Image Geometry"})));
  args.insertOrAssign(QuickSurfaceMeshFilter::k_CellFeatureIdsArrayPath_Key, std::make_any<DataPath>(DataPath({"Elsewhere", "Feature Ids"})));
  args.insertOrAssign(QuickSurfaceMeshFilter::k_FixProblemVoxels_Key, std::make_any<bool>(false));

  // Preflight must return a result. It must not dereference the missing Cell Attribute Matrix.
  auto preflightResult = filter.preflight(dataStructure, args);
  SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions);
}

Run only this test:

ctest -R "QuickSurfaceMeshFilter: Geometry without a Cell Attribute Matrix" --output-on-failure

Output at 2eb6b87 (simplnx-Rel, macOS arm64):

QuickSurfaceMeshFilterTest.cpp:108: FAILED:
  {Unknown expression after the reported line}
due to a fatal error condition:
  SIGSEGV - Segmentation violation signal

test cases: 1 | 1 failed
assertions: 6 | 5 passed | 1 failed

	842 - SimplnxCore::QuickSurfaceMeshFilter: Geometry without a Cell Attribute Matrix (SEGFAULT)

Proposed Fix

In QuickSurfaceMeshFilter::preflightImpl(), take the cell count from the geometry instead of its Cell Attribute Matrix:

-  const usize elementTupleCount = gridGeom.getCellData()->getNumberOfTuples();
+  const usize elementTupleCount = gridGeom.getNumberOfCells();

getNumberOfCells() is the count the Cell Attribute Matrix must match, so the -76531 check on Cell Attribute Arrays to Transfer behaves the same for valid geometries. A geometry without a Cell Attribute Matrix then has no arrays to transfer, and preflight continues.

Add a Rectilinear Grid case to the test as well, because the filter accepts both geometry types.

Notes

  • Found while reviewing #1829. That PR requires Feature Ids to be in the geometry's Cell Attribute Matrix only when Attempt to Fix Problem Voxels is on, and it adds a check that Feature Ids has one tuple per cell (-76533). Neither check stops this crash when the option is off: the same line is still reached at the PR head. This was found by reading the code; the test was not run on the PR branch.
  • The QuickSurfaceMesh algorithm does not read the Cell Attribute Matrix during execute (checked by a search of Algorithms/QuickSurfaceMesh*.cpp). The only dereference found is this one in preflight.
Lenguaje dominante
C++
Estrellas
18
Forks
15
Merge medio
3 d 35 min
PR fusionados (30 d)
25

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de BlueQuartzSoftware/simplnx

Todos los issues de BlueQuartzSoftware/simplnx

Issues similares

Más issues de C++

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.