Skip to content

Bug: _vert_id tag type mismatch in deriveMdlFromManifold/derive2DMdlFromManifold (mds/apfMDS.cc) #537

Description

@cekees

Both deriveMdlFromManifold (3D) and derive2DMdlFromManifold (2D) create the internal _vert_id mesh tag as a long tag and populate it with global vertex IDs:

apf::MeshTag* vIDTag = mesh->createLongTag("_vert_id", 1);
mesh->setLongTag(vit->second, vIDTag, &(vit->first));

Each of these functions then calls a private helper — getFaceIdInRegion (from deriveMdlFromManifold) or getEdgeIdInFace (from derive2DMdlFromManifold) — to identify which face/edge of a boundary element is being classified. Both helpers read the same _vert_id tag back, but declare the destination as int and call getIntTag:

int vID;
mesh->getIntTag(verts[0], vIDTag, &vID);

On any platform where long and int have different widths (i.e. every common 64-bit LP64 target), getIntTag reads a mismatched-width value out of a tag that was stored as long, so vID comes back as garbage. The vertex-ID comparisons that follow (if (vID != bface_data[2] && ...)) then never match for any vertex, and both helpers fall through to their explicit last-resort return value — which the source itself labels as a bug:

return 12; // Should give segmentation fault

That sentinel (12 for the face case, out-of-range for the up-to-4-entry apf::Downward array it indexes into afterward) produces a garbage MeshEntity*, which crashes on the subsequent setIntTag() call. This reproduces reliably and identically on both the 2.2.9 and 4.1.0 tags — the two getIntTag/createLongTag sites have been present unchanged across that entire range.

Fix: read the tag with the same type it was written with (getLongTag instead of getIntTag, long locals instead of int).

--- a/mds/apfMDS.cc
+++ b/mds/apfMDS.cc
@@ -56,20 +56,20 @@ static int getFaceIdInRegion(apf::Mesh* mesh, apf::MeshEntity* region,
 {
   apf::Downward verts;
   apf::MeshTag* vIDTag = mesh->findTag("_vert_id");
-  int vID;
+  long vID;
   mesh->getDownward(region, 0, verts);
   // Go through all vertices. What vertex is not on the face can be used to determine the face id.
   // TODO: Good way to assert that the rest of the 3 actually exist?
-  mesh->getIntTag(verts[0], vIDTag, &vID);
+  mesh->getLongTag(verts[0], vIDTag, &vID);
   if (vID != bface_data[2] && vID != bface_data[3] && vID != bface_data[4])
     return 2;
-  mesh->getIntTag(verts[1], vIDTag, &vID);
+  mesh->getLongTag(verts[1], vIDTag, &vID);
   if (vID != bface_data[2] && vID != bface_data[3] && vID != bface_data[4])
     return 3;
-  mesh->getIntTag(verts[2], vIDTag, &vID);
+  mesh->getLongTag(verts[2], vIDTag, &vID);
   if (vID != bface_data[2] && vID != bface_data[3] && vID != bface_data[4])
     return 1;
-  mesh->getIntTag(verts[3], vIDTag, &vID);
+  mesh->getLongTag(verts[3], vIDTag, &vID);
   if (vID != bface_data[2] && vID != bface_data[3] && vID != bface_data[4])
     return 0;
   return 12; // Should give segmentation fault
@@ -80,12 +80,13 @@ static int getEdgeIdInFace(apf::Mesh* mesh, apf::MeshEntity* face,
 {
   apf::Downward verts, edges;
   apf::MeshTag* vIDTag = mesh->findTag("_vert_id");
-  int vID[2], eID;
+  long vID[2];
+  int eID;
   mesh->getDownward(face, 1, edges);
   for (eID = 0; eID < 3; ++eID) {
     mesh->getDownward(edges[eID], 0, verts);
-    mesh->getIntTag(verts[0], vIDTag, &vID[0]);
-    mesh->getIntTag(verts[1], vIDTag, &vID[1]);
+    mesh->getLongTag(verts[0], vIDTag, &vID[0]);
+    mesh->getLongTag(verts[1], vIDTag, &vID[1]);
     if((vID[0] == bedge_data[2] && vID[1] == bedge_data[3]) ||
        (vID[0] == bedge_data[3] && vID[1] == bedge_data[2])) {
       return eID;

Confirmed present identically on tags 2.2.9 and 4.1.0; likely affects everything in between.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions