[PATCH] doc: sandbox: Add additional valgrind documentation

[Sean Anderson]

This document some additional options which can be used with valgrind, as
well as directions for future work. It also fixes up inline literals to
actually be inline literals (and not italics). The content of this
documentation is primarily adapted from [1].

[1] https://lore.kernel.org/u-boot/57cb4b49-fa30-1194-9ac3-faa53e8033bd@gmail.com/

Signed-off-by: Sean Anderson <seanga2 at gmail.com>
---

 doc/arch/sandbox.rst | 65 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst
index e1119492b4..cd5090be71 100644
--- a/doc/arch/sandbox.rst
+++ b/doc/arch/sandbox.rst
@@ -479,19 +479,76 @@ It is possible to run U-Boot under valgrind to check memory allocations::
 
     valgrind ./u-boot
 
-For more detailed results, enable `CONFIG_VALGRIND`. There are many false
-positives due to `malloc` itself. Suppress these with::
+For more detailed results, enable ``CONFIG_VALGRIND``. There are many false
+positives due to ``malloc`` itself. Suppress these with::
 
     valgrind --suppressions=scripts/u-boot.supp ./u-boot
 
 If you are running sandbox SPL or TPL, then valgrind will not by default
 notice when U-Boot jumps from TPL to SPL, or from SPL to U-Boot proper. To
-fix this, use `--trace-children=yes`. To show who alloc'd some troublesome
-memory, use `--track-origins=yes`. To uncover possible errors, try running all
+fix this, use ``--trace-children=yes``. To show who alloc'd some troublesome
+memory, use ``--track-origins=yes``. To uncover possible errors, try running all
 unit tests with::
 
     valgrind --track-origins=yes --suppressions=scripts/u-boot.supp ./u-boot -Tc 'ut all'
 
+Additional options
+^^^^^^^^^^^^^^^^^^
+
+The following options are useful in addition to the above examples:
+
+* ``--error-limit=no`` will enable printing more than 1000 errors in a
+  single session.
+* ``--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
+  an error.
+* Passing ``-t cooked`` to U-Boot will keep the console in a sane state if you
+  terminate it early (instead of having to run tset).
+
+Future work
+^^^^^^^^^^^
+
+The biggest limitation to the current approach is that
+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
+
+.. code-block::
+
+    foo = malloc(sizeof(*foo));
+    if (!foo)
+        return -ENOMEM;
+
+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
+  is undefined.
+* 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.
+
 Testing
 -------
 
-- 
2.35.1