* [PATCH] merge: fix leak with merge.defaultToUpstream
@ 2026-07-28 13:00 Toon Claes
2026-07-28 15:19 ` Jeff King
0 siblings, 1 reply; 3+ messages in thread
From: Toon Claes @ 2026-07-28 13:00 UTC (permalink / raw)
To: git; +Cc: Toon Claes
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 <toon@iotcl.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] merge: fix leak with merge.defaultToUpstream
2026-07-28 13:00 [PATCH] merge: fix leak with merge.defaultToUpstream Toon Claes
@ 2026-07-28 15:19 ` Jeff King
2026-07-28 18:12 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Jeff King @ 2026-07-28 15:19 UTC (permalink / raw)
To: Toon Claes; +Cc: git
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] merge: fix leak with merge.defaultToUpstream
2026-07-28 15:19 ` Jeff King
@ 2026-07-28 18:12 ` Junio C Hamano
0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2026-07-28 18:12 UTC (permalink / raw)
To: Jeff King; +Cc: Toon Claes, git
Jeff King <peff@peff.net> 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-28 18:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28 13:00 [PATCH] merge: fix leak with merge.defaultToUpstream Toon Claes
2026-07-28 15:19 ` Jeff King
2026-07-28 18:12 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox