BUG: Compute Triangle Face Ids accepts Face Labels with the wrong tuple count

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/SharedFeatureFaceFilter.cppを読み、preflight検証の流れを追って、その動作をExtractTripleLinesFilter.cppまたはWriteStlFileFilter.cppと比較してください。issueには再現方法と提案するタプル数チェックが記載されています。src/Plugins/SimplnxCore/test/SharedFeatureFaceTest.cppに回帰テストケースを追加または適応し、ctest -R "SharedFeatureFaceFilter: Face Labels tuple count" --output-on-failureを実行してください。タプル数が一致する場合はpreflightを通過し、一致しない場合はエラーで失敗すれば完了です。

Problem

Compute Triangle Face Ids does not check that the Face Labels array has one tuple per triangle of the selected Triangle Geometry. Preflight accepts a Face Labels array with fewer tuples than the geometry has faces. Execute then loops over every face of the geometry, so it reads past the end of the Face Labels array and writes past the end of the created Feature Face Ids array, which the filter sizes from the Face Labels array.

• Expected: preflight fails with an error that names both counts, as Extract Triple Lines and Write STL Files from Triangle Geometry do for the same kind of mismatch.
• Actual: preflight passes. Execute reads and writes out of bounds (undefined behavior: wrong output, memory corruption, or a crash).

Occurs at commit 419f9db357d0c5e9adbdb0ab4511d36a433aecfb (develop).

Steps to Reproduce

The verification test below builds this input in memory:

1. Create a Triangle Geometry with 3 faces and a face Attribute Matrix that holds a valid int32 Face Labels array with 3 tuples and 2 components.
2. Create a top-level Data Group OtherGroup that holds an int32 array FaceLabels with 2 tuples and 2 components.
3. Run Compute Triangle Face Ids with Triangle Geometry = the geometry from step 1 and Face Labels = OtherGroup/FaceLabels.
4. Preflight passes. (Execute would read Face Labels tuple 2 and write Feature Face Ids tuple 2, which do not exist.)

The same input in the GUI (not run; the unit test is the verified reproduction): Read STL File with Generate Triangle Face Labels on, using any STL file with 3 or more triangles; Create Data Group for OtherGroup; Create Data Array with Output Numeric Type = int32, Total Number of Components = 2, Set Tuple Dimensions [not required if creating inside an Attribute Matrix] on and Data Array Dimensions (Slowest to Fastest Dimensions) = 2, Created Array = OtherGroup/FaceLabels; then Compute Triangle Face Ids with that array as Face Labels.

Cause

The Face Labels parameter restricts only the type and the component shape:

https://github.com/BlueQuartzSoftware/simplnx/blob/419f9db357d0c5e9adbdb0ab4511d36a433aecfb/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/SharedFeatureFaceFilter.cpp#L58-L59

Preflight checks only that the array exists, and then creates the Feature Face Ids array with the tuple shape of the Face Labels array. It never compares that tuple count with the number of faces:

https://github.com/BlueQuartzSoftware/simplnx/blob/419f9db357d0c5e9adbdb0ab4511d36a433aecfb/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/SharedFeatureFaceFilter.cpp#L104-L116

    const auto* faceLabelDataArray = dataStructure.getDataAs<Int32Array>(pFaceLabelsArrayPathValue);
    if(nullptr == faceLabelDataArray)
    {
      return IFilter::MakePreflightErrorResult(-12901, ...);
    }
    ...
    auto createArrayAction = std::make_unique<CreateArrayAction>(nx::core::DataType::int32, faceLabelDataArray->getTupleShape(), std::vector<usize>{1}, createdArrayPath);

The algorithm takes the loop count from the geometry, not from the Face Labels array:

https://github.com/BlueQuartzSoftware/simplnx/blob/419f9db357d0c5e9adbdb0ab4511d36a433aecfb/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/SharedFeatureFace.cpp#L141-L161

  usize totalPoints = triangleGeom.getNumberOfFaces();
  ...
  for(usize t = 0; t < totalPoints; ++t)
  {
    ...
    int32 fl0 = surfaceMeshFaceLabels[t * 2];
    int32 fl1 = surfaceMeshFaceLabels[t * 2 + 1];

and writes surfaceMeshFeatureFaceIds[t] for each face (L177, L184). When the Face Labels array has fewer tuples than the geometry has faces, both accesses go past the end of their arrays. The in-memory DataStore does not check the index, so these are unchecked heap reads and writes. The out-of-bounds access is inferred from the code; the test stops at preflight and does not execute it.

Sibling filters reject this mismatch in preflight, for example Extract Triple Lines:

https://github.com/BlueQuartzSoftware/simplnx/blob/419f9db357d0c5e9adbdb0ab4511d36a433aecfb/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/ExtractTripleLinesFilter.cpp#L144-L151

and Write STL Files from Triangle Geometry:

https://github.com/BlueQuartzSoftware/simplnx/blob/419f9db357d0c5e9adbdb0ab4511d36a433aecfb/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/WriteStlFileFilter.cpp#L159-L163

Verification Unit Test

Add to src/Plugins/SimplnxCore/test/SharedFeatureFaceTest.cpp (after the existing test case; it uses the constants in the file's SharedFeatureFaceFilterTest namespace):

TEST_CASE("SimplnxCore::SharedFeatureFaceFilter: Face Labels tuple count must match the triangle count", "[SimplnxCore][SharedFeatureFaceFilter]")
{
  UnitTest::LoadPlugins();

  // A valid Triangle Geometry with 5 vertices and 3 faces, and a matching 3-tuple Face Labels array
  DataStructure dataStructure;
  auto* triangleGeomPtr = TriangleGeom::Create(dataStructure, k_TriangleGeometryName);
  REQUIRE(triangleGeomPtr != nullptr);

  const std::vector<float32> vertices = {0.0F, 0.0F, 0.0F, 1.0F, 0.0F, 0.0F, 0.0F, 1.0F, 0.0F, 1.0F, 1.0F, 0.0F, 2.0F, 0.0F, 0.0F};
  const std::vector<IGeometry::MeshIndexType> faces = {0, 1, 2, 1, 3, 2, 1, 4, 3};
  const usize numVertices = vertices.size() / 3;
  const usize numFaces = faces.size() / 3;

  auto* vertexAttrMatPtr = AttributeMatrix::Create(dataStructure, INodeGeometry0D::k_VertexAttributeMatrixName, {numVertices}, triangleGeomPtr->getId());
  REQUIRE(vertexAttrMatPtr != nullptr);
  triangleGeomPtr->setVertexAttributeMatrix(*vertexAttrMatPtr);
  auto* faceAttrMatPtr = AttributeMatrix::Create(dataStructure, k_FaceAttributeMatrixName, {numFaces}, triangleGeomPtr->getId());
  REQUIRE(faceAttrMatPtr != nullptr);
  triangleGeomPtr->setFaceAttributeMatrix(*faceAttrMatPtr);

  auto* vertexListPtr = IGeometry::SharedVertexList::Create(dataStructure, "SharedVertexList", std::make_unique<DataStore<float32>>(std::vector<usize>{numVertices}, std::vector<usize>{3}, 0.0F),
                                                            triangleGeomPtr->getId());
  REQUIRE(vertexListPtr != nullptr);
  auto* faceListPtr = IGeometry::SharedFaceList::Create(dataStructure, "SharedTriList", std::make_unique<DataStore<IGeometry::MeshIndexType>>(std::vector<usize>{numFaces}, std::vector<usize>{3}, 0),
                                                        triangleGeomPtr->getId());
  REQUIRE(faceListPtr != nullptr);
  auto& vertexListRef = vertexListPtr->getDataStoreRef();
  for(usize index = 0; index < vertices.size(); index++)
  {
    vertexListRef[index] = vertices[index];
  }
  auto& faceListRef = faceListPtr->getDataStoreRef();
  for(usize index = 0; index < faces.size(); index++)
  {
    faceListRef[index] = faces[index];
  }
  triangleGeomPtr->setVertices(*vertexListPtr);
  triangleGeomPtr->setFaceList(*faceListPtr);

  auto* faceLabelsPtr = Int32Array::Create(dataStructure, k_FaceLabelsName, std::make_unique<DataStore<int32>>(std::vector<usize>{numFaces}, std::vector<usize>{2}, 0), faceAttrMatPtr->getId());
  REQUIRE(faceLabelsPtr != nullptr);
  const std::vector<int32> faceLabels = {1, 2, 1, 2, 2, 3};
  auto& faceLabelsRef = faceLabelsPtr->getDataStoreRef();
  for(usize index = 0; index < faceLabels.size(); index++)
  {
    faceLabelsRef[index] = faceLabels[index];
  }

  // A valid int32 array with 2 components but only 2 tuples, in a group outside the geometry
  const DataPath otherGroupPath({"OtherGroup"});
  const DataPath shortFaceLabelsPath = otherGroupPath.createChildPath(k_FaceLabelsName);
  auto* otherGroupPtr = DataGroup::Create(dataStructure, otherGroupPath.getTargetName());
  REQUIRE(otherGroupPtr != nullptr);
  auto* shortFaceLabelsPtr =
      Int32Array::Create(dataStructure, k_FaceLabelsName, std::make_unique<DataStore<int32>>(std::vector<usize>{numFaces - 1}, std::vector<usize>{2}, 1), otherGroupPtr->getId());
  REQUIRE(shortFaceLabelsPtr != nullptr);

  SharedFeatureFaceFilter filter;
  Arguments args;
  args.insertOrAssign(SharedFeatureFaceFilter::k_RandomizeFeatures_Key, std::make_any<bool>(false));
  args.insertOrAssign(SharedFeatureFaceFilter::k_TriGeometryDataPath_Key, std::make_any<GeometrySelectionParameter::ValueType>(k_GeometryPath));
  args.insertOrAssign(SharedFeatureFaceFilter::k_FeatureFaceIdsArrayName_Key, std::make_any<DataObjectNameParameter::ValueType>(k_FeatureFaceIdsArrayName));
  args.insertOrAssign(SharedFeatureFaceFilter::k_GrainBoundaryAttributeMatrixName_Key, std::make_any<DataObjectNameParameter::ValueType>(k_GrainBoundaryAttributeMatrixName));
  args.insertOrAssign(SharedFeatureFaceFilter::k_FeatureNumTrianglesArrayName_Key, std::make_any<DataObjectNameParameter::ValueType>(k_FeatureNumTrianglesArrayName));
  args.insertOrAssign(SharedFeatureFaceFilter::k_FeatureFaceLabelsArrayName_Key, std::make_any<DataObjectNameParameter::ValueType>(k_FeatureFaceLabelsArrayName));

  // Control: the matching Face Labels array passes preflight
  args.insertOrAssign(SharedFeatureFaceFilter::k_FaceLabelsArrayPath_Key, std::make_any<DataPath>(k_FaceLabelsPath));
  auto validPreflightResult = filter.preflight(dataStructure, args);
  SIMPLNX_RESULT_REQUIRE_VALID(validPreflightResult.outputActions);

  // 2 Face Labels tuples for 3 triangles must be rejected before execute reads past the array
  args.insertOrAssign(SharedFeatureFaceFilter::k_FaceLabelsArrayPath_Key, std::make_any<DataPath>(shortFaceLabelsPath));
  auto preflightResult = filter.preflight(dataStructure, args);
  SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions);
}

Run only this test from the build directory:

ctest -R "SharedFeatureFaceFilter: Face Labels tuple count" --output-on-failure

Failure at commit 419f9db (the control preflight passes, the mismatched preflight is accepted):

src/Plugins/SimplnxCore/test/SharedFeatureFaceTest.cpp:182: FAILED:
  REQUIRE( simplnxCheckedResult.invalid() )
with expansion:
  false

test cases:  1 |  0 passed | 1 failed
assertions: 11 | 10 passed | 1 failed

Proposed Fix

In SharedFeatureFaceFilter::preflightImpl() (src/Plugins/SimplnxCore/src/SimplnxCore/Filters/SharedFeatureFaceFilter.cpp), get the Triangle Geometry and return an error when the Face Labels tuple count is not the face count, before the output actions are created:

+  const auto* triangleGeom = dataStructure.getDataAs<TriangleGeom>(triangleGeometryPath);
+  if(nullptr == triangleGeom)
+  {
+    return IFilter::MakePreflightErrorResult(-12902, fmt::format("Triangle Geometry does not exist at path '{}'", triangleGeometryPath.toString()));
+  }
+
   // Create the Face Data/FeatureIds Array
   {
     const auto* faceLabelDataArray = dataStructure.getDataAs<Int32Array>(pFaceLabelsArrayPathValue);
     if(nullptr == faceLabelDataArray)
     {
       return IFilter::MakePreflightErrorResult(-12901, ...);
     }
+    if(faceLabelDataArray->getNumberOfTuples() != triangleGeom->getNumberOfFaces())
+    {
+      return IFilter::MakePreflightErrorResult(-12903, fmt::format("Face Labels array '{}' has {} tuples but Triangle Geometry '{}' has {} faces. They must match.", pFaceLabelsArrayPathValue.toString(),
+                                                                   faceLabelDataArray->getNumberOfTuples(), triangleGeometryPath.toString(), triangleGeom->getNumberOfFaces()));
+    }

The filter needs #include "simplnx/DataStructure/Geometry/TriangleGeom.hpp". The error codes are suggestions; -12901 is the only code the filter uses now. After the fix, the test can also check the new error code. The algorithm can then rely on the two counts being equal.

Notes

• Found while reviewing #1858 (the fix for #1772).
• Closest existing issue checked: #1833.
• The verification test was built and run on macOS (Apple clang, Release) at commit 419f9db.