From 14621e7720da31aaff4d1eec29886b69077569ba Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Wed, 22 Dec 2021 16:28:45 -0600 Subject: Fix: Potential use after scope in curve to mesh node I don't think this has been visible, since I only ran into it after changing other code that affected this. However, some attributes can keep a reference to the source component to use when tagging caches dirty (like the position attribute tagging the normals dirty). Here, the component was created inside a function, then the attributes were used afterwards. Also add some comments warning about this in the header file. --- source/blender/blenkernel/BKE_attribute_access.hh | 13 ++++++++++--- source/blender/blenkernel/intern/curve_to_mesh_convert.cc | 10 ++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) (limited to 'source/blender') diff --git a/source/blender/blenkernel/BKE_attribute_access.hh b/source/blender/blenkernel/BKE_attribute_access.hh index fd30813a506..ae9969aae82 100644 --- a/source/blender/blenkernel/BKE_attribute_access.hh +++ b/source/blender/blenkernel/BKE_attribute_access.hh @@ -181,11 +181,14 @@ struct ReadAttributeLookup { * Used when looking up a "plain attribute" based on a name for reading from it and writing to it. */ struct WriteAttributeLookup { - /* The virtual array that is used to read from and write to the attribute. */ + /** The virtual array that is used to read from and write to the attribute. */ GVMutableArray varray; - /* Domain the attributes lives on in the geometry. */ + /** Domain the attributes lives on in the geometry component. */ AttributeDomain domain; - /* Call this after changing the attribute to invalidate caches that depend on this attribute. */ + /** + * Call this after changing the attribute to invalidate caches that depend on this attribute. + * \note Do not call this after the component the attribute is from has been destructed. + */ std::function tag_modified_fn; /* Convenience function to check if the attribute has been found. */ @@ -205,6 +208,10 @@ struct WriteAttributeLookup { * VMutableArray_Span in many cases). * - An output attribute can live side by side with an existing attribute with a different domain * or data type. The old attribute will only be overwritten when the #save function is called. + * + * \note The lifetime of an output attribute should not be longer than the the lifetime of the + * geometry component it comes from, since it can keep a reference to the component for use in + * the #save method. */ class OutputAttribute { public: diff --git a/source/blender/blenkernel/intern/curve_to_mesh_convert.cc b/source/blender/blenkernel/intern/curve_to_mesh_convert.cc index 5522a84d094..8c256da44cd 100644 --- a/source/blender/blenkernel/intern/curve_to_mesh_convert.cc +++ b/source/blender/blenkernel/intern/curve_to_mesh_convert.cc @@ -401,10 +401,8 @@ struct ResultAttributes { }; static ResultAttributes create_result_attributes(const CurveEval &curve, const CurveEval &profile, - Mesh &mesh) + MeshComponent &mesh_component) { - MeshComponent mesh_component; - mesh_component.replace(&mesh, GeometryOwnershipType::Editable); Set curve_attributes; /* In order to prefer attributes on the main curve input when there are name collisions, first @@ -708,7 +706,11 @@ Mesh *curve_to_mesh_sweep(const CurveEval &curve, const CurveEval &profile, cons mesh->smoothresh = DEG2RADF(180.0f); BKE_mesh_normals_tag_dirty(mesh); - ResultAttributes attributes = create_result_attributes(curve, profile, *mesh); + /* Create the mesh component for retrieving attributes at this scope, since output attributes + * can keep a reference to the component for updating after retrieving write access. */ + MeshComponent mesh_component; + mesh_component.replace(mesh, GeometryOwnershipType::Editable); + ResultAttributes attributes = create_result_attributes(curve, profile, mesh_component); threading::parallel_for(curves.index_range(), 128, [&](IndexRange curves_range) { for (const int i_spline : curves_range) { -- cgit v1.2.3