py/gc: Support valgrind memcheck on the Python heap.

This seems to work quite well, can run individual test cases under
valgrind and (although slow) it shows any memory safety issues.

Replaces the valgrind false-positives fix previously applied in 8407159.
That one effectively disabled gc roots from being marked, so things would
be garbage collected even when there were valid references to them.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>

Signed-off-by: Angus Gratton <angus@redyak.com.au>
pull/14196/head
Angus Gratton 2024-02-08 10:39:29 +11:00
rodzic 6175735cda
commit d70865e406
2 zmienionych plików z 131 dodań i 13 usunięć

69
py/gc.c
Wyświetl plik

@ -26,16 +26,14 @@
*/
#include <assert.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include "py/gc.h"
#include "py/runtime.h"
#if MICROPY_DEBUG_VALGRIND
#include <valgrind/memcheck.h>
#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;

75
py/gc_valgrind.h 100644
Wyświetl plik

@ -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 <valgrind/memcheck.h>
// 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