From cd2cfdeab07216c93e434a8f1f020be8f85d512c Mon Sep 17 00:00:00 2001 From: Guillermo Venegas Date: Tue, 23 Sep 2025 16:50:43 +0200 Subject: [PATCH] Allocator: Properly free polymorphic objects Currently `MEM_delete` frees pointers expecting that they match to the pointers allocated with `MEM_new`, otherwise it can cause undefined behavior when freeing memory(using `--debug-memory` flag breaks in place, if not it can corrupts other data, generating a incorrect back-traces). However polymorphic objects lifetime can be managed by pointer of their most derived type or by any pointer in their ancestor tree that defines a virtual destructor, which sometimes can differ in offset when pointing to the same object. This changes ensures the correct pointer is being freed, by using the pointer to the most derived type (returned by`dynamic_cast(...);`[0]). ---------- [0] = [dynamic_cast](https://en.cppreference.com/w/cpp/language/dynamic_cast.html): `a) If expression is a pointer to (possibly cv-qualified) void, the result is a pointer to the most derived object pointed to by expression.` ----------- As an example, given the followings structs: ```c++ struct A { int a; virtual ~A() = default; }; struct B { int b; virtual ~B() = default; }; struct Derived : public A , public B { int c; }; std::unique_ptr a_ptr = std::make_unique(); std::unique_ptr b_ptr = std::make_unique(); ``` Using std smart pointers to manage `Derived` objects can be done with `A` or `B` pointers. However if a `Derived` object memory is managed with `MEM_delete`, using a `B` pointer for freeing the memory currently may silently break Blender, since it don't accounts for the full object memory layout, the `dynamic_cast(ptr);` cast gives a more safe pointer for freeing the memory. Note that object destruction is successfully handled through the virtual destructor. ---------- This instead could be an assert to ensure polymorphic objects to be deleted as the most derived object type. Pull Request: https://projects.blender.org/blender/blender/pulls/146269 --- intern/guardedalloc/MEM_guardedalloc.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/intern/guardedalloc/MEM_guardedalloc.h b/intern/guardedalloc/MEM_guardedalloc.h index 5dbd01bac46..af72aeef044 100644 --- a/intern/guardedalloc/MEM_guardedalloc.h +++ b/intern/guardedalloc/MEM_guardedalloc.h @@ -426,9 +426,23 @@ template inline void MEM_delete(const T *ptr) if (ptr == nullptr) { return; } + const void *complete_ptr = [ptr]() { + if constexpr (std::is_polymorphic_v) { + /* Polymorphic objects lifetime can be managed with pointers to their most derived type or + * with pointers to any of their ancestor types in their hierarchy tree that define a virtual + * destructor, however ancestor pointers may differ in a offset from the same derived object. + * For freeing the correct memory allocated with #MEM_new, we need to ensure that the given + * pointer is equal to the pointer to the most derived object, which can be obtained with + * `dynamic_cast(ptr)`. */ + return dynamic_cast(ptr); + } + else { + return static_cast(ptr); + } + }(); /* C++ allows destruction of `const` objects, so the pointer is allowed to be `const`. */ ptr->~T(); - mem_guarded::internal::mem_freeN_ex(const_cast(ptr), + mem_guarded::internal::mem_freeN_ex(const_cast(complete_ptr), mem_guarded::internal::AllocationType::NEW_DELETE); }