gc: Support valgrind tracking of Python heap allocations

[projectgus]

Extends Valgrind memcheck support in the unix port to track the Python heap, allowing Valgrind to report use-after-free and (in some cases) out of bounds reads and writes when accessing Python heap memory.

Valgrind support is implemented in a dedicated header, to try and keep gc.c relatively clean.

Also adds an additional valgrind build configuration: MICROPY_DEBUG_VALGRIND_MAX_ADDRSPACE. When set, the heap will never reuse the address ranges of freed memory - new allocations are always at a higher address than any previous allocation. Assuming there's enough total heap, this means that use after free errors can never be masked by the freed address being already reallocated for something else.

This branch was used when reproducing and verifying #14029, and seems like it will be useful for other purposes as well.

TODO

• Add support for valgrind in run-tests.py.
• Check the existing valgrind logic in gc_get_ptr() that was added in gc.c: Ensure a gap of one byte before the finaliser table. #7722 (8407159). I need to verify but this logic appears to prevent the GC marking any "pointer" regions if valgrind is enabled. The side effect seems to be that objects retained on the stack (like the running bytecode!), and root pointers, are being garbage collected prematurely. I think we can implement this differently to still avoid warnings if the GC reads uninitialised stack words, but without incorrect GC behaviour. EDIT: Done, have implemented this in a different way so normal gc scans can continue.

Limitations

• Valgrind supports defining a "redzone" between each allocation so out of bounds read or write is always caught. This branch does not have this, so if an allocation is a block-aligned length and the next sequential block in the heap is also allocated then Valgrind can't detect an out of bounds read or write that spills over into the next block. I couldn't find an unobtrusive way to patch gc.c to add a redzone, but someone might have more luck than me...

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (5114f2c) to head (e273f6d).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #14196   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21200    21214   +14     
=======================================
+ Hits        20860    20874   +14     
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

[stinos]

Looks interesting. I once used MTuner to look at the microPython heap; it's been years but IIRC it was quite interesting and useful as well, also because with the tagging you could highlight which memory is what. Hooking it is done calling functions similar to valgrind's. Hence a request: how about making memory profiling in gc.c generic, simply by name, i.e. keep everything as it is now but instead of VALGRIND_MP_MALLOC call it MP_MEMPROFILER_ALLOC etc, and then at the top of the file use

#if MICROPY_MEMPROFILER_VALGRIND
#include "py/mem_valgrind.h"
#elif MICROPY_MEMPROFILER_SOMETHINGELSE
#include "py/mem_somethingelse.h"
#else
#define MP_MEMPROFILER_ALLOC(...)
...
#endif

Or maybe that should even be a separate include file so that if one wants to use memory tagging for debugging, which essentially means adding MP_MEMPROFILER_TAG(some_piece_of_mem, "tag1") in a bunch of locations, they can do that by including that same file.

[projectgus]

Thanks very interesting, thanks @stinos! I hadn't seen MTuner and I agree it could be a very useful tool for some MicroPython analysis.

how about making memory profiling in gc.c generic, simply by name, i.e. keep everything as it is now but instead of VALGRIND_MP_MALLOC call it MP_MEMPROFILER_ALLOC etc, and then at the top of the file use

I would quibble with calling valgrind's memcheck a profiler, it's more of a runtime correctness checker (i.e. closer to tools like ASan or UBSan than tools like MTuner). This is kind of a philosophical point, but kind of not because there's a bunch of places in this PR where we have to tell valgrind "we're about to access some memory that's supposed to be inaccessible now, but it's not an error" (for example, see around gc.c:868). This concept doesn't really have an equivalent when doing memory profiling.

That said, I do think it's a good idea to make the macros generic if possible. I will take a look and see how well they can be made to fit. I really like the idea of making it easier to integrate multiple different tools with the MicroPython core, while keeping the main source files pretty clean.

[projectgus]

there's a bunch of places in this PR where we have to tell valgrind "we're about to access some memory that's supposed to be inaccessible now, but it's not an error" (for example, see around gc.c:868)

Actually maybe this is less complex than I thought, can probably collapse down to something like:

• MP_GC_TRACE_INIT()
• MP_GC_TRACE_PRE_ALLOC()
• MP_GC_TRACE_ALLOC()
• MP_GC_TRACE_REALLOC()
• MP_GC_TRACE_FREE()

Will try this soon, thanks!

[stinos]

I would quibble with calling valgrind's memcheck a profiler, it's more of a runtime correctness checker

Sure; I didn't give the naming a lot of thought but indeed GC_TRACE does cover the various checkers/viewers/... in a better way.