diff --git a/py/gc.c b/py/gc.c index 8a03ce5264..ff34188885 100644 --- a/py/gc.c +++ b/py/gc.c @@ -26,16 +26,14 @@ */ #include +#include +#include #include #include #include "py/gc.h" #include "py/runtime.h" -#if MICROPY_DEBUG_VALGRIND -#include -#endif - #if MICROPY_ENABLE_GC #if MICROPY_DEBUG_VERBOSE // print debugging info @@ -120,6 +118,8 @@ #define GC_EXIT() #endif +#include "py/gc_valgrind.h" + // TODO waste less memory; currently requires that all entries in alloc_table have a corresponding block in pool static void gc_setup_area(mp_state_mem_area_t *area, void *start, void *end) { // calculate parameters for GC (T=total, A=alloc table, F=finaliser table, P=pool; all in bytes): @@ -170,6 +170,12 @@ static void gc_setup_area(mp_state_mem_area_t *area, void *start, void *end) { area->next = NULL; #endif + // Valgrind: Assume 'area' came to us from malloc, so resize it such that it + // doesn't cover the 'pool' area + // + // This frees up the area between gc_pool_start and gc_pool_end to have its allocations tracked. + VALGRIND_RESIZEINPLACE_BLOCK(start, end - start, area->gc_pool_start - (uint8_t *)start, 0); + DEBUG_printf("GC layout:\n"); DEBUG_printf(" alloc table at %p, length " UINT_FMT " bytes, " UINT_FMT " blocks\n", @@ -404,9 +410,15 @@ static void gc_mark_subtree(size_t block) // check that the consecutive blocks didn't overflow past the end of the area assert(area->gc_pool_start + (block + n_blocks) * BYTES_PER_BLOCK <= area->gc_pool_end); + size_t n_bytes = n_blocks * BYTES_PER_BLOCK; + #if MICROPY_DEBUG_VALGRIND + // Only search the real allocation size so valgrind doesn't complain + n_bytes = valgrind_get_alloc_sz((void *)PTR_FROM_BLOCK(area, block), n_blocks); + #endif + // check this block's children void **ptrs = (void **)PTR_FROM_BLOCK(area, block); - for (size_t i = n_blocks * BYTES_PER_BLOCK / sizeof(void *); i > 0; i--, ptrs++) { + for (size_t i = n_bytes / sizeof(void *); i > 0; i--, ptrs++) { MICROPY_GC_HOOK_LOOP(i); void *ptr = *ptrs; // If this is a heap pointer that hasn't been marked, mark it and push @@ -525,6 +537,7 @@ static void gc_sweep(void) { #if MICROPY_PY_GC_COLLECT_RETVAL MP_STATE_MEM(gc_collected)++; #endif + VALGRIND_MP_FREE(PTR_FROM_BLOCK(area, block)); // fall through to free the head MP_FALLTHROUGH @@ -592,15 +605,15 @@ void gc_collect_start(void) { __attribute__((no_sanitize_address)) #endif static void *gc_get_ptr(void **ptrs, int i) { - #if MICROPY_DEBUG_VALGRIND - if (!VALGRIND_CHECK_MEM_IS_ADDRESSABLE(&ptrs[i], sizeof(*ptrs))) { - return NULL; - } - #endif return ptrs[i]; } void gc_collect_root(void **ptrs, size_t len) { + #if MICROPY_DEBUG_VALGRIND + // ptrs may include undefined words on the stack, tell valgrind this is OK + VALGRIND_MAKE_MEM_DEFINED_IF_ADDRESSABLE(ptrs, len * sizeof(*ptrs)); + #endif + #if !MICROPY_GC_SPLIT_HEAP mp_state_mem_area_t *area = &MP_STATE_MEM(area); #endif @@ -848,18 +861,29 @@ found: GC_EXIT(); + // The number of bytes allocated from the heap + size_t block_byte_len = (end_block - start_block + 1) * BYTES_PER_BLOCK; + + // Valgrind: Mark the whole block as accessible so that gc can zero bytes if needed, + // without registering this as an allocation + VALGRIND_MAKE_MEM_UNDEFINED(ret_ptr, block_byte_len); + #if MICROPY_GC_CONSERVATIVE_CLEAR // be conservative and zero out all the newly allocated blocks - memset((byte *)ret_ptr, 0, (end_block - start_block + 1) * BYTES_PER_BLOCK); + memset((byte *)ret_ptr, 0, block_byte_len); #else // zero out the additional bytes of the newly allocated blocks // This is needed because the blocks may have previously held pointers // to the heap and will not be set to something else if the caller // doesn't actually use the entire block. As such they will continue // to point to the heap and may prevent other blocks from being reclaimed. - memset((byte *)ret_ptr + n_bytes, 0, (end_block - start_block + 1) * BYTES_PER_BLOCK - n_bytes); + memset((byte *)ret_ptr + n_bytes, 0, block_byte_len - n_bytes); #endif + // Valgrind: Mark the region as no-access again, then track the real allocation + VALGRIND_MAKE_MEM_NOACCESS(ret_ptr, block_byte_len); + VALGRIND_MP_MALLOC(ret_ptr, n_bytes); + #if MICROPY_ENABLE_FINALISER if (has_finaliser) { // clear type pointer in case it is never set @@ -953,6 +977,8 @@ void gc_free(void *ptr) { block += 1; } while (ATB_GET_KIND(area, block) == AT_TAIL); + VALGRIND_MP_FREE(ptr); + GC_EXIT(); #if EXTENSIVE_HEAP_PROFILING @@ -1084,7 +1110,12 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) { // return original ptr if it already has the requested number of blocks if (new_blocks == n_blocks) { + VALGRIND_MP_RESIZE_BLOCK(ptr, n_blocks, n_bytes); + GC_EXIT(); + + DEBUG_printf("gc_realloc(%p -> %p, %d bytes -> %d blocks)\n", ptr_in, ptr_in, n_bytes, new_blocks); + return ptr_in; } @@ -1107,6 +1138,8 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) { area->gc_last_free_atb_index = (block + new_blocks) / BLOCKS_PER_ATB; } + VALGRIND_MP_RESIZE_BLOCK(ptr, n_blocks, n_bytes); + GC_EXIT(); #if EXTENSIVE_HEAP_PROFILING @@ -1127,6 +1160,9 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) { area->gc_last_used_block = MAX(area->gc_last_used_block, end_block); + // Valgrind: grow allocation to full block size, as we're about to zero all blocks + VALGRIND_MP_RESIZE_BLOCK(ptr, n_blocks, new_blocks * BYTES_PER_BLOCK); + GC_EXIT(); #if MICROPY_GC_CONSERVATIVE_CLEAR @@ -1137,6 +1173,9 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) { memset((byte *)ptr_in + n_bytes, 0, new_blocks * BYTES_PER_BLOCK - n_bytes); #endif + // Valgrind: Shrink the allocation back to the real size + VALGRIND_MP_RESIZE_BLOCK(ptr, new_blocks, n_bytes); + #if EXTENSIVE_HEAP_PROFILING gc_dump_alloc_table(&mp_plat_print); #endif @@ -1165,7 +1204,11 @@ void *gc_realloc(void *ptr_in, size_t n_bytes, bool allow_move) { return NULL; } - DEBUG_printf("gc_realloc(%p -> %p)\n", ptr_in, ptr_out); + DEBUG_printf("gc_realloc(%p -> %p, %d bytes -> %d blocks)\n", ptr_in, ptr_out, n_bytes, new_blocks); + + // Valgrind: memcpy copies all blocks, so grow the previous allocation to match + VALGRIND_MP_RESIZE_BLOCK(ptr_in, n_blocks, n_blocks * BYTES_PER_BLOCK); + memcpy(ptr_out, ptr_in, n_blocks * BYTES_PER_BLOCK); gc_free(ptr_in); return ptr_out; diff --git a/py/gc_valgrind.h b/py/gc_valgrind.h new file mode 100644 index 0000000000..6cd7810e33 --- /dev/null +++ b/py/gc_valgrind.h @@ -0,0 +1,75 @@ +/* + * This file is part of the MicroPython project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2024 Angus Gratton + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef MICROPY_INCLUDED_PY_GC_VALGRIND_H +#define MICROPY_INCLUDED_PY_GC_VALGRIND_H + +// This header is intended for including into gc.c directly. +// +// Defining some helper macros here helps keep valgrind integration in gc.c +// as unobtrusive as possible. + +#include "py/mpconfig.h" + +#if MICROPY_DEBUG_VALGRIND + +#include + +// MicroPython heap only knows size of an allocation in blocks, +// this function queries valgrind (if enabled) to tell us the size +// in bytes. +static size_t valgrind_get_alloc_sz(void *p, size_t num_blocks) { + size_t max_bytes = num_blocks * BYTES_PER_BLOCK; + VALGRIND_DISABLE_ERROR_REPORTING; // Checking reports an error otherwise + uintptr_t first_invalid = VALGRIND_CHECK_MEM_IS_ADDRESSABLE(p, max_bytes); + VALGRIND_ENABLE_ERROR_REPORTING; + return first_invalid ? (first_invalid - (uintptr_t)p) : max_bytes; +} + +// Note: Currently we tell valgrind that the memory is zeroed if MICROPY_GC_CONSERVATIVE_CLEAR +// is set. Running with this unset results in a lot of valgrind errors! +#define VALGRIND_MP_MALLOC(PTR, LEN_BYTES) \ + VALGRIND_MALLOCLIKE_BLOCK((PTR), (LEN_BYTES), 0, MICROPY_GC_CONSERVATIVE_CLEAR); + +// Tell valgrind the block at PTR was OLD_NUM_BLOCKS in length, now NEW_LEN_BYTES in length +#define VALGRIND_MP_RESIZE_BLOCK(PTR, OLD_NUM_BLOCKS, NEW_LEN_BYTES) \ + VALGRIND_RESIZEINPLACE_BLOCK((PTR), valgrind_get_alloc_sz((PTR), (OLD_NUM_BLOCKS)), NEW_LEN_BYTES, 0) + +#define VALGRIND_MP_FREE(PTR) VALGRIND_FREELIKE_BLOCK((PTR), 0) + +#else // MICROPY_DEBUG_VALGRIND + +// No-op definitions +#define VALGRIND_MP_MALLOC(...) +#define VALGRIND_MP_RESIZE_BLOCK(...) +#define VALGRIND_MP_FREE(...) + +#define VALGRIND_RESIZEINPLACE_BLOCK(...) +#define VALGRIND_MAKE_MEM_UNDEFINED(...) +#define VALGRIND_MAKE_MEM_NOACCESS(...) + +#endif // MICROPY_DEBUG_VALGRIND +#endif // MICROPY_INCLUDED_PY_GC_VALGRIND_H