[PATCH v3 0/3] malloc: Enable profiling dlmalloc with valgrind
seanga2 at gmail.com
Wed Mar 30 20:13:11 CEST 2022
On 3/23/22 2:04 PM, Sean Anderson wrote:
> This series adds support for running valgrind against U-Boot's internal
> malloc. This allows for much more useful reports to be generated.
> Some example output of valgrind run against u-boot/master with this branch
> applied may be found at . Note that valgrind gives up around acpi. This
> feature still needs a lot of work on suppressions/hints to filter out the noise
>  https://gist.githubusercontent.com/Forty-Bot/199bf06f9cdd6871e54f8f484c16e111/raw/2a2f99108eef84b48e27a54332f3f71f4e2e5342/gistfile1.txt
This is a bit of a follow up to the above. Over the past week I've sent
a few patches [1-4] with various bugs found with valgrind. Hopefully
this gives a good idea of the kind of problems valgrind is equipped to
find. In particular, it is good at determining where we go past the end
of buffers or otherwise read uninitialized memory.
I've also discovered why I originally stopped working on this series:
supressions don't "un-taint" uninitialized memory accesses. Currently, I
have dlmalloc's reads its bookkeeping information marked as a "red
zone." This means that all reads to it are marked as illegal by
valgrind. This is fine for regular code, but dlmalloc really does need
to access this area, so we suppress its violations. However, if dlmalloc
then passes a result calculated from a "tainted" access, that result is
still tainted. So the first accessor will raise a warning. This means
that every construct like
foo = malloc(sizeof(*foo));
will raise a warning when we check the result of malloc. Whoops.
There are three ways (as I see it) to address this:
- Don't mark dlmalloc bookkeeping information as a red zone. This is the
simplest solution, but reduces the power of valgrind immensely, since
we can no longer determine that (e.g.) access past the end of an array
- Implement red zones properly. This would involve growing every
allocation by a fixed amount (16 bytes or so) and then using that
extra space for a real red zone that neither regular code nor dlmalloc
needs to access. Unfortunately, this would probably some fairly
intensive surgery to dlmalloc to add/remove the offset appropriately.
- Mark bookkeeping information as valid before we use it in dlmalloc,
and then mark it invalid before returning. This would be the most
correct, but it would be very tricky to implement since there are so
many code paths to mark. I think it would be the most effort out of
the three options here.
Until one of the above options are implemented, it will remain difficult
to sift through the massive amount of spurious warnings.
That said, if anyone wants to play around with this a bit, I have some
additional invocation suggestions which I have been using, but which are
- "--error-limit=no" will enable printing more than 1000 errors in a
- "--vgdb=yes --vgdb-error=0" will let you use gdb to attach like
gdb -ex "target remote | vgdb" u-boot
This is very helpful for inspecting the program state when there is
- "t cooked" to U-Boot will keep the console in a sane state if you
terminate it early (instead of having to run tset).
The above should probably be documented in patch 3. I will add it if I
do a v4 (or I will send a follow-up).
More information about the U-Boot