From cb77d1f30882b2662dc097c802af95e721604ae0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 30 Jun 2026 02:41:59 -0400 Subject: [PATCH 1/2] t: move LSan errors from stdout to stderr When we find LSan errors, we dump them via "say_color", which goes to stdout. This is mostly harmless, since stdout and stderr tend to go to the same place (either the user's terminal, or to the ".out" file with --verbose-log). But when running under a TAP harness like prove, they are split and stdout is interpreted as TAP output. Historically even this was fine, as the extra lines on stdout would be ignored. But since 389c83025d (t: let prove fail when parsing invalid TAP output, 2026-06-04) we instruct the TAP reader to complain, and a leaking test will result in complaints like this (this is a real leak which we have yet to fix): $ GIT_TEST_COMMIT_GRAPH=1 make SANITIZE=leak test [...] Test Summary Report ------------------- t4014-format-patch.sh (Wstat: 256 (exited 1) Tests: 226 Failed: 30) Failed tests: 197-226 Non-zero exit status: 1 Parse errors: Unknown TAP token: "" Unknown TAP token: "=================================================================" Unknown TAP token: "==git==3693658==ERROR: LeakSanitizer: detected memory leaks" Unknown TAP token: "" Unknown TAP token: "Direct leak of 200 byte(s) in 1 object(s) allocated from:" Displayed the first 5 of 1531 TAP syntax errors. Re-run prove with the -p option to see them all. You still see the failing tests, so it's mostly just an annoyance. We can fix it by redirecting to stderr (actually descriptor 4, which is our verbose-respecting variant). I confirmed manually that the output still appears with --verbose-log, and even with a single-test "-i --verbose-only=197" going to the terminal. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/test-lib.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 70fd3e9baf..63db941bb7 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1190,14 +1190,14 @@ check_test_results_san_file_ () { then return fi && - say_color error "$(cat "$TEST_RESULTS_SAN_FILE".*)" && + say_color >&4 error "$(cat "$TEST_RESULTS_SAN_FILE".*)" && if test "$test_failure" = 0 then - say "Our logs revealed a memory leak, exit non-zero!" && + say >&4 "Our logs revealed a memory leak, exit non-zero!" && invert_exit_code=t else - say "Our logs revealed a memory leak..." + say >&4 "Our logs revealed a memory leak..." fi } From 973a0373ffd1b3f1f149bfac88c0bef3e6a0afd0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 30 Jun 2026 02:43:01 -0400 Subject: [PATCH 2/2] format-patch: fix leak of rev_info in prepare_bases() In prepare_bases() we do a custom revision walk, separate from the main format-patch walk. After we finish, we fail to call release_revisions(), possibly leaking its contents. We failed to notice it so far because the revision machinery doesn't always allocate. But at least one case can trigger the leak: if a commit graph is present, then the topo-walk allocates revs.topo_walk_info and some associated data structures. You can see it in the test suite by running: make SANITIZE=leak cd t GIT_TEST_COMMIT_GRAPH=1 ./t4014-format-patch.sh which yields many entries like: ==git==3687620==ERROR: LeakSanitizer: detected memory leaks Direct leak of 200 byte(s) in 1 object(s) allocated from: #0 0x7f4ccba185cb in malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:74 #1 0x55cd452cdd0b in do_xmalloc wrapper.c:55 #2 0x55cd452cdd9d in xmalloc wrapper.c:76 #3 0x55cd45255473 in init_topo_walk revision.c:3845 #4 0x55cd45255bef in prepare_revision_walk revision.c:4017 #5 0x55cd44ffec40 in prepare_bases builtin/log.c:1872 #6 0x55cd450010ec in cmd_format_patch builtin/log.c:2439 The un-released rev_info has been there since the code was added in fa2ab86d18 (format-patch: add '--base' option to record base tree info, 2016-04-26), but back then we didn't even have a way to release rev_info resources! The actual leak probably started around f0d9cc4196 (revision.c: begin refactoring --topo-order logic, 2018-11-01), but it's hard to bisect because there were so many other unrelated leaks back then. So I'm not sure exactly when the leak started beyond "long ago", but it is easy-ish to find now (since we've plugged all those other leaks) and the solution is clear. I didn't add a new test since we can demonstrate it with the existing ones, but it does require tweaking a test variable. We might consider ways to get more automatic leak-checking coverage there, but I think it should be done outside of this fix. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/log.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/log.c b/builtin/log.c index 8c0939dd42..baf98b75e1 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1884,6 +1884,7 @@ static void prepare_bases(struct base_tree_info *bases, bases->nr_patch_id++; } clear_commit_base(&commit_base); + release_revisions(&revs); } static void print_bases(struct base_tree_info *bases, FILE *file)