From c5feb0cada9e97b6ac0f180821f1e48a5dcdba84 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Thu, 10 Aug 2023 11:28:01 +0200 Subject: [PATCH] MEM_guarded_alloc: Fix LSAN not reporting memory leaks. The fact that the guarded-allocated memory blocks are all linked to the static `membase` listbase is enough for LSAN to not detect them as leaks. So this commit adds a new (private) callback to clear the memlist, which is only used in the destructor of the `MemLeakDetector` class. Many thanks to @Sergey for identifying the root issue here. Pull Request: https://projects.blender.org/blender/blender/pulls/110995 --- intern/guardedalloc/intern/leak_detector.cc | 5 +++++ intern/guardedalloc/intern/mallocn.c | 6 ++++++ .../guardedalloc/intern/mallocn_guarded_impl.c | 4 ++++ intern/guardedalloc/intern/mallocn_intern.h | 16 ++++++++++++++++ .../guardedalloc/intern/mallocn_lockfree_impl.c | 2 ++ 5 files changed, 33 insertions(+) diff --git a/intern/guardedalloc/intern/leak_detector.cc b/intern/guardedalloc/intern/leak_detector.cc index 4ba257faa52..cfd8f19ecc9 100644 --- a/intern/guardedalloc/intern/leak_detector.cc +++ b/intern/guardedalloc/intern/leak_detector.cc @@ -41,6 +41,11 @@ class MemLeakPrinter { double(mem_in_use) / 1024 / 1024); MEM_printmemlist(); + /* In guarded implementation, the fact that all allocated memory blocks are stored in the + * static `membase` listbase is enough for LSAN to not detect them as leaks. Clearing it solves + * that issue. */ + mem_clearmemlist(); + if (fail_on_memleak) { /* There are many other ways to change the exit code to failure here: * - Make the destructor `noexcept(false)` and throw an exception. diff --git a/intern/guardedalloc/intern/mallocn.c b/intern/guardedalloc/intern/mallocn.c index 6ed05a81b18..5114850842f 100644 --- a/intern/guardedalloc/intern/mallocn.c +++ b/intern/guardedalloc/intern/mallocn.c @@ -49,6 +49,8 @@ uint (*MEM_get_memory_blocks_in_use)(void) = MEM_lockfree_get_memory_blocks_in_u void (*MEM_reset_peak_memory)(void) = MEM_lockfree_reset_peak_memory; size_t (*MEM_get_peak_memory)(void) = MEM_lockfree_get_peak_memory; +void (*mem_clearmemlist)(void) = mem_lockfree_clearmemlist; + #ifndef NDEBUG const char *(*MEM_name_ptr)(void *vmemh) = MEM_lockfree_name_ptr; void (*MEM_name_ptr_set)(void *vmemh, const char *str) = MEM_lockfree_name_ptr_set; @@ -129,6 +131,8 @@ void MEM_use_lockfree_allocator(void) MEM_reset_peak_memory = MEM_lockfree_reset_peak_memory; MEM_get_peak_memory = MEM_lockfree_get_peak_memory; + mem_clearmemlist = mem_lockfree_clearmemlist; + #ifndef NDEBUG MEM_name_ptr = MEM_lockfree_name_ptr; MEM_name_ptr_set = MEM_lockfree_name_ptr_set; @@ -161,6 +165,8 @@ void MEM_use_guarded_allocator(void) MEM_reset_peak_memory = MEM_guarded_reset_peak_memory; MEM_get_peak_memory = MEM_guarded_get_peak_memory; + mem_clearmemlist = mem_guarded_clearmemlist; + #ifndef NDEBUG MEM_name_ptr = MEM_guarded_name_ptr; MEM_name_ptr_set = MEM_guarded_name_ptr_set; diff --git a/intern/guardedalloc/intern/mallocn_guarded_impl.c b/intern/guardedalloc/intern/mallocn_guarded_impl.c index 35fe07e22a7..e92ddecf252 100644 --- a/intern/guardedalloc/intern/mallocn_guarded_impl.c +++ b/intern/guardedalloc/intern/mallocn_guarded_impl.c @@ -849,6 +849,10 @@ void MEM_guarded_printmemlist_pydict(void) { MEM_guarded_printmemlist_internal(1); } +void mem_guarded_clearmemlist(void) +{ + membase->first = membase->last = NULL; +} void MEM_guarded_freeN(void *vmemh) { diff --git a/intern/guardedalloc/intern/mallocn_intern.h b/intern/guardedalloc/intern/mallocn_intern.h index 382fef243d6..3f048a558a8 100644 --- a/intern/guardedalloc/intern/mallocn_intern.h +++ b/intern/guardedalloc/intern/mallocn_intern.h @@ -103,6 +103,16 @@ size_t memory_usage_current(void); size_t memory_usage_peak(void); void memory_usage_peak_reset(void); +/** + * Clear the listbase of allocated memory blocks. + * + * WARNING: This will make the whole guardedalloc system fully inconsistent. It is only intented to + * be called in one place: the destructor of the #MemLeakPrinter class, which is only + * instantiated once as a static variable by #MEM_init_memleak_detection, and therefore destructed + * once at program exit. + */ +extern void (*mem_clearmemlist)(void); + /* Prototypes for counted allocator functions */ size_t MEM_lockfree_allocN_len(const void *vmemh) ATTR_WARN_UNUSED_RESULT; void MEM_lockfree_freeN(void *vmemh); @@ -142,6 +152,9 @@ size_t MEM_lockfree_get_memory_in_use(void); unsigned int MEM_lockfree_get_memory_blocks_in_use(void); void MEM_lockfree_reset_peak_memory(void); size_t MEM_lockfree_get_peak_memory(void) ATTR_WARN_UNUSED_RESULT; + +void mem_lockfree_clearmemlist(void); + #ifndef NDEBUG const char *MEM_lockfree_name_ptr(void *vmemh); void MEM_lockfree_name_ptr_set(void *vmemh, const char *str); @@ -186,6 +199,9 @@ size_t MEM_guarded_get_memory_in_use(void); unsigned int MEM_guarded_get_memory_blocks_in_use(void); void MEM_guarded_reset_peak_memory(void); size_t MEM_guarded_get_peak_memory(void) ATTR_WARN_UNUSED_RESULT; + +void mem_guarded_clearmemlist(void); + #ifndef NDEBUG const char *MEM_guarded_name_ptr(void *vmemh); void MEM_guarded_name_ptr_set(void *vmemh, const char *str); diff --git a/intern/guardedalloc/intern/mallocn_lockfree_impl.c b/intern/guardedalloc/intern/mallocn_lockfree_impl.c index e21030b6a07..bfc6db8292f 100644 --- a/intern/guardedalloc/intern/mallocn_lockfree_impl.c +++ b/intern/guardedalloc/intern/mallocn_lockfree_impl.c @@ -338,6 +338,8 @@ void MEM_lockfree_printmemlist_pydict(void) {} void MEM_lockfree_printmemlist(void) {} +void mem_lockfree_clearmemlist(void) {} + /* unused */ void MEM_lockfree_callbackmemlist(void (*func)(void *)) {