Fix T56108: Crash editing corrupted vertex groups
authorCampbell Barton <ideasman42@gmail.com>
Tue, 11 Feb 2020 01:35:10 +0000 (12:35 +1100)
committerCampbell Barton <ideasman42@gmail.com>
Tue, 11 Feb 2020 02:20:49 +0000 (13:20 +1100)
While the file in this report had corrupted values,
this is avoidable without adding any extra overhead.

Use unsigned vertex group indices since we don't need negative values,
this is an alternative to checking they aren't negative in many places.

Vertex group values over INT_MAX is still considered invalid,
so any accidental unsigned wrapping won't be silently ignored.

source/blender/blenkernel/BKE_deform.h
source/blender/blenkernel/intern/armature.c
source/blender/blenkernel/intern/deform.c
source/blender/blenkernel/intern/mesh_validate.c
source/blender/collada/ControllerExporter.cpp
source/blender/makesdna/DNA_meshdata_types.h
source/blender/modifiers/intern/MOD_mask.c

index ca06716599c4e6f660e0296bc854ed5cad3c2e50..2911002b9e98b32a80780d5c48113ec624ef0d01 100644 (file)
@@ -106,7 +106,7 @@ void defvert_normalize_subset(struct MDeformVert *dvert,
 void defvert_normalize_lock_single(struct MDeformVert *dvert,
                                    const bool *vgroup_subset,
                                    const int vgroup_tot,
-                                   const int def_nr_lock);
+                                   const uint def_nr_lock);
 void defvert_normalize_lock_map(struct MDeformVert *dvert,
                                 const bool *vgroup_subset,
                                 const int vgroup_tot,
index ed752986dddc8d81f747754c31b9df85e6e934b7..d4498017ffdb8d1e3806adeadae4d9c0e1fb8f1d 100644 (file)
@@ -1562,8 +1562,8 @@ static void armature_vert_task(void *__restrict userdata,
     int deformed = 0;
     unsigned int j;
     for (j = dvert->totweight; j != 0; j--, dw++) {
-      const int index = dw->def_nr;
-      if (index >= 0 && index < data->defbase_tot && (pchan = data->defnrToPC[index])) {
+      const uint index = dw->def_nr;
+      if (index < data->defbase_tot && (pchan = data->defnrToPC[index])) {
         float weight = dw->weight;
         Bone *bone = pchan->bone;
 
index 79dcdd15bf7dcc6227f51ac646f80bf93cc78d7d..41ff9594cffc4cb6ab4bf15a36226b279a14ea46 100644 (file)
@@ -255,10 +255,9 @@ void defvert_remap(MDeformVert *dvert, int *map, const int map_len)
   unsigned int i;
   for (i = dvert->totweight; i != 0; i--, dw++) {
     if (dw->def_nr < map_len) {
-      dw->def_nr = map[dw->def_nr];
+      BLI_assert(map[dw->def_nr] >= 0);
 
-      /* just in case */
-      BLI_assert(dw->def_nr >= 0);
+      dw->def_nr = map[dw->def_nr];
     }
   }
 }
@@ -337,7 +336,7 @@ void defvert_normalize(MDeformVert *dvert)
 void defvert_normalize_lock_single(MDeformVert *dvert,
                                    const bool *vgroup_subset,
                                    const int vgroup_tot,
-                                   const int def_nr_lock)
+                                   const uint def_nr_lock)
 {
   if (dvert->totweight == 0) {
     /* nothing */
index 4aa5bfa04ab2e9a049c18b6075c90cf931a47e60..e4de75b923fdf59b60ff2f58f5c92f0e41187f07 100644 (file)
@@ -784,22 +784,24 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
       for (j = 0, dw = dv->dw; j < dv->totweight; j++, dw++) {
         /* note, greater than max defgroups is accounted for in our code, but not < 0 */
         if (!isfinite(dw->weight)) {
-          PRINT_ERR("\tVertex deform %u, group %d has weight: %f", i, dw->def_nr, dw->weight);
+          PRINT_ERR("\tVertex deform %u, group %u has weight: %f", i, dw->def_nr, dw->weight);
           if (do_fixes) {
             dw->weight = 0.0f;
             fix_flag.verts_weight = true;
           }
         }
         else if (dw->weight < 0.0f || dw->weight > 1.0f) {
-          PRINT_ERR("\tVertex deform %u, group %d has weight: %f", i, dw->def_nr, dw->weight);
+          PRINT_ERR("\tVertex deform %u, group %u has weight: %f", i, dw->def_nr, dw->weight);
           if (do_fixes) {
             CLAMP(dw->weight, 0.0f, 1.0f);
             fix_flag.verts_weight = true;
           }
         }
 
-        if (dw->def_nr < 0) {
-          PRINT_ERR("\tVertex deform %u, has invalid group %d", i, dw->def_nr);
+        /* Not technically incorrect since this is unsigned, however,
+         * a value over INT_MAX is almost certainly caused by wrapping an unsigned int. */
+        if (dw->def_nr >= INT_MAX) {
+          PRINT_ERR("\tVertex deform %u, has invalid group %u", i, dw->def_nr);
           if (do_fixes) {
             defvert_remove_group(dv, dw);
             fix_flag.verts_weight = true;
index ef91fcfa62c55e2aaf6b9b911bf72c9da6d2a1b8..0119aba7dfd2ff4934002b8a71af3da8226f12c0 100644 (file)
@@ -230,19 +230,17 @@ void ControllerExporter::export_skin_controller(Object *ob, Object *ob_arm)
       float sumw = 0.0f;
 
       for (j = 0; j < vert->totweight; j++) {
-        int idx = vert->dw[j].def_nr;
+        uint idx = vert->dw[j].def_nr;
         if (idx >= joint_index_by_def_index.size()) {
           /* XXX: Maybe better find out where and
            *      why the Out Of Bound indexes get created ? */
           oob_counter += 1;
         }
         else {
-          if (idx >= 0) {
-            int joint_index = joint_index_by_def_index[idx];
-            if (joint_index != -1 && vert->dw[j].weight > 0.0f) {
-              jw[joint_index] += vert->dw[j].weight;
-              sumw += vert->dw[j].weight;
-            }
+          int joint_index = joint_index_by_def_index[idx];
+          if (joint_index != -1 && vert->dw[j].weight > 0.0f) {
+            jw[joint_index] += vert->dw[j].weight;
+            sumw += vert->dw[j].weight;
           }
         }
       }
index 648389d610f6ac3d697ff1306158569c73dab5f6..be904c5bf9437dcd9fed1a57a8298dff156f85e9 100644 (file)
@@ -280,7 +280,7 @@ typedef struct MStringProperty {
  */
 typedef struct MDeformWeight {
   /** The index for the vertex group, must *always* be unique when in an array. */
-  int def_nr;
+  unsigned int def_nr;
   /** Weight between 0.0 and 1.0. */
   float weight;
 } MDeformWeight;
index 8d0f6825ee0512a6e8399ab35ca100bdf9b73075..00b0068bd11b452ad946a530f92c63dc0230a744 100644 (file)
@@ -131,7 +131,7 @@ static Mesh *applyModifier(ModifierData *md, const ModifierEvalContext *ctx, Mes
     bDeformGroup *def;
     bool *bone_select_array;
     int bone_select_tot = 0;
-    const int defbase_tot = BLI_listbase_count(&ob->defbase);
+    const uint defbase_tot = (uint)BLI_listbase_count(&ob->defbase);
 
     /* check that there is armature object with bones to use, otherwise return original mesh */
     if (ELEM(NULL, oba, oba->pose, ob->defbase.first)) {