Improve consistency of MEM_guardedalloc usage in C++ code #124512

New Issue

Bastien Montagne · 2024-07-11T12:48:37+02:00

Bastien Montagne commented

2024-07-11 12:48:37 +02:00

The usage of our guarded allocator code with C++ data creation (new/delete) is confusing at best in current code.

MEM_new: Allocates required amount of memory (using guardedalloc code) and calls the default 'placement new' operation on it.
MEM_CXX_CLASS_ALLOC_FUNCS: This macro defines new/delete operators for a given C++ type, using the guardedalloc code. Most of its usages are conditioned by the CMake WITH_CXX_GUARDEDALLOC setting.
WITH_CXX_GUARDEDALLOC: This CMake setting also conditions the global override of new/delete operators to use guardedalloc code. It is disabled by default.

The fact that WITH_CXX_GUARDEDALLOC is disabled by default means that it is barely ever used. Often enabling it will reveal some memory management issues.

The global overriding of new/delete operators is most likely not worth the risk of inconsistencies (it makes it too easy to run into situations where some data allocated from a linked library with 'normal' new gets deleted by the 'guardedalloc' new e.g.)

Proposal

Remove WITH_CXX_GUARDEDALLOC: There should not be an option to disable guardedalloc in most common cases.
- **Remove the global override of new/delete.
- **Always use MEM_CXX_CLASS_ALLOC_FUNCS where it is currently used.

Question

Do we want to keep the two current systems (MEM_new/MEM_delete and MEM_CXX_CLASS_ALLOC_FUNCS)?

MEM_new/MEM_delete has the advantage of being 'universal', and allows to control where usage of guardedalloc is desired, and where standard new allocation can be used instead.

MEM_CXX_CLASS_ALLOC_FUNCS allows to use standard new/delete C++ code for Blender C++ types, while automatically using the guardedallocator.

The usage of our guarded allocator code with C++ data creation (`new`/`delete`) is confusing at best in current code. * `MEM_new`: Allocates required amount of memory (using guardedalloc code) and calls the default 'placement new' operation on it. * `MEM_CXX_CLASS_ALLOC_FUNCS`: This macro defines new/delete operators for a given C++ type, using the guardedalloc code. Most of its usages are conditioned by the CMake `WITH_CXX_GUARDEDALLOC` setting. * `WITH_CXX_GUARDEDALLOC`: This CMake setting also conditions the global override of new/delete operators to use guardedalloc code. It is disabled by default. The fact that `WITH_CXX_GUARDEDALLOC` is disabled by default means that it is barely ever used. Often enabling it will reveal some memory management issues. The global overriding of `new`/`delete` operators is most likely not worth the risk of inconsistencies (it makes it too easy to run into situations where some data allocated from a linked library with 'normal' new gets deleted by the 'guardedalloc' new e.g.) ## Proposal * **Remove `WITH_CXX_GUARDEDALLOC`**: There should not be an option to disable guardedalloc in most common cases. * **Remove the global override of `new`/`delete`. * **Always use `MEM_CXX_CLASS_ALLOC_FUNCS` where it is currently used. ## Question Do we want to keep the two current systems (`MEM_new`/`MEM_delete` and `MEM_CXX_CLASS_ALLOC_FUNCS`)? `MEM_new`/`MEM_delete` has the advantage of being 'universal', and allows to control where usage of guardedalloc is desired, and where standard `new` allocation can be used instead. `MEM_CXX_CLASS_ALLOC_FUNCS` allows to use standard new/delete C++ code for Blender C++ types, while automatically using the guardedallocator.

Bastien Montagne added the

Type

Design

label 2024-07-11 12:48:37 +02:00

Bastien Montagne added this to the Module: Core project 2024-07-11 12:48:38 +02:00

Jacques Lucke commented

2024-10-02 11:13:21 +02:00

Do we want to keep the two current systems (MEM_new/MEM_delete and MEM_CXX_CLASS_ALLOC_FUNCS)?

I'm not fully sure yet. It's nice that with MEM_CXX_CLASS_ALLOC_FUNCS one can just use normal std::make_unique etc. Could make our own version of that though, but that doesn't seem nice. I also still have to check how overriding operator new works with child classes.

Just wanted to mention some downsides of MEM_CXX_CLASS_ALLOC_FUNCS:

Currently, that always has to use aligned allocation internally, because our allocator is usually 8 byte, but not 16 byte aligned which new seems to expect in some cases (see __STDCPP_DEFAULT_NEW_ALIGNMENT__).
It's not possible to give a custom allocation name at the place of allocation. When trying to find memory leaks, I often find it useful when the allocation name contains information about where the memory was allocated and not just what type it is.

> Do we want to keep the two current systems (MEM_new/MEM_delete and MEM_CXX_CLASS_ALLOC_FUNCS)? I'm not fully sure yet. It's nice that with `MEM_CXX_CLASS_ALLOC_FUNCS` one can just use normal `std::make_unique` etc. Could make our own version of that though, but that doesn't seem nice. I also still have to check how overriding `operator new` works with child classes. Just wanted to mention some downsides of `MEM_CXX_CLASS_ALLOC_FUNCS`: * Currently, that always has to use aligned allocation internally, because our allocator is usually 8 byte, but not 16 byte aligned which `new` seems to expect in some cases (see `__STDCPP_DEFAULT_NEW_ALIGNMENT__`). * It's not possible to give a custom allocation name at the place of allocation. When trying to find memory leaks, I often find it useful when the allocation name contains information about where the memory was allocated and not just what type it is.

Bastien Montagne commented

2024-10-02 11:21:54 +02:00

TBH, since we entered the ASAN era, I very rarely use/need our guardedallocation name info anymore?

Jacques Lucke referenced this issue

2024-11-12 16:21:25 +01:00

Core: remove WITH_CXX_GUARDEDALLOC option #130181

Bastien Montagne changed title from ~~WIP: Improve consistency of MEM_guardedalloc usage in C++ code~~ to Improve consistency of MEM_guardedalloc usage in C++ code

2024-11-13 13:28:53 +01:00

Jacques Lucke referenced this issue from a commit

2024-11-13 13:40:09 +01:00

Core: remove WITH_CXX_GUARDEDALLOC option

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Improve consistency of MEM_guardedalloc usage in C++ code #124512

Proposal

Question