[Bf-blender-cvs] [f500ef58e20] sculpt-dev: Sculpt: fix memory corruption

Joseph Eagar noreply at git.blender.org
Sun Oct 17 10:59:36 CEST 2021


Commit: f500ef58e20fc05a8a40677f682225e73acaa52a
Author: Joseph Eagar
Date:   Sun Oct 17 01:57:33 2021 -0700
Branches: sculpt-dev
https://developer.blender.org/rBf500ef58e20fc05a8a40677f682225e73acaa52a

Sculpt: fix memory corruption

* Fixed a nasty bit of memory corruption
* Along the way, added ASAN support to
  bmesh customdata blocks.
* Each CD layer is padded by 32 bytes
  inside the bmesh data block.
* Also fixed a few minor errors in
  mempool's asan support.
* Tried and failed to fix numerical
  stability issues with lasso/box
  trim brushes.

===================================================================

M	source/blender/blenkernel/BKE_pbvh.h
M	source/blender/blenkernel/intern/customdata.c
M	source/blender/blenkernel/intern/dyntopo.c
M	source/blender/blenlib/intern/BLI_mempool.c
M	source/blender/bmesh/intern/bmesh_core.c
M	source/blender/bmesh/intern/bmesh_interp.c
M	source/blender/bmesh/intern/bmesh_log.c
M	source/blender/bmesh/operators/bmo_dupe.c
M	source/blender/editors/sculpt_paint/paint_mask.c

===================================================================

diff --git a/source/blender/blenkernel/BKE_pbvh.h b/source/blender/blenkernel/BKE_pbvh.h
index ebcb82ef4fb..7893c40a2aa 100644
--- a/source/blender/blenkernel/BKE_pbvh.h
+++ b/source/blender/blenkernel/BKE_pbvh.h
@@ -997,3 +997,28 @@ BLI_INLINE bool _pbvh_nan_check(const float *co, const char *func, const char *f
   return bad;
 }
 #define PBVH_CHECK_NAN(co) _pbvh_nan_check(co, __func__, __FILE__, __LINE__)
+
+typedef struct DynTopoState DynTopoState;
+
+typedef struct DynRemeshParams {
+  float edge_size;
+  float detail_range;
+  float relax_strength;
+} DynRemeshParams;
+
+/*
+Simple wrapper api to use the dyntopo remesher in
+non-sculpt contexts.
+
+existing_pbvh can be NULL.
+
+Note that all the sculpt customdata layers will be created
+if they don't exist, so cd_vert/face_node_offset, cd_mask_offset,
+cd_sculpt_vert, etc*/
+DynTopoState *BKE_dyntopo_init(struct BMesh *bm, PBVH *existing_pbvh);
+void BKE_dyntopo_free(DynTopoState *ds);
+void BKE_dyntopo_default_params(DynRemeshParams *params, float edge_size);
+void BKE_dyntopo_remesh(DynTopoState *ds,
+                        DynRemeshParams *params,
+                        int steps,
+                        PBVHTopologyUpdateMode mode);
diff --git a/source/blender/blenkernel/intern/customdata.c b/source/blender/blenkernel/intern/customdata.c
index f199d39e395..d6c41fd66bd 100644
--- a/source/blender/blenkernel/intern/customdata.c
+++ b/source/blender/blenkernel/intern/customdata.c
@@ -34,6 +34,7 @@
 #include "DNA_hair_types.h"
 #include "DNA_meshdata_types.h"
 
+#include "BLI_asan.h"
 #include "BLI_bitmap.h"
 #include "BLI_compiler_attrs.h"
 #include "BLI_endian_switch.h"
@@ -69,6 +70,8 @@
 /* number of layers to add when growing a CustomData object */
 #define CUSTOMDATA_GROW 5
 
+#define BM_ASAN_PAD 32
+
 /* ensure typemap size is ok */
 BLI_STATIC_ASSERT(ARRAY_SIZE(((CustomData *)NULL)->typemap) == CD_NUMTYPES, "size mismatch");
 
@@ -2577,8 +2580,16 @@ static void customData_update_offsets(CustomData *data)
         size += 8 - (size & 7);
       }
 
+#if (defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer))
+      offset += BM_ASAN_PAD;
+#endif
+
       data->layers[j].offset = offset;
       offset += size;
+
+#if (defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer))
+      offset += BM_ASAN_PAD;
+#endif
     }
   }
 
@@ -2589,22 +2600,61 @@ static void customData_update_offsets(CustomData *data)
       }
 
       typeInfo = layerType_getInfo(data->layers[j].type);
+      int size = (int)typeInfo->size;
 
       if (i < ARRAY_SIZE(aligns) && typeInfo->size != aligns[i]) {
         continue;
       }
 
+#if (defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer))
+      offset += BM_ASAN_PAD;
+#endif
+
       BLI_BITMAP_SET(donemap, j, true);
 
       data->layers[j].offset = offset;
-      offset += typeInfo->size;
+      offset += size;
+
+#if (defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer))
+      offset += BM_ASAN_PAD;
+#endif
     }
   }
 
   data->totsize = offset;
+
   CustomData_update_typemap(data);
 }
 
+void CustomData_bmesh_asan_poison(const CustomData *data, void *block)
+{
+#if (defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer))
+  if (!block) {
+    return;
+  }
+
+  char *ptr = (char *)block;
+
+  BLI_asan_unpoison(block, data->totsize);
+
+  for (int i = 0; i < data->totlayer; i++) {
+    CustomDataLayer *layer = data->layers + i;
+    const LayerTypeInfo *typeInfo = layerType_getInfo(layer->type);
+
+    BLI_asan_poison((ptr + layer->offset - BM_ASAN_PAD), BM_ASAN_PAD);
+    BLI_asan_poison((ptr + layer->offset + typeInfo->size), BM_ASAN_PAD);
+  }
+#endif
+}
+
+void CustomData_bmesh_asan_unpoison(const CustomData *data, void *block)
+{
+  if (!block) {
+    return;
+  }
+  BLI_asan_unpoison(block, data->totsize);
+}
+
 /* to use when we're in the middle of modifying layers */
 static int CustomData_get_layer_index__notypemap(const CustomData *data, int type)
 {
@@ -3643,6 +3693,13 @@ int CustomData_get_offset(const CustomData *data, int type)
   return data->layers[layer_index].offset;
 }
 
+int CustomData_get_named_offset(const CustomData *data, int type, const char *name)
+{
+  int idx = CustomData_get_named_layer_index(data, type, name);
+
+  return idx == -1 ? -1 : data->layers[idx].offset;
+}
+
 int CustomData_get_n_offset(const CustomData *data, int type, int n)
 {
   /* get the layer index of the active layer of type */
@@ -4025,6 +4082,8 @@ void CustomData_bmesh_free_block(CustomData *data, void **block)
     return;
   }
 
+  CustomData_bmesh_asan_unpoison(data, *block);
+
   for (int i = 0; i < data->totlayer; i++) {
     if (!(data->layers[i].flag & CD_FLAG_NOFREE)) {
       const LayerTypeInfo *typeInfo = layerType_getInfo(data->layers[i].type);
@@ -4051,6 +4110,9 @@ void CustomData_bmesh_free_block_data(CustomData *data, void *block)
   if (block == NULL) {
     return;
   }
+
+  CustomData_bmesh_asan_unpoison(data, block);
+
   for (int i = 0; i < data->totlayer; i++) {
     if (!(data->layers[i].flag & CD_FLAG_NOFREE)) {
       const LayerTypeInfo *typeInfo = layerType_getInfo(data->layers[i].type);
@@ -4060,9 +4122,12 @@ void CustomData_bmesh_free_block_data(CustomData *data, void *block)
       }
     }
   }
+
   if (data->totsize) {
     memset(block, 0, data->totsize);
   }
+
+  CustomData_bmesh_asan_poison(data, block);
 }
 
 static void CustomData_bmesh_alloc_block(CustomData *data, void **block)
@@ -4074,6 +4139,8 @@ static void CustomData_bmesh_alloc_block(CustomData *data, void **block)
   if (data->totsize > 0) {
     *block = BLI_mempool_alloc(data->pool);
 
+    CustomData_bmesh_asan_poison(data, *block);
+
     /*clear toolflags pointer when created for the first time*/
     int cd_tflags = data->typemap[CD_TOOLFLAGS];
     if (cd_tflags != -1) {
@@ -4101,6 +4168,9 @@ void CustomData_bmesh_free_block_data_exclude_by_type(CustomData *data,
   if (block == NULL) {
     return;
   }
+
+  CustomData_bmesh_asan_unpoison(data, block);
+
   for (int i = 0; i < data->totlayer; i++) {
     if ((CD_TYPE_AS_MASK(data->layers[i].type) & mask_exclude) == 0) {
       const LayerTypeInfo *typeInfo = layerType_getInfo(data->layers[i].type);
@@ -4113,6 +4183,8 @@ void CustomData_bmesh_free_block_data_exclude_by_type(CustomData *data,
       memset(POINTER_OFFSET(block, offset), 0, typeInfo->size);
     }
   }
+
+  CustomData_bmesh_asan_poison(data, block);
 }
 
 static void CustomData_bmesh_set_default_n(CustomData *data, void **block, int n)
@@ -4146,6 +4218,9 @@ void CustomData_bmesh_set_default(CustomData *data, void **block)
 
 void CustomData_bmesh_swap_data_simple(CustomData *data, void **block1, void **block2)
 {
+  CustomData_bmesh_asan_unpoison(data, *block1);
+  CustomData_bmesh_asan_unpoison(data, *block2);
+
   int cd_id = data->typemap[CD_MESH_ID];
   cd_id = cd_id >= 0 ? data->layers[cd_id].offset : -1;
 
@@ -4178,6 +4253,9 @@ void CustomData_bmesh_swap_data_simple(CustomData *data, void **block1, void **b
       *flags2 = tmp;
     }
   }
+
+  CustomData_bmesh_asan_poison(data, *block1);
+  CustomData_bmesh_asan_poison(data, *block2);
 }
 
 void CustomData_bmesh_swap_data(CustomData *source,
@@ -4193,7 +4271,10 @@ void CustomData_bmesh_swap_data(CustomData *source,
     CustomData_bmesh_alloc_block(dest, dest_block);
 
     if (*dest_block) {
+      CustomData_bmesh_asan_unpoison(dest, *dest_block);
       memset(*dest_block, 0, dest->totsize);
+      CustomData_bmesh_asan_poison(dest, *dest_block);
+
       CustomData_bmesh_set_default(dest, dest_block);
     }
   }
@@ -4269,8 +4350,11 @@ void CustomData_bmesh_copy_data_exclude_by_type(const CustomData *source,
 
   if (*dest_block == NULL) {
     CustomData_bmesh_alloc_block(dest, dest_block);
+
     if (*dest_block) {
+      CustomData_bmesh_asan_unpoison(dest, *dest_block);
       memset(*dest_block, 0, dest->totsize);
+      CustomData_bmesh_asan_poison(dest, *dest_block);
     }
   }
 
diff --git a/source/blender/blenkernel/intern/dyntopo.c b/source/blender/blenkernel/intern/dyntopo.c
index 7a57e8d4299..79170665075 100644
--- a/source/blender/blenkernel/intern/dyntopo.c
+++ b/source/blender/blenkernel/intern/dyntopo.c
@@ -612,7 +612,7 @@ BLI_INLINE void surface_smooth_v_safe(PBVH *pbvh, BMVert *v, float fac)
     BMVert *v2 = e->v1 == v ? e->v2 : e->v1;
 
     // can't check for boundary here, thread
-    pbvh_check_vert_boundary(pbvh, v2);
+    // pbvh_check_vert_boundary(pbvh, v2);
 
     MSculptVert *mv2 = BKE_PBVH_SCULPTVERT(cd_sculpt_vert, v2);
     const bool bound2 = mv2->flag & SCULPTVERT_SMOOTH_BOUNDARY;
@@ -4668,6 +4668,11 @@ static bool do_cleanup_3_4(EdgeQueueContext *eq_ctx,
   return modified;
 }
 
+float mask_cb_nop(SculptVertRef vertex, void *userdata)
+{
+  return 1.0f;
+}
+
 /* Collapse short edges, subdivide long edges */
 bool BKE_pbvh_bmesh_update_topology(PBVH *pbvh,
                                     PBVHTopologyUpdateMode mode,
@@ -4749,30 +4754,6 @@ bool BKE_pbvh_bmesh_update_topology(PBVH *pbvh,
     safe_smooth = DYNTOPO_SAFE_SMOOTH_FAC;
   }
 
-  /*
-
-
-typedef struct EdgeQueueContext {
-  EdgeQueue *q;
-  BLI_mempool *pool;
-  BMesh *bm;
-  DyntopoMaskCB mask_cb;
-  void *mask_cb_data;
-  int cd_sculpt_vert;
-  int cd_vert_mask_offset;
-  int cd_vert_node_offset;
-  int cd_face_node_offset;
-  float avg_elen;
-  float max_elen;
-  float min_elen;
-  float totedge;
-  BMVert **val34_verts;
-  int val34_verts_tot;
-  int val34_verts_size;
-  bool local_mode;
-  float surface_smooth_fac;
-} EdgeQueueContext;
-*/
   EdgeQueueContext eq_ctx = {.q = NULL,
                              .pool = NULL,
                              .bm = pbvh->bm,
@@ -5702,3 +5683,271 @@ static void pbvh_split_edges(EdgeQueueContext *eq_ctx,
   }
 #endif
 }
+
+extern char dyntopop_node_idx_layer_id[];
+extern char dyntopop_faces_areas_layer_id[];
+
+typedef struct DynTopoState {
+  PBVH *pbvh;
+  bool is_fake_pbvh;
+} DynTopoState;
+
+/* existing_pbvh may be NULL, if so a fake one will be created.
+Note that all the sculpt customdata layers will be created
+if they don't exist, so cd_vert/face_node_offset, cd_mask_offset,
+cd_sculpt_vert, etc*/
+DynTopoState *BKE_dyntopo_init(BMesh *bm, PBVH *existing_p

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list