Valgrind: suppress warnings with MemPool & MEM_* trailing padding

Suppress false positive Valgrind warnings which flooded the output.

- BLI_mempool alloc/free & iteration.
- Set alignment padding bytes at the end of MEM_* allocations
  as "defined" since this causes many false positive warnings
  in blend file writing and MEMFILE comparisons.
- Set MEM_* allocations as undefined when `--debug-memory`
  is passed in to account for debug initialization.
- Initialize pad bytes in TextLine allocations.
This commit is contained in:
Campbell Barton
2023-12-05 16:31:58 +11:00
parent d48b9d32c6
commit 20fd012adb
5 changed files with 164 additions and 27 deletions

View File

@@ -6,6 +6,10 @@ if(HAVE_MALLOC_STATS_H)
add_definitions(-DHAVE_MALLOC_STATS_H)
endif()
if(WITH_MEM_VALGRIND)
add_definitions(-DWITH_MEM_VALGRIND)
endif()
set(INC
PUBLIC .
)

View File

@@ -19,6 +19,12 @@
#include "MEM_guardedalloc.h"
/* Quiet warnings when dealing with allocated data written into the blend file.
* This also rounds up and causes warnings which we don't consider bugs in practice. */
#ifdef WITH_MEM_VALGRIND
# include "valgrind/memcheck.h"
#endif
/* to ensure strict conversions */
#include "../../source/blender/blenlib/BLI_strict_flags.h"
@@ -445,14 +451,28 @@ void *MEM_guarded_mallocN(size_t len, const char *str)
{
MemHead *memh;
#ifdef WITH_MEM_VALGRIND
const size_t len_unaligned = len;
#endif
len = SIZET_ALIGN_4(len);
memh = (MemHead *)malloc(len + sizeof(MemHead) + sizeof(MemTail));
if (LIKELY(memh)) {
make_memhead_header(memh, len, str);
if (UNLIKELY(malloc_debug_memset && len)) {
memset(memh + 1, 255, len);
if (LIKELY(len)) {
if (UNLIKELY(malloc_debug_memset)) {
memset(memh + 1, 255, len);
}
#ifdef WITH_MEM_VALGRIND
if (malloc_debug_memset) {
VALGRIND_MAKE_MEM_UNDEFINED(memh + 1, len_unaligned);
}
else {
VALGRIND_MAKE_MEM_DEFINED((const char *)(memh + 1) + len_unaligned, len - len_unaligned);
}
#endif /* WITH_MEM_VALGRIND */
}
#ifdef DEBUG_MEMCOUNTER
@@ -510,6 +530,9 @@ void *MEM_guarded_mallocN_aligned(size_t len, size_t alignment, const char *str)
*/
assert(alignment < 1024);
#ifdef WITH_MEM_VALGRIND
const size_t len_unaligned = len;
#endif
len = SIZET_ALIGN_4(len);
MemHead *memh = (MemHead *)aligned_malloc(
@@ -524,8 +547,18 @@ void *MEM_guarded_mallocN_aligned(size_t len, size_t alignment, const char *str)
make_memhead_header(memh, len, str);
memh->alignment = (short)alignment;
if (UNLIKELY(malloc_debug_memset && len)) {
memset(memh + 1, 255, len);
if (LIKELY(len)) {
if (UNLIKELY(malloc_debug_memset)) {
memset(memh + 1, 255, len);
}
#ifdef WITH_MEM_VALGRIND
if (malloc_debug_memset) {
VALGRIND_MAKE_MEM_UNDEFINED(memh + 1, len_unaligned);
}
else {
VALGRIND_MAKE_MEM_DEFINED((const char *)(memh + 1) + len_unaligned, len - len_unaligned);
}
#endif /* WITH_MEM_VALGRIND */
}
#ifdef DEBUG_MEMCOUNTER

View File

@@ -16,6 +16,12 @@
#include "MEM_guardedalloc.h"
/* Quiet warnings when dealing with allocated data written into the blend file.
* This also rounds up and causes warnings which we don't consider bugs in practice. */
#ifdef WITH_MEM_VALGRIND
# include "valgrind/memcheck.h"
#endif
/* to ensure strict conversions */
#include "../../source/blender/blenlib/BLI_strict_flags.h"
@@ -244,13 +250,27 @@ void *MEM_lockfree_mallocN(size_t len, const char *str)
{
MemHead *memh;
#ifdef WITH_MEM_VALGRIND
const size_t len_unaligned = len;
#endif
len = SIZET_ALIGN_4(len);
memh = (MemHead *)malloc(len + sizeof(MemHead));
if (LIKELY(memh)) {
if (UNLIKELY(malloc_debug_memset && len)) {
memset(memh + 1, 255, len);
if (LIKELY(len)) {
if (UNLIKELY(malloc_debug_memset)) {
memset(memh + 1, 255, len);
}
#ifdef WITH_MEM_VALGRIND
if (malloc_debug_memset) {
VALGRIND_MAKE_MEM_UNDEFINED(memh + 1, len_unaligned);
}
else {
VALGRIND_MAKE_MEM_DEFINED((const char *)(memh + 1) + len_unaligned, len - len_unaligned);
}
#endif /* WITH_MEM_VALGRIND */
}
memh->len = len;
@@ -305,6 +325,9 @@ void *MEM_lockfree_mallocN_aligned(size_t len, size_t alignment, const char *str
*/
size_t extra_padding = MEMHEAD_ALIGN_PADDING(alignment);
#ifdef WITH_MEM_VALGRIND
const size_t len_unaligned = len;
#endif
len = SIZET_ALIGN_4(len);
MemHeadAligned *memh = (MemHeadAligned *)aligned_malloc(
@@ -317,8 +340,18 @@ void *MEM_lockfree_mallocN_aligned(size_t len, size_t alignment, const char *str
*/
memh = (MemHeadAligned *)((char *)memh + extra_padding);
if (UNLIKELY(malloc_debug_memset && len)) {
memset(memh + 1, 255, len);
if (LIKELY(len)) {
if (UNLIKELY(malloc_debug_memset)) {
memset(memh + 1, 255, len);
}
#ifdef WITH_MEM_VALGRIND
if (malloc_debug_memset) {
VALGRIND_MAKE_MEM_UNDEFINED(memh + 1, len_unaligned);
}
else {
VALGRIND_MAKE_MEM_DEFINED((const char *)(memh + 1) + len_unaligned, len - len_unaligned);
}
#endif /* WITH_MEM_VALGRIND */
}
memh->len = len | (size_t)MEMHEAD_ALIGN_FLAG;

View File

@@ -57,6 +57,8 @@ static void txt_delete_line(Text *text, TextLine *line);
static void txt_delete_sel(Text *text);
static void txt_make_dirty(Text *text);
static TextLine *txt_line_malloc() ATTR_MALLOC ATTR_WARN_UNUSED_RESULT;
/** \} */
/* -------------------------------------------------------------------- */
@@ -66,7 +68,6 @@ static void txt_make_dirty(Text *text);
static void text_init_data(ID *id)
{
Text *text = (Text *)id;
TextLine *tmp;
BLI_assert(MEMCMP_STRUCT_AFTER_IS_ZERO(text, id));
@@ -79,7 +80,7 @@ static void text_init_data(ID *id)
BLI_listbase_clear(&text->lines);
tmp = (TextLine *)MEM_mallocN(sizeof(TextLine), "textline");
TextLine *tmp = txt_line_malloc();
tmp->line = (char *)MEM_mallocN(1, "textline_string");
tmp->format = nullptr;
@@ -125,7 +126,7 @@ static void text_copy_data(Main * /*bmain*/, ID *id_dst, const ID *id_src, const
/* Walk down, reconstructing. */
LISTBASE_FOREACH (TextLine *, line_src, &text_src->lines) {
TextLine *line_dst = static_cast<TextLine *>(MEM_mallocN(sizeof(*line_dst), __func__));
TextLine *line_dst = txt_line_malloc();
line_dst->line = BLI_strdupn(line_src->line, line_src->len);
line_dst->len = line_src->len;
@@ -362,9 +363,7 @@ static void text_from_buf(Text *text, const uchar *buffer, const int len)
lines_count = 0;
for (i = 0; i < len; i++) {
if (buffer[i] == '\n') {
TextLine *tmp;
tmp = (TextLine *)MEM_mallocN(sizeof(TextLine), "textline");
TextLine *tmp = txt_line_malloc();
tmp->line = (char *)MEM_mallocN(llen + 1, "textline_string");
tmp->format = nullptr;
@@ -392,9 +391,7 @@ static void text_from_buf(Text *text, const uchar *buffer, const int len)
* - last character in buffer is \n. in this case new line is needed to
* deal with newline at end of file. (see #28087) (sergey) */
if (llen != 0 || lines_count == 0 || buffer[len - 1] == '\n') {
TextLine *tmp;
tmp = (TextLine *)MEM_mallocN(sizeof(TextLine), "textline");
TextLine *tmp = txt_line_malloc();
tmp->line = (char *)MEM_mallocN(llen + 1, "textline_string");
tmp->format = nullptr;
@@ -591,6 +588,14 @@ void BKE_text_file_modified_ignore(Text *text)
/** \name Editing Utility Functions
* \{ */
static TextLine *txt_line_malloc()
{
TextLine *l = static_cast<TextLine *>(MEM_mallocN(sizeof(TextLine), "textline"));
/* Quiet VALGRIND warning, may avoid unintended differences with MEMFILE undo as well. */
memset(l->_pad0, 0, sizeof(l->_pad0));
return l;
}
static void make_new_line(TextLine *line, char *newline)
{
if (line->line) {
@@ -607,9 +612,7 @@ static void make_new_line(TextLine *line, char *newline)
static TextLine *txt_new_linen(const char *str, int str_len)
{
TextLine *tmp;
tmp = (TextLine *)MEM_mallocN(sizeof(TextLine), "textline");
TextLine *tmp = txt_line_malloc();
tmp->line = static_cast<char *>(MEM_mallocN(str_len + 1, "textline_string"));
tmp->format = nullptr;
@@ -1406,7 +1409,7 @@ void txt_from_buf_for_undo(Text *text, const char *buf, size_t buf_len)
const char *buf_step_next = strchr(buf_step, '\n');
const int len = buf_step_next - buf_step;
TextLine *l = static_cast<TextLine *>(MEM_mallocN(sizeof(TextLine), "textline"));
TextLine *l = txt_line_malloc();
l->line = static_cast<char *>(MEM_mallocN(len + 1, "textline_string"));
l->len = len;
l->format = nullptr;
@@ -1681,7 +1684,7 @@ void txt_split_curline(Text *text)
/* Make the new TextLine */
ins = static_cast<TextLine *>(MEM_mallocN(sizeof(TextLine), "textline"));
ins = txt_line_malloc();
ins->line = left;
ins->format = nullptr;
ins->len = text->curc;

View File

@@ -242,12 +242,16 @@ static BLI_freenode *mempool_chunk_add(BLI_mempool *pool,
BLI_freenode *next;
BLI_asan_unpoison(curnode, pool->esize - POISON_REDZONE_SIZE);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(curnode, pool->esize - POISON_REDZONE_SIZE);
#endif
curnode->next = next = NODE_STEP_NEXT(curnode);
curnode->freeword = FREEWORD;
BLI_asan_poison(curnode, pool->esize);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(curnode, pool->esize);
#endif
curnode = next;
}
}
@@ -256,8 +260,14 @@ static BLI_freenode *mempool_chunk_add(BLI_mempool *pool,
BLI_freenode *next;
BLI_asan_unpoison(curnode, pool->esize - POISON_REDZONE_SIZE);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(curnode, pool->esize - POISON_REDZONE_SIZE);
#endif
curnode->next = next = NODE_STEP_NEXT(curnode);
BLI_asan_poison(curnode, pool->esize);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(curnode, pool->esize);
#endif
curnode = next;
}
@@ -266,14 +276,27 @@ static BLI_freenode *mempool_chunk_add(BLI_mempool *pool,
/* terminate the list (rewind one)
* will be overwritten if 'curnode' gets passed in again as 'last_tail' */
BLI_asan_unpoison(curnode, pool->esize - POISON_REDZONE_SIZE);
BLI_asan_poison(curnode, pool->esize);
if (POISON_REDZONE_SIZE > 0) {
BLI_asan_unpoison(curnode, pool->esize - POISON_REDZONE_SIZE);
BLI_asan_poison(curnode, pool->esize);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(curnode, pool->esize - POISON_REDZONE_SIZE);
VALGRIND_MAKE_MEM_UNDEFINED(curnode, pool->esize);
#endif
}
curnode = NODE_STEP_PREV(curnode);
BLI_asan_unpoison(curnode, pool->esize - POISON_REDZONE_SIZE);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(curnode, pool->esize - POISON_REDZONE_SIZE);
#endif
curnode->next = NULL;
BLI_asan_poison(curnode, pool->esize);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(curnode, pool->esize);
#endif
#ifdef USE_TOTALLOC
pool->totalloc += pool->pchunk;
@@ -282,8 +305,14 @@ static BLI_freenode *mempool_chunk_add(BLI_mempool *pool,
/* final pointer in the previously allocated chunk is wrong */
if (last_tail) {
BLI_asan_unpoison(last_tail, pool->esize - POISON_REDZONE_SIZE);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(last_tail, pool->esize - POISON_REDZONE_SIZE);
#endif
last_tail->next = CHUNK_DATA(mpchunk);
BLI_asan_poison(last_tail, pool->esize);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(last_tail, pool->esize);
#endif
}
return curnode;
@@ -295,6 +324,9 @@ static void mempool_chunk_free(BLI_mempool_chunk *mpchunk, BLI_mempool *pool)
BLI_asan_unpoison(mpchunk, sizeof(BLI_mempool_chunk) + pool->esize * pool->csize);
#else
UNUSED_VARS(pool);
#endif
#ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(mpchunk, sizeof(BLI_mempool_chunk) + pool->esize * pool->csize);
#endif
MEM_freeN(mpchunk);
}
@@ -394,6 +426,17 @@ void *BLI_mempool_alloc(BLI_mempool *pool)
free_pop = pool->free;
BLI_asan_unpoison(free_pop, pool->esize - POISON_REDZONE_SIZE);
#ifdef WITH_MEM_VALGRIND
VALGRIND_MEMPOOL_ALLOC(pool, free_pop, pool->esize - POISON_REDZONE_SIZE);
/* Mark as define, then undefine immediately before returning so:
* - `free_pop->next` can be read without reading "undefined" memory.
* - `freeword` can be set without causing the memory to be considered "defined".
*
* These could be handled on a more granular level - dealing with defining & underlining these
* members explicitly but that requires more involved calls,
* adding overhead for no real benefit. */
VALGRIND_MAKE_MEM_DEFINED(free_pop, pool->esize - POISON_REDZONE_SIZE);
#endif
BLI_assert(pool->chunk_tail->next == NULL);
@@ -405,7 +448,7 @@ void *BLI_mempool_alloc(BLI_mempool *pool)
pool->totused++;
#ifdef WITH_MEM_VALGRIND
VALGRIND_MEMPOOL_ALLOC(pool, free_pop, pool->esize);
VALGRIND_MAKE_MEM_UNDEFINED(free_pop, pool->esize - POISON_REDZONE_SIZE);
#endif
return (void *)free_pop;
@@ -687,6 +730,9 @@ void *BLI_mempool_iterstep(BLI_mempool_iter *iter)
ret = curnode;
BLI_asan_unpoison(ret, iter->pool->esize - POISON_REDZONE_SIZE);
# ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(ret, iter->pool->esize - POISON_REDZONE_SIZE);
# endif
if (++iter->curindex != iter->pool->pchunk) {
curnode = POINTER_OFFSET(curnode, esize);
@@ -696,10 +742,16 @@ void *BLI_mempool_iterstep(BLI_mempool_iter *iter)
iter->curchunk = iter->curchunk->next;
if (UNLIKELY(iter->curchunk == NULL)) {
BLI_asan_unpoison(ret, iter->pool->esize - POISON_REDZONE_SIZE);
# ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(ret, iter->pool->esize - POISON_REDZONE_SIZE);
# endif
void *ret2 = (ret->freeword == FREEWORD) ? NULL : ret;
if (ret->freeword == FREEWORD) {
BLI_asan_poison(ret, iter->pool->esize);
# ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(ret, iter->pool->esize);
# endif
}
return ret2;
@@ -727,6 +779,9 @@ void *mempool_iter_threadsafe_step(BLI_mempool_threadsafe_iter *ts_iter)
ret = curnode;
BLI_asan_unpoison(ret, esize - POISON_REDZONE_SIZE);
# ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_DEFINED(ret, iter->pool->esize);
# endif
if (++iter->curindex != iter->pool->pchunk) {
curnode = POINTER_OFFSET(curnode, esize);
@@ -746,6 +801,9 @@ void *mempool_iter_threadsafe_step(BLI_mempool_threadsafe_iter *ts_iter)
if (UNLIKELY(iter->curchunk == NULL)) {
if (ret->freeword == FREEWORD) {
BLI_asan_poison(ret, esize);
# ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(ret, iter->pool->esize);
# endif
mempool_asan_unlock(iter->pool);
return NULL;
}
@@ -760,6 +818,9 @@ void *mempool_iter_threadsafe_step(BLI_mempool_threadsafe_iter *ts_iter)
if (UNLIKELY(iter->curchunk == NULL)) {
if (ret->freeword == FREEWORD) {
BLI_asan_poison(ret, iter->pool->esize);
# ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(ret, iter->pool->esize);
# endif
mempool_asan_unlock(iter->pool);
return NULL;
}
@@ -774,6 +835,9 @@ void *mempool_iter_threadsafe_step(BLI_mempool_threadsafe_iter *ts_iter)
if (ret->freeword == FREEWORD) {
BLI_asan_poison(ret, iter->pool->esize);
# ifdef WITH_MEM_VALGRIND
VALGRIND_MAKE_MEM_UNDEFINED(ret, iter->pool->esize);
# endif
}
else {
break;