On 14/03/2021 17:54, Andrzej Hunt wrote:
I was wrong when it comes to LSAN being broken. What was actually
happening is: we default to running ASAN and LSAN with abort_on_error=1,
and I had overridden that setting when running with ASAN. When I
switched to LSAN, abort_on_error was enabled again - and I was just
misinterpreting the intentional abortion as opposed to seeing an
unexplained crashes. [As far as I can tell, abort_on_error is needed to
detect leaks during a test_must_fail and similar scenarios.]
Actually I might have been unintentionally right the first time round:
running clang+LSAN against t0000 (with pipe-related fixes applied)
results in reports as follows, which are effectively useless:
Direct leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x44fc98 in malloc
/home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/lsan/lsan_interceptors.cpp:56:3
#1 0x7fcdbb3dc749 in strdup (/lib64/libc.so.6+0x8b749)
Switching back to ASAN shows me the real stack instead:
Direct leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup
/home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ab1b8 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14
... rest of stack here ...
To make it trickier, that new "leak" happens inside a test_must_fail -
the LSAN output is swallowed, making it hard to diagnose. I'll try to
prepare a separate patch to not discard stderr in that scenario.
The issue here is that:
- test_must_fail does actually keep git's stderr on stderr (there is
some redirection within test_must_fail itself - but eventually
what was git's stderr becomes test_must_fail's stderr).
- many uses of test_must_fail redirect stderr into a temporary file.
- when git fails as expected, test_must_fail succeeds, and the
next command in the test validates that temporary file (that
validation step prints that error output on failure).
- however if git aborted, or didn't fail as expected: test_must_fail
returns an error code. git's error messages remain in the temporary
file and aren't printed.
My potentially naive idea is that within test_must_fail, git's output
should be sent to both test_must_fail's stderr, and also to fd-4 (which
seems to be where verbose output should be sent).
However hat might also result in errors being printed twice (I believe
that will happen for test_must_fail uses that don't redirect stderr into
a temporary file).
