# [PATCH] merge: fix leak with merge.defaultToUpstream 3 messages from 2026-07-28 to 2026-07-28. Participants: Toon Claes, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/66081 ## Toon Claes, 2026-07-28 13:00 Subject: [PATCH] merge: fix leak with merge.defaultToUpstream Message-ID: <20260728-toon-fix-merge-leak-v1-1-d6bd161a3acf@iotcl.com> ``` By default the setting 'merge.defaultToUpstream' for git-merge(1) is set to 'true', which means when `git merge` is invoked with no arguments it merges the upstream branch configured for the current branch. With this configuration set to 'true', setup_with_upstream() is called. That function allocates an array of arguments and hands it back to cmd_merge() via its `argv` parameter. This array is never freed, so cmd_merge() leaks it on every invocation. Track the allocated array in a separate variable and free it at the end. The leak has been present since 93e535a5b7 (merge: merge with the default upstream branch without argument, 2011-03-24). Although the leak sanitizer was enabled for tests in fc1ddf42af (t: remove TEST_PASSES_SANITIZE_LEAK annotations, 2024-11-21), it went unnoticed because no test calls `git merge` without arguments, exercising the default-to-upstream path. Add such a test in t7600, which fails under the leak sanitizer without this fix. Signed-off-by: Toon Claes --- I ran into this leak while running `yay`[1] (I use Arch btw). `yay` uses Git to fetch packages from source, and it happens to call `git merge` without any revision to merge (it only passes the options `--no-edit` and `--ff`). Because I have Git with the leak sanitizer enabled in my $PATH, my `yay` tripped on a leak. This series fixes the leak and adds a test to reproduce it. [1]: https://github.com/Jguer/yay --- builtin/merge.c | 7 +++++-- t/t7600-merge.sh | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 58d1b7bb07..5b4eb23a83 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1373,7 +1373,7 @@ int cmd_merge(int argc, struct commit_list *common = NULL; const char *best_strategy = NULL, *wt_strategy = NULL; struct commit_list *remoteheads = NULL, *p; - void *branch_to_free; + void *branch_to_free, *argv_to_free = NULL; int orig_argc = argc; int merge_log_config = -1; @@ -1517,8 +1517,10 @@ int cmd_merge(int argc, option_commit = 1; if (!argc) { - if (default_to_upstream) + if (default_to_upstream) { argc = setup_with_upstream(&argv); + argv_to_free = argv; + } else die(_("No commit specified and merge.defaultToUpstream not set.")); } else if (argc == 1 && !strcmp(argv[0], "-")) { @@ -1880,6 +1882,7 @@ int cmd_merge(int argc, } strbuf_release(&buf); free(branch_to_free); + free(argv_to_free); free(pull_twohead); free(pull_octopus); discard_index(the_repository->index); diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh index 7f2a1db16d..e31d261f9d 100755 --- a/t/t7600-merge.sh +++ b/t/t7600-merge.sh @@ -1166,4 +1166,21 @@ test_expect_success 'suggested names are not ambiguous' ' test_grep remotes/origin/not-local stderr ' +test_expect_success 'merge with no argument defaults to upstream' ' + test_when_finished "rm -rf upstream downstream" && + git init upstream && + ( + cd upstream && + test_commit one && + test_commit two + ) && + git clone upstream downstream && + ( + cd downstream && + git reset --hard HEAD^ && + git merge && + test_cmp_rev origin/main HEAD + ) +' + test_done --- base-commit: 13c7afec212fc97ce257d15601659314c6673d6c change-id: 20260728-toon-fix-merge-leak-6d3bc5af2082 ``` ## Jeff King, 2026-07-28 15:19 Subject: Re: [PATCH] merge: fix leak with merge.defaultToUpstream Message-ID: <20260728151959.GC41931@coredump.intra.peff.net> In-Reply-To: <20260728-toon-fix-merge-leak-v1-1-d6bd161a3acf@iotcl.com> ``` On Tue, Jul 28, 2026 at 03:00:04PM +0200, Toon Claes wrote: > The leak has been present since 93e535a5b7 (merge: merge with the > default upstream branch without argument, 2011-03-24). Although the leak > sanitizer was enabled for tests in fc1ddf42af (t: remove > TEST_PASSES_SANITIZE_LEAK annotations, 2024-11-21), it went unnoticed > because no test calls `git merge` without arguments, exercising the > default-to-upstream path. Add such a test in t7600, which fails under > the leak sanitizer without this fix. Wow, I'm surprised we didn't cover this case in the test suite. Increasing coverage is good. > @@ -1517,8 +1517,10 @@ int cmd_merge(int argc, > option_commit = 1; > > if (!argc) { > - if (default_to_upstream) > + if (default_to_upstream) { > argc = setup_with_upstream(&argv); > + argv_to_free = argv; > + } The fix looks correct to me. This whole argv-juggling is pretty gross (especially the part below which overwrites argv[0]!). I suspect using a separate strvec to hold the heads would be cleaner, but it is probably not worth anybody's time to micro-polish this. -Peff ``` ## Junio C Hamano, 2026-07-28 18:12 Subject: Re: [PATCH] merge: fix leak with merge.defaultToUpstream Message-ID: In-Reply-To: <20260728151959.GC41931@coredump.intra.peff.net> ``` Jeff King writes: > On Tue, Jul 28, 2026 at 03:00:04PM +0200, Toon Claes wrote: > >> The leak has been present since 93e535a5b7 (merge: merge with the >> default upstream branch without argument, 2011-03-24). Although the leak >> sanitizer was enabled for tests in fc1ddf42af (t: remove >> TEST_PASSES_SANITIZE_LEAK annotations, 2024-11-21), it went unnoticed >> because no test calls `git merge` without arguments, exercising the >> default-to-upstream path. Add such a test in t7600, which fails under >> the leak sanitizer without this fix. > > Wow, I'm surprised we didn't cover this case in the test suite. > Increasing coverage is good. > >> @@ -1517,8 +1517,10 @@ int cmd_merge(int argc, >> option_commit = 1; >> >> if (!argc) { >> - if (default_to_upstream) >> + if (default_to_upstream) { >> argc = setup_with_upstream(&argv); >> + argv_to_free = argv; >> + } > > The fix looks correct to me. This whole argv-juggling is pretty gross > (especially the part below which overwrites argv[0]!). I suspect using a > separate strvec to hold the heads would be cleaner, but it is probably > not worth anybody's time to micro-polish this. Thanks for heading off my OCD before it showed ;-). And of course, thanks Toon for noticing and fixing. Will queue. ```