Fix #119589: use-after-free when accessing not-fully-evaluated object geometry

While the evaluated result is not well defined, we expect Blender to not crash
when there are dependency cycles.

The evaluation of one object often takes the evaluated geometry of another
object into account. This works fine if the other object is already fully
evaluated. However, if there is a dependency cycle, the other object may not be
evaluated already. Currently, we have no way to check for this and were mostly
just relying on luck that the other objects geometry is in some valid state
(even if it's not the fully evaluated geometry).

This patch adds the ability to explicitly check if an objects geometry is fully
evaluated already, so that it can be accessed by other objects. If there are not
dependency cycles, this should always be true. If not, it may be false
sometimes, and in this case the other objects geometry should be ignored. The
same also applies to the object transforms and the geometry of a collection.

For that, new functions are added in `DEG_depsgraph_query.hh`. Those should be
used whenever accessing another objects or collections object during depsgraph
evaluation. More similar functions may be added in the future.
```
bool DEG_object_geometry_is_evaluated(const Object &object);
bool DEG_object_transform_is_evaluated(const Object &object);
bool DEG_collection_geometry_is_evaluated(const Collection &collection);
```

To determine if the these components are fully evaluated, a reference to the
corresponding depsgraph is needed. A possible solution to that is to pass the
depsgraph through the call stack to these functions. While possible, there are a
couple of annoyances. For one, the parameter would need to be added in many new
places. I don't have an exact number, but it's like 50 or so. Another
complication is that under some circumstances, multiple depsgraphs may have to
be passed around, for example when evaluating node tools (also see
`GeoNodesOperatorDepsgraphs`).

To simplify the patch and other code in the future, a different route is taken
where the depsgraph pointer is added to `ID_Runtime`, making it readily
accessible similar to the `ID.orig_id`. The depsgraph pointer is set in the same
place where the `orig_id` is set.

As a nice side benefit, this also improves the situation in simple cases like
having two cubes with a boolean modifier and they union each other.

Pull Request: https://projects.blender.org/blender/blender/pulls/123444

source/blender/blenkernel/intern/geometry_set_instances.cc

@@ -13,6 +13,8 @@
 #include "DNA_layer_types.h"
 #include "DNA_object_types.h"
 
+#include "DEG_depsgraph_query.hh"
+
 namespace blender::bke {
 
 static void add_final_mesh_as_geometry_component(const Object &object, GeometrySet &geometry_set)
@@ -28,6 +30,9 @@ static void add_final_mesh_as_geometry_component(const Object &object, GeometryS
 
 GeometrySet object_get_evaluated_geometry_set(const Object &object)
 {
+  if (!DEG_object_geometry_is_evaluated(object)) {
+    return {};
+  }
   if (object.type == OB_MESH && object.mode == OB_MODE_EDIT) {
     GeometrySet geometry_set;
     if (object.runtime->geometry_set_eval != nullptr) {

source/blender/blenkernel/intern/modifier.cc

@@ -944,6 +944,10 @@ void BKE_modifier_deform_vertsEM(ModifierData *md,
 
 Mesh *BKE_modifier_get_evaluated_mesh_from_evaluated_object(Object *ob_eval)
 {
+  if (!DEG_object_geometry_is_evaluated(*ob_eval)) {
+    return nullptr;
+  }
+
   Mesh *mesh = nullptr;
 
   if ((ob_eval->type == OB_MESH) && (ob_eval->mode & OB_MODE_EDIT)) {

source/blender/blenkernel/intern/object.cc

@@ -4138,6 +4138,10 @@ bool BKE_object_obdata_texspace_get(Object *ob,
 
 Mesh *BKE_object_get_evaluated_mesh_no_subsurf(const Object *object)
 {
+  if (!DEG_object_geometry_is_evaluated(*object)) {
+    return nullptr;
+  }
+
   /* First attempt to retrieve the evaluated mesh from the evaluated geometry set. Most
    * object types either store it there or add a reference to it if it's owned elsewhere. */
   blender::bke::GeometrySet *geometry_set_eval = object->runtime->geometry_set_eval;
@@ -4165,6 +4169,10 @@ Mesh *BKE_object_get_evaluated_mesh_no_subsurf(const Object *object)
 
 Mesh *BKE_object_get_evaluated_mesh(const Object *object)
 {
+  if (!DEG_object_geometry_is_evaluated(*object)) {
+    return nullptr;
+  }
+
   Mesh *mesh = BKE_object_get_evaluated_mesh_no_subsurf(object);
   if (!mesh) {
     return nullptr;

source/blender/depsgraph/DEG_depsgraph_query.hh

@@ -105,6 +105,16 @@ Object *DEG_get_original_object(Object *object);
 /** Get original version of given evaluated ID data-block. */
 ID *DEG_get_original_id(ID *id);
 
+/**
+ * Get the depsgraph that owns the given ID. This is efficient because the depsgraph is cached on
+ * the ID.
+ *
+ * Only IDs that use the copy-on-eval mechanism of depsgraph evaluation have a corresponding
+ * depsgraph. Original IDs as well as temporary IDs e.g. generated by geometry nodes do not have a
+ * corresponding depsgraph, so null is returned here for these IDs.
+ */
+Depsgraph *DEG_get_depsgraph_by_id(const ID &id);
+
 /**
  * Check whether given ID is an original.
  *
@@ -134,6 +144,24 @@ bool DEG_is_fully_evaluated(const Depsgraph *depsgraph);
  */
 bool DEG_id_is_fully_evaluated(const Depsgraph *depsgraph, const ID *id_eval);
 
+/**
+ * Returns false when the objects geometry is not fully evaluated in its depsgraph yet. In this
+ * case, the geometry must not be accessed. Otherwise returns true when geometry is fully evaluated
+ * or the object does not belong to any specific depsgraph.
+ *
+ * The result of this function is non deterministic when multi-threading is used because the
+ * depsgraph nodes are not totally ordered. When the depsgraph contains all correct relations and
+ * there are no cycles, the result should always be `true` here though. So it does not break
+ * determinism when there are no dependency graph cycles.
+ */
+bool DEG_object_geometry_is_evaluated(const Object &object);
+
+/** Same as above but for the transformation of the object. */
+bool DEG_object_transform_is_evaluated(const Object &object);
+
+/** Same as above but for the geometry of all objects in the collection. */
+bool DEG_collection_geometry_is_evaluated(const Collection &collection);
+
 /** \} */
 
 /* -------------------------------------------------------------------- */

source/blender/depsgraph/intern/depsgraph.cc

@@ -113,7 +113,7 @@ IDNode *Depsgraph::add_id_node(ID *id, ID *id_cow_hint)
   if (!id_node) {
     DepsNodeFactory *factory = type_get_factory(NodeType::ID_REF);
     id_node = (IDNode *)factory->create_node(id, "", id->name);
-    id_node->init_copy_on_write(id_cow_hint);
+    id_node->init_copy_on_write(*this, id_cow_hint);
     /* Register node in ID hash.
      *
      * NOTE: We address ID nodes by the original ID pointer they are

source/blender/depsgraph/intern/depsgraph_query.cc

@@ -288,6 +288,11 @@ ID *DEG_get_original_id(ID *id)
   return deg::get_original_id(id);
 }
 
+Depsgraph *DEG_get_depsgraph_by_id(const ID &id)
+{
+  return id.runtime.depsgraph;
+}
+
 bool DEG_is_original_id(const ID *id)
 {
   /* Some explanation of the logic.
@@ -360,3 +365,54 @@ bool DEG_id_is_fully_evaluated(const Depsgraph *depsgraph, const ID *id_eval)
   }
   return true;
 }
+
+static bool operation_needs_update(const ID &id,
+                                   const deg::NodeType component_type,
+                                   const deg::OperationCode opcode)
+{
+  const Depsgraph *depsgraph = DEG_get_depsgraph_by_id(id);
+  if (!depsgraph) {
+    return false;
+  }
+  const deg::Depsgraph &deg_graph = *reinterpret_cast<const deg::Depsgraph *>(depsgraph);
+  /* Only us the original ID pointer to look up the IDNode, do not dereference it. */
+  const ID *id_orig = deg::get_original_id(&id);
+  if (!id_orig) {
+    return false;
+  }
+  const deg::IDNode *id_node = deg_graph.find_id_node(id_orig);
+  if (!id_node) {
+    return false;
+  };
+  const deg::ComponentNode *component_node = id_node->find_component(component_type);
+  if (!component_node) {
+    return false;
+  }
+  const deg::OperationNode *operation_node = component_node->find_operation(opcode);
+  if (!operation_node) {
+    return false;
+  }
+  /* Technically, there is potential for a race condition here, because the depsgraph evaluation
+   * might update this flag, but it's very unlikely to cause issues right now. Maybe this should
+   * become an atomic eventually. */
+  const bool needs_update = operation_node->flag & deg::DEPSOP_FLAG_NEEDS_UPDATE;
+  return needs_update;
+}
+
+bool DEG_object_geometry_is_evaluated(const Object &object)
+{
+  return !operation_needs_update(
+      object.id, deg::NodeType::GEOMETRY, deg::OperationCode::GEOMETRY_EVAL);
+}
+
+bool DEG_object_transform_is_evaluated(const Object &object)
+{
+  return !operation_needs_update(
+      object.id, deg::NodeType::TRANSFORM, deg::OperationCode::TRANSFORM_FINAL);
+}
+
+bool DEG_collection_geometry_is_evaluated(const Collection &collection)
+{
+  return !operation_needs_update(
+      collection.id, deg::NodeType::GEOMETRY, deg::OperationCode::GEOMETRY_EVAL_DONE);
+}

source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc

@@ -832,7 +832,7 @@ ID *deg_expand_eval_copy_datablock(const Depsgraph *depsgraph, const IDNode *id_
 #endif
   /* Do it now, so remapping will understand that possibly remapped self ID
    * is not to be remapped again. */
-  deg_tag_eval_copy_id(id_cow, id_orig);
+  deg_tag_eval_copy_id(const_cast<Depsgraph &>(*depsgraph), id_cow, id_orig);
   /* Perform remapping of the nodes. */
   RemapCallbackUserData user_data = {nullptr};
   user_data.depsgraph = depsgraph;
@@ -1050,7 +1050,7 @@ bool deg_validate_eval_copy_datablock(ID *id_cow)
   return data.is_valid;
 }
 
-void deg_tag_eval_copy_id(ID *id_cow, const ID *id_orig)
+void deg_tag_eval_copy_id(deg::Depsgraph &depsgraph, ID *id_cow, const ID *id_orig)
 {
   BLI_assert(id_cow != id_orig);
   BLI_assert((id_orig->tag & LIB_TAG_COPIED_ON_EVAL) == 0);
@@ -1058,6 +1058,7 @@ void deg_tag_eval_copy_id(ID *id_cow, const ID *id_orig)
   /* This ID is no longer localized, is a self-sustaining copy now. */
   id_cow->tag &= ~LIB_TAG_LOCALIZED;
   id_cow->orig_id = (ID *)id_orig;
+  id_cow->runtime.depsgraph = &reinterpret_cast<::Depsgraph &>(depsgraph);
 }
 
 bool deg_eval_copy_is_expanded(const ID *id_cow)

source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.h

@@ -56,8 +56,8 @@ void deg_create_eval_copy(struct ::Depsgraph *depsgraph, const struct IDNode *id
  */
 bool deg_validate_eval_copy_datablock(ID *id_cow);
 
-/** Tag given ID block as being copy-on-written. */
-void deg_tag_eval_copy_id(struct ID *id_cow, const struct ID *id_orig);
+/** Tag given ID block as being copy-on-eval. */
+void deg_tag_eval_copy_id(Depsgraph &depsgraph, struct ID *id_cow, const struct ID *id_orig);
 
 /**
  * Check whether ID data-block is expanded.

source/blender/depsgraph/intern/node/deg_node_id.cc

@@ -79,7 +79,7 @@ void IDNode::init(const ID *id, const char * /*subdata*/)
   previously_visible_components_mask = 0;
 }
 
-void IDNode::init_copy_on_write(ID *id_cow_hint)
+void IDNode::init_copy_on_write(Depsgraph &depsgraph, ID *id_cow_hint)
 {
   /* Create pointer as early as possible, so we can use it for function
    * bindings. Rest of data we'll be copying to the new datablock when
@@ -97,7 +97,7 @@ void IDNode::init_copy_on_write(ID *id_cow_hint)
     id_cow = (ID *)BKE_libblock_alloc_notest(GS(id_orig->name));
     DEG_COW_PRINT(
         "Create shallow copy for %s: id_orig=%p id_cow=%p\n", id_orig->name, id_orig, id_cow);
-    deg_tag_eval_copy_id(id_cow, id_orig);
+    deg_tag_eval_copy_id(depsgraph, id_cow, id_orig);
   }
   else {
     id_cow = id_orig;

source/blender/depsgraph/intern/node/deg_node_id.hh

@@ -44,7 +44,7 @@ struct IDNode : public Node {
 
   /** Initialize 'id' node - from pointer data given. */
   virtual void init(const ID *id, const char *subdata) override;
-  void init_copy_on_write(ID *id_cow_hint = nullptr);
+  void init_copy_on_write(Depsgraph &depsgraph, ID *id_cow_hint = nullptr);
   ~IDNode();
   void destroy();
 

source/blender/makesdna/DNA_ID.h

@@ -33,6 +33,7 @@ struct ID;
 struct Library;
 struct PackedFile;
 struct UniqueName_Map;
+struct Depsgraph;
 
 /* Runtime display data */
 struct DrawData;
@@ -401,6 +402,13 @@ typedef struct ID_Runtime_Remap {
 
 typedef struct ID_Runtime {
   ID_Runtime_Remap remap;
+  /**
+   * The depsgraph that owns this data block. This is only set on data-blocks which are
+   * copied-on-eval by the depsgraph. Additional data-blocks created during depsgraph evaluation
+   * are not owned by any specific depsgraph and thus this pointer is null for those.
+   */
+  struct Depsgraph *depsgraph;
+  void *_pad;
 } ID_Runtime;
 
 typedef struct ID {

source/blender/nodes/geometry/nodes/node_geo_collection_info.cc

@@ -14,6 +14,8 @@
 #include "BKE_collection.hh"
 #include "BKE_instances.hh"
 
+#include "DEG_depsgraph_query.hh"
+
 #include "node_geometry_util.hh"
 
 #include <algorithm>
@@ -70,6 +72,13 @@ static void node_geo_exec(GeoNodeExecParams params)
     params.set_default_remaining_outputs();
     return;
   }
+  if (!DEG_collection_geometry_is_evaluated(*collection)) {
+    params.error_message_add(NodeWarningType::Error,
+                             TIP_("Can't access collections geometry because it's not evaluated "
+                                  "yet. This can happen when there is a dependency cycle"));
+    params.set_default_remaining_outputs();
+    return;
+  }
 
   const NodeGeometryCollectionInfo &storage = node_storage(params.node());
   const bool use_relative_transform = (storage.transform_space ==

source/blender/nodes/geometry/nodes/node_geo_object_info.cc

@@ -19,6 +19,8 @@
 
 #include "GEO_transform.hh"
 
+#include "BLT_translation.hh"
+
 #include "node_geometry_util.hh"
 
 namespace blender::nodes::node_geo_object_info_cc {
@@ -59,6 +61,25 @@ static void node_geo_exec(GeoNodeExecParams params)
     params.set_default_remaining_outputs();
     return;
   }
+  if (!DEG_object_geometry_is_evaluated(*object)) {
+    params.error_message_add(NodeWarningType::Error,
+                             TIP_("Can't access object's geometry because it's not evaluated yet. "
+                                  "This can happen when there is a dependency cycle"));
+    params.set_default_remaining_outputs();
+    return;
+  }
+  if (transform_space_relative) {
+    if (!DEG_object_transform_is_evaluated(*object) ||
+        !DEG_object_transform_is_evaluated(*self_object))
+    {
+      params.error_message_add(
+          NodeWarningType::Error,
+          TIP_("Can't access objects transforms because it's not evaluated yet. "
+               "This can happen when there is a dependency cycle"));
+      params.set_default_remaining_outputs();
+      return;
+    }
+  }
 
   const float4x4 &object_matrix = object->object_to_world();
   const float4x4 transform = self_object->world_to_object() * object_matrix;