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<void *>(...);`[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> a_ptr = std::make_unique<Derived>();
std::unique_ptr<B> b_ptr = std::make_unique<Derived>();
```
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<void *>(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
This commit is contained in:
Guillermo Venegas
2025-09-23 16:50:43 +02:00
committed by Bastien Montagne
parent 6872fb1aaf
commit cd2cfdeab0

View File

@@ -426,9 +426,23 @@ template<typename T> inline void MEM_delete(const T *ptr)
if (ptr == nullptr) {
return;
}
const void *complete_ptr = [ptr]() {
if constexpr (std::is_polymorphic_v<T>) {
/* 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<void *>(ptr)`. */
return dynamic_cast<const void *>(ptr);
}
else {
return static_cast<const void *>(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<T *>(ptr),
mem_guarded::internal::mem_freeN_ex(const_cast<void *>(complete_ptr),
mem_guarded::internal::AllocationType::NEW_DELETE);
}