* [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1
@ 2021-04-09 18:47 Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
` (12 more replies)
0 siblings, 13 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt
This series fixes approximately half of the real leaks I've found while
running t0002 - t0099 under LSAN.
2 more series will likely be needed to allow t0000-t0099 to pass with LSAN
enabled. (I have all the necessary fixes ready and tested on my machine,
although I want to revisit some of my changes before I'm happy enough to
send them out for review - either way I figure it's easiest to deal with one
batch at a time, so I'll hold off on sending those out for now. One series
is going to consist almost entirely of UNLEAK annotations that are boring
and not worth merging until the real leaks are fixed.)
The exciting news is that once we succeed in getting t000-t0099 to run leak
free, we'll be a significant step closer to being able to run the entire
test-suite leak-free:
* before the merging of ah/plugleaks (fixing leaks in t0001): 53% of test
cases fail when LSAN is enabled (12386/23330).
* with ah/plugleaks + this series + my 2 currently unpublished series: 34%
of test cases fail when LSAN is enabled (7829/23342).
(I haven't bothered to test most of the intermediate stages, but ISTR that
ah/plugleaks which only had a marginal effect - somewhere on the order of
51-52% test cases were failing after that work merged.)
On the topic of avoiding regressions: I've started running a subset of the
test-suite with LSAN enabled (in addition to the full test-suite with ASAN
and UBSAN) on my Github fork of git, automatically on a daily basis. This
should hopefully help catch any new leaks that appear (and also new
ASAN/UBSAN issues). [The entire test-suite takes around 35 minutes with ASAN
or UBSAN enabled, which isn't too bad compared to the default
linux-gcc/linux-clang jobs which take a similar amount of time - although
they run the test-suite twice with 2 configurations.]
Andrzej Hunt (12):
revision: free remainder of old commit list in limit_list
wt-status: fix multiple small leaks
ls-files: free max_prefix when done
bloom: clear each bloom_key after use
branch: FREE_AND_NULL instead of NULL'ing real_ref
builtin/bugreport: don't leak prefixed filename
builtin/check-ignore: clear_pathspec before returning
builtin/checkout: clear pending objects after diffing
mailinfo: also free strbuf lists when clearing mailinfo
builtin/for-each-ref: free filter and UNLEAK sorting.
builtin/rebase: release git_format_patch_opt too
builtin/rm: avoid leaking pathspec and seen
bloom.c | 1 +
branch.c | 2 +-
builtin/bugreport.c | 8 +++++---
builtin/check-ignore.c | 1 +
builtin/checkout.c | 1 +
builtin/for-each-ref.c | 3 +++
builtin/ls-files.c | 1 +
builtin/rebase.c | 1 +
builtin/rm.c | 2 ++
mailinfo.c | 14 +++-----------
revision.c | 1 +
wt-status.c | 4 ++++
12 files changed, 24 insertions(+), 15 deletions(-)
base-commit: 89b43f80a514aee58b662ad606e6352e03eaeee4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-929%2Fahunt%2Fleaksan-100-part1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-929/ahunt/leaksan-100-part1-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/929
--
gitgitgadget
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/12] revision: free remainder of old commit list in limit_list
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-10 7:29 ` René Scharfe
2021-04-09 18:47 ` [PATCH 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
` (11 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
limit_list() iterates over the original revs->commits list, and consumes
many of its entries via pop_commit. However we might stop iterating over
the list early (e.g. if we realise that the rest of the list is
uninteresting). If we do stop iterating early, list will be pointing to
the unconsumed portion of revs->commits - and we need to free this list
to avoid a leak. (revs->commits itself will be an invalid pointer: it
will have been free'd during the first pop_commit.)
This leak was found while running t0090. It's not likely to be very
impactful, but it can happen quite early during some checkout
invocations, and hence seems to be worth fixing:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac084 in do_xmalloc wrapper.c:41:8
#2 0x9ac05a in xmalloc wrapper.c:62:9
#3 0x7175d6 in commit_list_insert commit.c:540:33
#4 0x71800f in commit_list_insert_by_date commit.c:604:9
#5 0x8f8d2e in process_parents revision.c:1128:5
#6 0x8f2f2c in limit_list revision.c:1418:7
#7 0x8f210e in prepare_revision_walk revision.c:3577:7
#8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
#9 0x512f05 in switch_branches builtin/checkout.c:1250:3
#10 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#11 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#13 0x4cd91d in run_builtin git.c:467:11
#14 0x4cb5f3 in handle_builtin git.c:719:3
#15 0x4ccf47 in run_argv git.c:808:4
#16 0x4caf49 in cmd_main git.c:939:19
#17 0x69dc0e in main common-main.c:52:11
#18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 48 byte(s) in 3 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac084 in do_xmalloc wrapper.c:41:8
#2 0x9ac05a in xmalloc wrapper.c:62:9
#3 0x717de6 in commit_list_append commit.c:1609:35
#4 0x8f1f9b in prepare_revision_walk revision.c:3554:12
#5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
#6 0x512f05 in switch_branches builtin/checkout.c:1250:3
#7 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#8 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
revision.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/revision.c b/revision.c
index 553c0faa9b38..7b509aab0c87 100644
--- a/revision.c
+++ b/revision.c
@@ -1460,6 +1460,7 @@ static int limit_list(struct rev_info *revs)
update_treesame(revs, c);
}
+ free_commit_list(list);
revs->commits = newlist;
return 0;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 02/12] wt-status: fix multiple small leaks
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
` (10 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
rev.prune_data is populated (in multiple functions) via copy_pathspec,
and therefore needs to be cleared after running the diff in those
functions.
rev(_info).pending is populated indirectly via setup_revisions, and also
needs to be cleared once diffing is done.
These leaks were found while running t0008 or t0021. The rev.prune_data
leaks are small (80B) but noisy, hence I won't bother including their
logs - the rev.pending leaks are bigger, and can happen early in the
course of other commands, and therefore possibly more valuable to fix -
see example log from a rebase below:
Direct leak of 2048 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9ac2a6 in xrealloc wrapper.c:126:8
#2 0x83da03 in add_object_array_with_path object.c:337:3
#3 0x8f5d8a in add_pending_object_with_path revision.c:329:2
#4 0x8ea50b in add_pending_object_with_mode revision.c:336:2
#5 0x8ea4fd in add_pending_object revision.c:342:2
#6 0x8ea610 in add_head_to_pending revision.c:354:2
#7 0x9b55f5 in has_uncommitted_changes wt-status.c:2474:2
#8 0x9b58c4 in require_clean_work_tree wt-status.c:2553:6
#9 0x606bcc in cmd_rebase builtin/rebase.c:1970:6
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7f2d18909349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 5 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ac048 in xstrdup wrapper.c:29:14
#2 0x83da8d in add_object_array_with_path object.c:349:17
#3 0x8f5d8a in add_pending_object_with_path revision.c:329:2
#4 0x8ea50b in add_pending_object_with_mode revision.c:336:2
#5 0x8ea4fd in add_pending_object revision.c:342:2
#6 0x8ea610 in add_head_to_pending revision.c:354:2
#7 0x9b55f5 in has_uncommitted_changes wt-status.c:2474:2
#8 0x9b58c4 in require_clean_work_tree wt-status.c:2553:6
#9 0x606bcc in cmd_rebase builtin/rebase.c:1970:6
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7f2d18909349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 2053 byte(s) leaked in 2 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
wt-status.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/wt-status.c b/wt-status.c
index 1aed68c43c26..34886655dbcc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -616,6 +616,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
copy_pathspec(&rev.prune_data, &s->pathspec);
run_diff_files(&rev, 0);
+ clear_pathspec(&rev.prune_data);
}
static void wt_status_collect_changes_index(struct wt_status *s)
@@ -652,6 +653,8 @@ static void wt_status_collect_changes_index(struct wt_status *s)
rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
copy_pathspec(&rev.prune_data, &s->pathspec);
run_diff_index(&rev, 1);
+ object_array_clear(&rev.pending);
+ clear_pathspec(&rev.prune_data);
}
static void wt_status_collect_changes_initial(struct wt_status *s)
@@ -2480,6 +2483,7 @@ int has_uncommitted_changes(struct repository *r,
diff_setup_done(&rev_info.diffopt);
result = run_diff_index(&rev_info, 1);
+ object_array_clear(&rev_info.pending);
return diff_result_code(&rev_info.diffopt, result);
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 03/12] ls-files: free max_prefix when done
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-10 8:12 ` René Scharfe
2021-04-09 18:47 ` [PATCH 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
` (9 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
common_prefix() returns a new string, which we store in max_prefix -
this string needs to be freed to avoid a leak. This leak is happening
in cmd_ls_files, hence is of no real consequence - an UNLEAK would be
just as good, but we might as well free the string properly.
Leak found while running t0002, see output below:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ab1b4 in do_xmalloc wrapper.c:41:8
#2 0x9ab248 in do_xmallocz wrapper.c:75:8
#3 0x9ab22a in xmallocz wrapper.c:83:9
#4 0x9ab2d7 in xmemdupz wrapper.c:99:16
#5 0x78d6a4 in common_prefix dir.c:191:15
#6 0x5aca48 in cmd_ls_files builtin/ls-files.c:669:16
#7 0x4cd92d in run_builtin git.c:453:11
#8 0x4cb5fa in handle_builtin git.c:704:3
#9 0x4ccf57 in run_argv git.c:771:4
#10 0x4caf49 in cmd_main git.c:902:19
#11 0x69ce2e in main common-main.c:52:11
#12 0x7f64d4d94349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/ls-files.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 60a2913a01e9..53e20bbf9cce 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -781,5 +781,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
}
dir_clear(&dir);
+ free((void *)max_prefix);
return 0;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 04/12] bloom: clear each bloom_key after use
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (2 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-11 7:26 ` SZEDER Gábor
2021-04-09 18:47 ` [PATCH 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
` (8 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
fill_bloom_key() allocates memory into bloom_key, we need to clean that
up once the key is no longer needed.
This fixes the following leak which was found while running t0002-t0099.
Although this leak is happening in code being called from a test-helper,
the same code is also used in various locations around git, and could
presumably happen during normal usage too.
Direct leak of 308 byte(s) in 11 object(s) allocated from:
#0 0x49a5e2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x6f4032 in xcalloc wrapper.c:140:8
#2 0x4f2905 in fill_bloom_key bloom.c:137:28
#3 0x4f34c1 in get_or_compute_bloom_filter bloom.c:284:4
#4 0x4cb484 in get_bloom_filter_for_commit t/helper/test-bloom.c:43:11
#5 0x4cb072 in cmd__bloom t/helper/test-bloom.c:97:3
#6 0x4ca7ef in cmd_main t/helper/test-tool.c:121:11
#7 0x4caace in main common-main.c:52:11
#8 0x7f798af95349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 308 byte(s) leaked in 11 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
bloom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/bloom.c b/bloom.c
index 52b87474c6eb..5e297038bb1f 100644
--- a/bloom.c
+++ b/bloom.c
@@ -283,6 +283,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
struct bloom_key key;
fill_bloom_key(e->path, strlen(e->path), &key, settings);
add_key_to_filter(&key, filter, settings);
+ clear_bloom_key(&key);
}
cleanup:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (3 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
` (7 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
real_ref was previously populated by dwim_ref(), which allocates new
memory. We need to make sure to free real_ref when discarding it.
(real_ref is already being freed at the end of create_branch() - but
if we discard it early then it will leak.)
This fixes the following leak found while running t0002-t0099:
Direct leak of 5 byte(s) in 1 object(s) allocated from:
#0 0x486954 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xdd6484 in xstrdup wrapper.c:29:14
#2 0xc0f658 in expand_ref refs.c:671:12
#3 0xc0ecf1 in repo_dwim_ref refs.c:644:22
#4 0x8b1184 in dwim_ref ./refs.h:162:9
#5 0x8b0b02 in create_branch branch.c:284:10
#6 0x550cbb in update_refs_for_switch builtin/checkout.c:1046:4
#7 0x54e275 in switch_branches builtin/checkout.c:1274:2
#8 0x548828 in checkout_branch builtin/checkout.c:1668:9
#9 0x541306 in checkout_main builtin/checkout.c:2025:9
#10 0x5395fa in cmd_checkout builtin/checkout.c:2077:8
#11 0x4d02a8 in run_builtin git.c:467:11
#12 0x4cbfe9 in handle_builtin git.c:719:3
#13 0x4cf04f in run_argv git.c:808:4
#14 0x4cb85a in cmd_main git.c:939:19
#15 0x820cf6 in main common-main.c:52:11
#16 0x7f30bd9dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
branch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/branch.c b/branch.c
index 9c9dae1eae32..514a3311d8d1 100644
--- a/branch.c
+++ b/branch.c
@@ -294,7 +294,7 @@ void create_branch(struct repository *r,
if (explicit_tracking)
die(_(upstream_not_branch), start_name);
else
- real_ref = NULL;
+ FREE_AND_NULL(real_ref);
}
break;
default:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 06/12] builtin/bugreport: don't leak prefixed filename
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (4 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
` (6 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
prefix_filename() returns newly allocated memory, and strbuf_addstr()
doesn't take ownership of its inputs. Therefore we have to make sure to
store and free prefix_filename()'s result.
As this leak is in cmd_bugreport(), we could just as well UNLEAK the
prefix - but there's no good reason not to just free it properly. This
leak was found while running t0091, see output below:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc66 in xrealloc wrapper.c:126:8
#2 0x93baed in strbuf_grow strbuf.c:98:2
#3 0x93c6ea in strbuf_add strbuf.c:295:2
#4 0x69f162 in strbuf_addstr ./strbuf.h:304:2
#5 0x69f083 in prefix_filename abspath.c:277:2
#6 0x4fb275 in cmd_bugreport builtin/bugreport.c:146:9
#7 0x4cd91d in run_builtin git.c:467:11
#8 0x4cb5f3 in handle_builtin git.c:719:3
#9 0x4ccf47 in run_argv git.c:808:4
#10 0x4caf49 in cmd_main git.c:939:19
#11 0x69df9e in main common-main.c:52:11
#12 0x7f523a987349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/bugreport.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/bugreport.c b/builtin/bugreport.c
index ad3cc9c02f62..9915a5841def 100644
--- a/builtin/bugreport.c
+++ b/builtin/bugreport.c
@@ -129,6 +129,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
char *option_output = NULL;
char *option_suffix = "%Y-%m-%d-%H%M";
const char *user_relative_path = NULL;
+ char *prefixed_filename;
const struct option bugreport_options[] = {
OPT_STRING('o', "output-directory", &option_output, N_("path"),
@@ -142,9 +143,9 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
bugreport_usage, 0);
/* Prepare the path to put the result */
- strbuf_addstr(&report_path,
- prefix_filename(prefix,
- option_output ? option_output : ""));
+ prefixed_filename = prefix_filename(prefix,
+ option_output ? option_output : "");
+ strbuf_addstr(&report_path, prefixed_filename);
strbuf_complete(&report_path, '/');
strbuf_addstr(&report_path, "git-bugreport-");
@@ -189,6 +190,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
fprintf(stderr, _("Created new report at '%s'.\n"),
user_relative_path);
+ free(prefixed_filename);
UNLEAK(buffer);
UNLEAK(report_path);
return !!launch_editor(report_path.buf, NULL, NULL);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 07/12] builtin/check-ignore: clear_pathspec before returning
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (5 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
` (5 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
parse_pathspec() allocates new memory into pathspec, therefore we need
to free it when we're done.
An UNLEAK would probably be just as good here - but clear_pathspec() is
not much more work so we might as well use it. check_ignore() is either
called once directly from cmd_check_ignore() (in which case the leak
really doesnt matter), or it can be called multiple times in a loop from
check_ignore_stdin_paths(), in which case we're potentially leaking
multiple times - but even in this scenario the leak is so small as to
have no real consequence.
Found while running t0008:
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9aca44 in do_xmalloc wrapper.c:41:8
#2 0x9aca1a in xmalloc wrapper.c:62:9
#3 0x873c17 in parse_pathspec pathspec.c:582:2
#4 0x503eb8 in check_ignore builtin/check-ignore.c:90:2
#5 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17
#6 0x4cd91d in run_builtin git.c:467:11
#7 0x4cb5f3 in handle_builtin git.c:719:3
#8 0x4ccf47 in run_argv git.c:808:4
#9 0x4caf49 in cmd_main git.c:939:19
#10 0x69e43e in main common-main.c:52:11
#11 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc46 in xrealloc wrapper.c:126:8
#2 0x93baed in strbuf_grow strbuf.c:98:2
#3 0x93d696 in strbuf_vaddf strbuf.c:392:3
#4 0x9400c6 in xstrvfmt strbuf.c:979:2
#5 0x940253 in xstrfmt strbuf.c:989:8
#6 0x92b72a in prefix_path_gently setup.c:115:15
#7 0x87442d in init_pathspec_item pathspec.c:439:11
#8 0x873cef in parse_pathspec pathspec.c:589:3
#9 0x503eb8 in check_ignore builtin/check-ignore.c:90:2
#10 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17
#11 0x4cd91d in run_builtin git.c:467:11
#12 0x4cb5f3 in handle_builtin git.c:719:3
#13 0x4ccf47 in run_argv git.c:808:4
#14 0x4caf49 in cmd_main git.c:939:19
#15 0x69e43e in main common-main.c:52:11
#16 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ac9e8 in xstrdup wrapper.c:29:14
#2 0x874542 in init_pathspec_item pathspec.c:468:20
#3 0x873cef in parse_pathspec pathspec.c:589:3
#4 0x503eb8 in check_ignore builtin/check-ignore.c:90:2
#5 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17
#6 0x4cd91d in run_builtin git.c:467:11
#7 0x4cb5f3 in handle_builtin git.c:719:3
#8 0x4ccf47 in run_argv git.c:808:4
#9 0x4caf49 in cmd_main git.c:939:19
#10 0x69e43e in main common-main.c:52:11
#11 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 179 byte(s) leaked in 3 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/check-ignore.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 3c652748d58c..467e92cc7b80 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -118,6 +118,7 @@ static int check_ignore(struct dir_struct *dir,
num_ignored++;
}
free(seen);
+ clear_pathspec(&pathspec);
return num_ignored;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 08/12] builtin/checkout: clear pending objects after diffing
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (6 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
` (4 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.
This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:
Direct leak of 2048 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc46 in xrealloc wrapper.c:126:8
#2 0x83e3a3 in add_object_array_with_path object.c:337:3
#3 0x8f672a in add_pending_object_with_path revision.c:329:2
#4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
#5 0x8eae9d in add_pending_object revision.c:342:2
#6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
#7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#12 0x4cd91d in run_builtin git.c:467:11
#13 0x4cb5f3 in handle_builtin git.c:719:3
#14 0x4ccf47 in run_argv git.c:808:4
#15 0x4caf49 in cmd_main git.c:939:19
#16 0x69e43e in main common-main.c:52:11
#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/checkout.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4c696ef4805b..190153c81571 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -602,6 +602,7 @@ static void show_local_changes(struct object *head,
diff_setup_done(&rev.diffopt);
add_pending_object(&rev, head, NULL);
run_diff_index(&rev, 0);
+ object_array_clear(&rev.pending);
}
static void describe_detached_head(const char *msg, struct commit *commit)
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (7 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-11 11:43 ` Junio C Hamano
2021-04-09 18:47 ` [PATCH 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
` (3 subsequent siblings)
12 siblings, 1 reply; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
mailinfo.p_hdr_info/s_hdr_info are null-terminated lists of strbuf's,
with entries pointing either to NULL or an allocated strbuf. Therefore
we need to free those strbuf's (and not just the data they contain)
whenever we're done with a given entry. (See handle_header() where those
new strbufs are malloc'd.)
Once we no longer need the list (and not just its entries) we can switch
over to strbuf_list_free() instead of manually iterating over the list,
which takes care of those additional details for us. We can only do this
in clear_mailinfo() - in handle_commit_message() we are only clearing the
array contents but want to reuse the array itself, hence we can't use
strbuf_list_free() there.
Leak output from t0023:
Direct leak of 72 byte(s) in 3 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac9f4 in do_xmalloc wrapper.c:41:8
#2 0x9ac9ca in xmalloc wrapper.c:62:9
#3 0x7f6cf7 in handle_header mailinfo.c:205:10
#4 0x7f5abf in check_header mailinfo.c:583:4
#5 0x7f5524 in mailinfo mailinfo.c:1197:3
#6 0x4dcc95 in parse_mail builtin/am.c:1167:6
#7 0x4d9070 in am_run builtin/am.c:1732:12
#8 0x4d5b7a in cmd_am builtin/am.c:2398:3
#9 0x4cd91d in run_builtin git.c:467:11
#10 0x4cb5f3 in handle_builtin git.c:719:3
#11 0x4ccf47 in run_argv git.c:808:4
#12 0x4caf49 in cmd_main git.c:939:19
#13 0x69e43e in main common-main.c:52:11
#14 0x7fc1fadfa349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 72 byte(s) leaked in 3 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
mailinfo.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/mailinfo.c b/mailinfo.c
index 5681d9130db6..95ce191f385b 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -821,7 +821,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
for (i = 0; header[i]; i++) {
if (mi->s_hdr_data[i])
strbuf_release(mi->s_hdr_data[i]);
- mi->s_hdr_data[i] = NULL;
+ FREE_AND_NULL(mi->s_hdr_data[i]);
}
return 0;
}
@@ -1236,22 +1236,14 @@ void setup_mailinfo(struct mailinfo *mi)
void clear_mailinfo(struct mailinfo *mi)
{
- int i;
-
strbuf_release(&mi->name);
strbuf_release(&mi->email);
strbuf_release(&mi->charset);
strbuf_release(&mi->inbody_header_accum);
free(mi->message_id);
- if (mi->p_hdr_data)
- for (i = 0; mi->p_hdr_data[i]; i++)
- strbuf_release(mi->p_hdr_data[i]);
- free(mi->p_hdr_data);
- if (mi->s_hdr_data)
- for (i = 0; mi->s_hdr_data[i]; i++)
- strbuf_release(mi->s_hdr_data[i]);
- free(mi->s_hdr_data);
+ strbuf_list_free(mi->p_hdr_data);
+ strbuf_list_free(mi->s_hdr_data);
while (mi->content < mi->content_top) {
free(*(mi->content_top));
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 10/12] builtin/for-each-ref: free filter and UNLEAK sorting.
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (8 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
` (2 subsequent siblings)
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
sorting might be a list allocated in ref_default_sorting() (in this case
it's a fixed single item list, which has nevertheless been xcalloc'd),
or it might be a list allocated in parse_opt_ref_sorting(). In either
case we could free these lists - but instead we UNLEAK as we're at the
end of cmd_for_each_ref. (There's no existing implementation of
clear_ref_sorting(), and writing a loop to free the list seems more
trouble than it's worth.)
filter.with_commit/no_commit are populated via
OPT_CONTAINS/OPT_NO_CONTAINS, both of which create new entries via
parse_opt_commits(), and also need to be free'd or UNLEAK'd. Because
free_commit_list() already exists, we choose to use that over an UNLEAK.
LSAN output from t0041:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a9d2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x9ac252 in xcalloc wrapper.c:140:8
#2 0x8a4a55 in ref_default_sorting ref-filter.c:2486:32
#3 0x56c6b1 in cmd_for_each_ref builtin/for-each-ref.c:72:13
#4 0x4cd91d in run_builtin git.c:467:11
#5 0x4cb5f3 in handle_builtin git.c:719:3
#6 0x4ccf47 in run_argv git.c:808:4
#7 0x4caf49 in cmd_main git.c:939:19
#8 0x69dabe in main common-main.c:52:11
#9 0x7f2bdc570349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9abf54 in do_xmalloc wrapper.c:41:8
#2 0x9abf2a in xmalloc wrapper.c:62:9
#3 0x717486 in commit_list_insert commit.c:540:33
#4 0x8644cf in parse_opt_commits parse-options-cb.c:98:2
#5 0x869bb5 in get_value parse-options.c:181:11
#6 0x8677dc in parse_long_opt parse-options.c:378:10
#7 0x8659bd in parse_options_step parse-options.c:817:11
#8 0x867fcd in parse_options parse-options.c:870:10
#9 0x56c62b in cmd_for_each_ref builtin/for-each-ref.c:59:2
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dabe in main common-main.c:52:11
#15 0x7f2bdc570349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/for-each-ref.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index cb9c81a04606..84efb71f82fc 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -83,5 +83,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
for (i = 0; i < maxcount; i++)
show_ref_array_item(array.items[i], &format);
ref_array_clear(&array);
+ free_commit_list(filter.with_commit);
+ free_commit_list(filter.no_commit);
+ UNLEAK(sorting);
return 0;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 11/12] builtin/rebase: release git_format_patch_opt too
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (9 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
options.git_format_patch_opt can be populated during cmd_rebase's setup,
and will therefore leak on return. Although we could just UNLEAK all of
options, we choose to strbuf_release() the individual member, which matches
the existing pattern (where we're freeing invidual members of options).
Leak found when running t0021:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9ac296 in xrealloc wrapper.c:126:8
#2 0x93b13d in strbuf_grow strbuf.c:98:2
#3 0x93bd3a in strbuf_add strbuf.c:295:2
#4 0x60ae92 in strbuf_addstr strbuf.h:304:2
#5 0x605f17 in cmd_rebase builtin/rebase.c:1759:3
#6 0x4cd91d in run_builtin git.c:467:11
#7 0x4cb5f3 in handle_builtin git.c:719:3
#8 0x4ccf47 in run_argv git.c:808:4
#9 0x4caf49 in cmd_main git.c:939:19
#10 0x69dbfe in main common-main.c:52:11
#11 0x7f66dae91349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/rebase.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 783b526f6e75..c0cd2c40c7d8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -2108,6 +2108,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
free(options.head_name);
free(options.gpg_sign_opt);
free(options.cmd);
+ strbuf_release(&options.git_format_patch_opt);
free(squash_onto_name);
return ret;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 12/12] builtin/rm: avoid leaking pathspec and seen
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (10 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
@ 2021-04-09 18:47 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
12 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-09 18:47 UTC (permalink / raw)
To: git; +Cc: Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
parse_pathspec() populates pathspec, hence we need to clear it once it's
no longer needed. seen is xcalloc'd within the same function and
likewise needs to be freed once its no longer needed.
cmd_rm() has multiple early returns, therefore we need to clear or free
as soon as this data is no longer needed, as opposed to doing a cleanup
at the end.
LSAN output from t0020:
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac0a4 in do_xmalloc wrapper.c:41:8
#2 0x9ac07a in xmalloc wrapper.c:62:9
#3 0x873277 in parse_pathspec pathspec.c:582:2
#4 0x646ffa in cmd_rm builtin/rm.c:266:2
#5 0x4cd91d in run_builtin git.c:467:11
#6 0x4cb5f3 in handle_builtin git.c:719:3
#7 0x4ccf47 in run_argv git.c:808:4
#8 0x4caf49 in cmd_main git.c:939:19
#9 0x69dc0e in main common-main.c:52:11
#10 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9ac2a6 in xrealloc wrapper.c:126:8
#2 0x93b14d in strbuf_grow strbuf.c:98:2
#3 0x93ccf6 in strbuf_vaddf strbuf.c:392:3
#4 0x93f726 in xstrvfmt strbuf.c:979:2
#5 0x93f8b3 in xstrfmt strbuf.c:989:8
#6 0x92ad8a in prefix_path_gently setup.c:115:15
#7 0x873a8d in init_pathspec_item pathspec.c:439:11
#8 0x87334f in parse_pathspec pathspec.c:589:3
#9 0x646ffa in cmd_rm builtin/rm.c:266:2
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ac048 in xstrdup wrapper.c:29:14
#2 0x873ba2 in init_pathspec_item pathspec.c:468:20
#3 0x87334f in parse_pathspec pathspec.c:589:3
#4 0x646ffa in cmd_rm builtin/rm.c:266:2
#5 0x4cd91d in run_builtin git.c:467:11
#6 0x4cb5f3 in handle_builtin git.c:719:3
#7 0x4ccf47 in run_argv git.c:808:4
#8 0x4caf49 in cmd_main git.c:939:19
#9 0x69dc0e in main common-main.c:52:11
#10 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 1 byte(s) in 1 object(s) allocated from:
#0 0x49a9d2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x9ac392 in xcalloc wrapper.c:140:8
#2 0x647108 in cmd_rm builtin/rm.c:294:9
#3 0x4cd91d in run_builtin git.c:467:11
#4 0x4cb5f3 in handle_builtin git.c:719:3
#5 0x4ccf47 in run_argv git.c:808:4
#6 0x4caf49 in cmd_main git.c:939:19
#7 0x69dbfe in main common-main.c:52:11
#8 0x7f4fac1b0349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/rm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/rm.c b/builtin/rm.c
index 4858631e0f02..2927678d37b6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -327,6 +327,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!seen_any)
exit(0);
}
+ clear_pathspec(&pathspec);
+ free(seen);
if (!index_only)
submodules_absorb_gitdir_if_needed();
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 01/12] revision: free remainder of old commit list in limit_list
2021-04-09 18:47 ` [PATCH 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
@ 2021-04-10 7:29 ` René Scharfe
2021-04-25 13:32 ` Andrzej Hunt
0 siblings, 1 reply; 35+ messages in thread
From: René Scharfe @ 2021-04-10 7:29 UTC (permalink / raw)
To: Andrzej Hunt via GitGitGadget, git; +Cc: Andrzej Hunt, Andrzej Hunt
Am 09.04.21 um 20:47 schrieb Andrzej Hunt via GitGitGadget:
> From: Andrzej Hunt <ajrhunt@google.com>
>
> limit_list() iterates over the original revs->commits list, and consumes
> many of its entries via pop_commit. However we might stop iterating over
> the list early (e.g. if we realise that the rest of the list is
> uninteresting). If we do stop iterating early, list will be pointing to
> the unconsumed portion of revs->commits - and we need to free this list
> to avoid a leak. (revs->commits itself will be an invalid pointer: it
> will have been free'd during the first pop_commit.)
>
> This leak was found while running t0090. It's not likely to be very
> impactful, but it can happen quite early during some checkout
> invocations, and hence seems to be worth fixing:
>
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
> #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
> #1 0x9ac084 in do_xmalloc wrapper.c:41:8
> #2 0x9ac05a in xmalloc wrapper.c:62:9
> #3 0x7175d6 in commit_list_insert commit.c:540:33
> #4 0x71800f in commit_list_insert_by_date commit.c:604:9
> #5 0x8f8d2e in process_parents revision.c:1128:5
> #6 0x8f2f2c in limit_list revision.c:1418:7
> #7 0x8f210e in prepare_revision_walk revision.c:3577:7
> #8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
> #9 0x512f05 in switch_branches builtin/checkout.c:1250:3
> #10 0x50f8de in checkout_branch builtin/checkout.c:1646:9
> #11 0x50ba12 in checkout_main builtin/checkout.c:2003:9
> #12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
> #13 0x4cd91d in run_builtin git.c:467:11
> #14 0x4cb5f3 in handle_builtin git.c:719:3
> #15 0x4ccf47 in run_argv git.c:808:4
> #16 0x4caf49 in cmd_main git.c:939:19
> #17 0x69dc0e in main common-main.c:52:11
> #18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> Indirect leak of 48 byte(s) in 3 object(s) allocated from:
> #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
> #1 0x9ac084 in do_xmalloc wrapper.c:41:8
> #2 0x9ac05a in xmalloc wrapper.c:62:9
> #3 0x717de6 in commit_list_append commit.c:1609:35
> #4 0x8f1f9b in prepare_revision_walk revision.c:3554:12
> #5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
> #6 0x512f05 in switch_branches builtin/checkout.c:1250:3
> #7 0x50f8de in checkout_branch builtin/checkout.c:1646:9
> #8 0x50ba12 in checkout_main builtin/checkout.c:2003:9
> #9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
> #10 0x4cd91d in run_builtin git.c:467:11
> #11 0x4cb5f3 in handle_builtin git.c:719:3
> #12 0x4ccf47 in run_argv git.c:808:4
> #13 0x4caf49 in cmd_main git.c:939:19
> #14 0x69dc0e in main common-main.c:52:11
> #15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
> ---
> revision.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/revision.c b/revision.c
> index 553c0faa9b38..7b509aab0c87 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1460,6 +1460,7 @@ static int limit_list(struct rev_info *revs)
> update_treesame(revs, c);
> }
>
> + free_commit_list(list);
This patch would benefit from more context, but this function is quite
long. So let me sketch it:
struct commit_list *list = revs->commits;
while (list) {
struct commit *commit = pop_commit(&list);
struct object *obj = &commit->object;
if (obj->flags & UNINTERESTING) {
break;
}
}
if (limiting_can_increase_treesame(revs))
for (list = newlist; list; list = list->next) {
}
free_commit_list(list);
So the while loop can leave list dangling and you want to free its
remaining entries. The for loop sometimes overwrites the list pointer,
though, and you will end up passing NULL to free_commit_list in that
case. So either the call should be moved between the loops or a fresh
variable should be used in the second loop instead of reusing list to
make sure the entries are released in all cases.
> revs->commits = newlist;
> return 0;
> }
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 03/12] ls-files: free max_prefix when done
2021-04-09 18:47 ` [PATCH 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
@ 2021-04-10 8:12 ` René Scharfe
2021-04-25 13:16 ` Andrzej Hunt
0 siblings, 1 reply; 35+ messages in thread
From: René Scharfe @ 2021-04-10 8:12 UTC (permalink / raw)
To: Andrzej Hunt via GitGitGadget, git; +Cc: Andrzej Hunt, Andrzej Hunt
Am 09.04.21 um 20:47 schrieb Andrzej Hunt via GitGitGadget:
> From: Andrzej Hunt <ajrhunt@google.com>
>
> common_prefix() returns a new string, which we store in max_prefix -
> this string needs to be freed to avoid a leak. This leak is happening
> in cmd_ls_files, hence is of no real consequence - an UNLEAK would be
> just as good, but we might as well free the string properly.
>
> Leak found while running t0002, see output below:
>
> Direct leak of 8 byte(s) in 1 object(s) allocated from:
> #0 0x49a85d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
> #1 0x9ab1b4 in do_xmalloc wrapper.c:41:8
> #2 0x9ab248 in do_xmallocz wrapper.c:75:8
> #3 0x9ab22a in xmallocz wrapper.c:83:9
> #4 0x9ab2d7 in xmemdupz wrapper.c:99:16
> #5 0x78d6a4 in common_prefix dir.c:191:15
> #6 0x5aca48 in cmd_ls_files builtin/ls-files.c:669:16
> #7 0x4cd92d in run_builtin git.c:453:11
> #8 0x4cb5fa in handle_builtin git.c:704:3
> #9 0x4ccf57 in run_argv git.c:771:4
> #10 0x4caf49 in cmd_main git.c:902:19
> #11 0x69ce2e in main common-main.c:52:11
> #12 0x7f64d4d94349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
> ---
> builtin/ls-files.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index 60a2913a01e9..53e20bbf9cce 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -781,5 +781,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
> }
>
> dir_clear(&dir);
> + free((void *)max_prefix);
This cast is necessary to ignore the const attribute of the pointer.
It's scary, but safe here because this function owns the referenced
object.
I think the promise to not modify the string given at the top of the
function is not worth having to take back that promise forcefully at
the end to dispose of it. Determining the correctness of this cast
requires reading the whole function. Removing the const from the
declaration (and the cast) would improve readability overall. Thoughts?
> return 0;
> }
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 04/12] bloom: clear each bloom_key after use
2021-04-09 18:47 ` [PATCH 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
@ 2021-04-11 7:26 ` SZEDER Gábor
2021-04-25 13:17 ` Andrzej Hunt
0 siblings, 1 reply; 35+ messages in thread
From: SZEDER Gábor @ 2021-04-11 7:26 UTC (permalink / raw)
To: Andrzej Hunt via GitGitGadget; +Cc: git, Andrzej Hunt, Andrzej Hunt
On Fri, Apr 09, 2021 at 06:47:23PM +0000, Andrzej Hunt via GitGitGadget wrote:
> From: Andrzej Hunt <ajrhunt@google.com>
>
> fill_bloom_key() allocates memory into bloom_key, we need to clean that
> up once the key is no longer needed.
>
> This fixes the following leak which was found while running t0002-t0099.
> Although this leak is happening in code being called from a test-helper,
> the same code is also used in various locations around git, and could
> presumably happen during normal usage too.
It does indeed happen: 'git commit-graph write --reachable
--changed-paths' generates Bloom filters for every commit, with each
filter containing all paths modified by its associated commit, so it
leaks a lot of 7 * 4byte hashes. This patch reduces the memory usage
of that command:
Max RSS
before after
---------------------------------------------
android-base 1275028k 1006576k -21.1%
chromium 3245144k 3127764k -3.6%
cmssw 793996k 699156k -12.0%
cpython 371584k 343480k -7.6%
elasticsearch 748104k 637936k -14.7%
freebsd-src 819020k 741272k -9.5%
gcc 867412k 730332k -15.8%
gecko-dev 2619112k 2457280k -6.2%
git 252684k 216900k -14.2%
glibc 239000k 222228k -7.0%
go 264132k 251344k -4.9%
homebrew-cask 542188k 480588k -11.4%
homebrew-core 805332k 715848k -11.1%
jdk 417832k 342928k -17.9%
libreoff-core 1257296k 1089980k -13.3%
linux 2033296k 1759712k -13.5%
llvm-project 1067216k 956704k -10.4%
mariadb-srv 695172k 559508k -19.5%
postgres 340132k 317416k -6.7%
rails 325432k 294332k -9.6%
rust 655244k 584904k -10.7%
tensorflow 507308k 480848k -5.2%
webkit 2466812k 2237332k -9.3%
Just out of curiosity, I disabled the questionable hardcoded 512 paths
limit on the size of modified path Bloom filters, and the memory usage
in the jdk repository sunk by over 55%, from 849520k to 379760k.
Please feel free to include any of the above data points in the commit
message.
> Direct leak of 308 byte(s) in 11 object(s) allocated from:
> #0 0x49a5e2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
> #1 0x6f4032 in xcalloc wrapper.c:140:8
> #2 0x4f2905 in fill_bloom_key bloom.c:137:28
> #3 0x4f34c1 in get_or_compute_bloom_filter bloom.c:284:4
> #4 0x4cb484 in get_bloom_filter_for_commit t/helper/test-bloom.c:43:11
> #5 0x4cb072 in cmd__bloom t/helper/test-bloom.c:97:3
> #6 0x4ca7ef in cmd_main t/helper/test-tool.c:121:11
> #7 0x4caace in main common-main.c:52:11
> #8 0x7f798af95349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> SUMMARY: AddressSanitizer: 308 byte(s) leaked in 11 allocation(s).
>
> Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
> ---
> bloom.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/bloom.c b/bloom.c
> index 52b87474c6eb..5e297038bb1f 100644
> --- a/bloom.c
> +++ b/bloom.c
> @@ -283,6 +283,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
> struct bloom_key key;
> fill_bloom_key(e->path, strlen(e->path), &key, settings);
> add_key_to_filter(&key, filter, settings);
> + clear_bloom_key(&key);
> }
>
> cleanup:
> --
> gitgitgadget
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo
2021-04-09 18:47 ` [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
@ 2021-04-11 11:43 ` Junio C Hamano
2021-04-25 13:15 ` Andrzej Hunt
0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2021-04-11 11:43 UTC (permalink / raw)
To: Andrzej Hunt via GitGitGadget; +Cc: git, Andrzej Hunt, Andrzej Hunt
"Andrzej Hunt via GitGitGadget" <gitgitgadget@gmail.com> writes:
> void clear_mailinfo(struct mailinfo *mi)
> {
> - int i;
> -
> strbuf_release(&mi->name);
> strbuf_release(&mi->email);
> strbuf_release(&mi->charset);
> strbuf_release(&mi->inbody_header_accum);
> free(mi->message_id);
>
> - if (mi->p_hdr_data)
> - for (i = 0; mi->p_hdr_data[i]; i++)
> - strbuf_release(mi->p_hdr_data[i]);
> - free(mi->p_hdr_data);
> - if (mi->s_hdr_data)
> - for (i = 0; mi->s_hdr_data[i]; i++)
> - strbuf_release(mi->s_hdr_data[i]);
> - free(mi->s_hdr_data);
So, the original allows mi->p_hdr_data to be NULL and does not do
this freeing (the same for the .s_hdr_data member).
> + strbuf_list_free(mi->p_hdr_data);
> + strbuf_list_free(mi->s_hdr_data);
Is it safe to feed NULL to the helper?
void strbuf_list_free(struct strbuf **sbs)
{
struct strbuf **s = sbs;
while (*s) {
strbuf_release(*s);
free(*s++);
}
free(sbs);
}
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo
2021-04-11 11:43 ` Junio C Hamano
@ 2021-04-25 13:15 ` Andrzej Hunt
0 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt @ 2021-04-25 13:15 UTC (permalink / raw)
To: Junio C Hamano, Andrzej Hunt via GitGitGadget; +Cc: git, Andrzej Hunt
On 11/04/2021 13:43, Junio C Hamano wrote:
> "Andrzej Hunt via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> void clear_mailinfo(struct mailinfo *mi)
>> {
>> - int i;
>> -
>> strbuf_release(&mi->name);
>> strbuf_release(&mi->email);
>> strbuf_release(&mi->charset);
>> strbuf_release(&mi->inbody_header_accum);
>> free(mi->message_id);
>>
>> - if (mi->p_hdr_data)
>> - for (i = 0; mi->p_hdr_data[i]; i++)
>> - strbuf_release(mi->p_hdr_data[i]);
>> - free(mi->p_hdr_data);
>> - if (mi->s_hdr_data)
>> - for (i = 0; mi->s_hdr_data[i]; i++)
>> - strbuf_release(mi->s_hdr_data[i]);
>> - free(mi->s_hdr_data);
>
> So, the original allows mi->p_hdr_data to be NULL and does not do
> this freeing (the same for the .s_hdr_data member).
>
>> + strbuf_list_free(mi->p_hdr_data);
>> + strbuf_list_free(mi->s_hdr_data);
>
> Is it safe to feed NULL to the helper?
>
> void strbuf_list_free(struct strbuf **sbs)
> {
> struct strbuf **s = sbs;
>
> while (*s) {
> strbuf_release(*s);
> free(*s++);
> }
> free(sbs);
> }
>
>
Indeed: AFAIUI dereferencing NULL is undefined behaviour. I think the
best solution is to add a NULL check in strbuf_list_free() - which is
the pattern I've seen in several other *_free() helpers (there are also
quite a few examples of *_free() helpers that are not NULL safe, but
IMHO having a NULL check will lead to fewer unpleasant surprises).
Incidentally I did run the entire test-suite against UBSAN, and it
didn't find any issues here. This seems like something that UBSAN should
be able to easily catch, so we probably don't have any tests exercising
clear_mailinfo() with NULL p_hdr_info/s_hdr_info?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 03/12] ls-files: free max_prefix when done
2021-04-10 8:12 ` René Scharfe
@ 2021-04-25 13:16 ` Andrzej Hunt
0 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt @ 2021-04-25 13:16 UTC (permalink / raw)
To: René Scharfe, Andrzej Hunt via GitGitGadget, git; +Cc: Andrzej Hunt
On 10/04/2021 10:12, René Scharfe wrote:
> Am 09.04.21 um 20:47 schrieb Andrzej Hunt via GitGitGadget:
>> From: Andrzej Hunt <ajrhunt@google.com>
>> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
>> index 60a2913a01e9..53e20bbf9cce 100644
>> --- a/builtin/ls-files.c
>> +++ b/builtin/ls-files.c
>> @@ -781,5 +781,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
>> }
>>
>> dir_clear(&dir);
>> + free((void *)max_prefix);
>
> This cast is necessary to ignore the const attribute of the pointer.
> It's scary, but safe here because this function owns the referenced
> object.
>
> I think the promise to not modify the string given at the top of the
> function is not worth having to take back that promise forcefully at
> the end to dispose of it. Determining the correctness of this cast
> requires reading the whole function. Removing the const from the
> declaration (and the cast) would improve readability overall. Thoughts?
I agree - I'll change this in V2 V2. In fact, Peff already given the
following explanation for why non-const is preferred in this scenario on
a previous patch of mine (which I failed to heed when preparing this patch):
> If a variable is meant to take ownership of memory, our usual
> convention is to not declare it as "const"."
https://lore.kernel.org/git/YEZ0jLppB9wOg%2Faf@coredump.intra.peff.net/
>
>> return 0;
>> }
>>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 04/12] bloom: clear each bloom_key after use
2021-04-11 7:26 ` SZEDER Gábor
@ 2021-04-25 13:17 ` Andrzej Hunt
0 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt @ 2021-04-25 13:17 UTC (permalink / raw)
To: SZEDER Gábor, Andrzej Hunt via GitGitGadget; +Cc: git, Andrzej Hunt
On 11/04/2021 09:26, SZEDER Gábor wrote:
> On Fri, Apr 09, 2021 at 06:47:23PM +0000, Andrzej Hunt via GitGitGadget wrote:
>> From: Andrzej Hunt <ajrhunt@google.com>
>>
>> fill_bloom_key() allocates memory into bloom_key, we need to clean that
>> up once the key is no longer needed.
>>
>> This fixes the following leak which was found while running t0002-t0099.
>> Although this leak is happening in code being called from a test-helper,
>> the same code is also used in various locations around git, and could
>> presumably happen during normal usage too.
>
> It does indeed happen: 'git commit-graph write --reachable
> --changed-paths' generates Bloom filters for every commit, with each
> filter containing all paths modified by its associated commit, so it
> leaks a lot of 7 * 4byte hashes. This patch reduces the memory usage
> of that command:
>
> Max RSS
> before after
> ---------------------------------------------
> android-base 1275028k 1006576k -21.1%
> chromium 3245144k 3127764k -3.6%
> cmssw 793996k 699156k -12.0%
> cpython 371584k 343480k -7.6%
> elasticsearch 748104k 637936k -14.7%
> freebsd-src 819020k 741272k -9.5%
> gcc 867412k 730332k -15.8%
> gecko-dev 2619112k 2457280k -6.2%
> git 252684k 216900k -14.2%
> glibc 239000k 222228k -7.0%
> go 264132k 251344k -4.9%
> homebrew-cask 542188k 480588k -11.4%
> homebrew-core 805332k 715848k -11.1%
> jdk 417832k 342928k -17.9%
> libreoff-core 1257296k 1089980k -13.3%
> linux 2033296k 1759712k -13.5%
> llvm-project 1067216k 956704k -10.4%
> mariadb-srv 695172k 559508k -19.5%
> postgres 340132k 317416k -6.7%
> rails 325432k 294332k -9.6%
> rust 655244k 584904k -10.7%
> tensorflow 507308k 480848k -5.2%
> webkit 2466812k 2237332k -9.3%
>
> Just out of curiosity, I disabled the questionable hardcoded 512 paths
> limit on the size of modified path Bloom filters, and the memory usage
> in the jdk repository sunk by over 55%, from 849520k to 379760k.
>
> Please feel free to include any of the above data points in the commit
> message.
Thank you for the detailed analysis - these kinds of results are very
motivating! I will include a brief summary (something like "10% typical
improvement for 'commit-graph write' for large repos") along with a link
to your posting for those who want the full picture.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 01/12] revision: free remainder of old commit list in limit_list
2021-04-10 7:29 ` René Scharfe
@ 2021-04-25 13:32 ` Andrzej Hunt
0 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt @ 2021-04-25 13:32 UTC (permalink / raw)
To: René Scharfe, Andrzej Hunt via GitGitGadget, git; +Cc: Andrzej Hunt
On 10/04/2021 09:29, René Scharfe wrote:
> Am 09.04.21 um 20:47 schrieb Andrzej Hunt via GitGitGadget:
>> From: Andrzej Hunt <ajrhunt@google.com>
>>
>> limit_list() iterates over the original revs->commits list, and consumes
>> many of its entries via pop_commit. However we might stop iterating over
>> the list early (e.g. if we realise that the rest of the list is
>> uninteresting). If we do stop iterating early, list will be pointing to
>> the unconsumed portion of revs->commits - and we need to free this list
>> to avoid a leak. (revs->commits itself will be an invalid pointer: it
>> will have been free'd during the first pop_commit.)
>>
>> This leak was found while running t0090. It's not likely to be very
>> impactful, but it can happen quite early during some checkout
>> invocations, and hence seems to be worth fixing:
>>
>> Direct leak of 16 byte(s) in 1 object(s) allocated from:
>> #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
>> #1 0x9ac084 in do_xmalloc wrapper.c:41:8
>> #2 0x9ac05a in xmalloc wrapper.c:62:9
>> #3 0x7175d6 in commit_list_insert commit.c:540:33
>> #4 0x71800f in commit_list_insert_by_date commit.c:604:9
>> #5 0x8f8d2e in process_parents revision.c:1128:5
>> #6 0x8f2f2c in limit_list revision.c:1418:7
>> #7 0x8f210e in prepare_revision_walk revision.c:3577:7
>> #8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
>> #9 0x512f05 in switch_branches builtin/checkout.c:1250:3
>> #10 0x50f8de in checkout_branch builtin/checkout.c:1646:9
>> #11 0x50ba12 in checkout_main builtin/checkout.c:2003:9
>> #12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
>> #13 0x4cd91d in run_builtin git.c:467:11
>> #14 0x4cb5f3 in handle_builtin git.c:719:3
>> #15 0x4ccf47 in run_argv git.c:808:4
>> #16 0x4caf49 in cmd_main git.c:939:19
>> #17 0x69dc0e in main common-main.c:52:11
>> #18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>>
>> Indirect leak of 48 byte(s) in 3 object(s) allocated from:
>> #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
>> #1 0x9ac084 in do_xmalloc wrapper.c:41:8
>> #2 0x9ac05a in xmalloc wrapper.c:62:9
>> #3 0x717de6 in commit_list_append commit.c:1609:35
>> #4 0x8f1f9b in prepare_revision_walk revision.c:3554:12
>> #5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
>> #6 0x512f05 in switch_branches builtin/checkout.c:1250:3
>> #7 0x50f8de in checkout_branch builtin/checkout.c:1646:9
>> #8 0x50ba12 in checkout_main builtin/checkout.c:2003:9
>> #9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
>> #10 0x4cd91d in run_builtin git.c:467:11
>> #11 0x4cb5f3 in handle_builtin git.c:719:3
>> #12 0x4ccf47 in run_argv git.c:808:4
>> #13 0x4caf49 in cmd_main git.c:939:19
>> #14 0x69dc0e in main common-main.c:52:11
>> #15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>>
>> Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
>> ---
>> revision.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/revision.c b/revision.c
>> index 553c0faa9b38..7b509aab0c87 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -1460,6 +1460,7 @@ static int limit_list(struct rev_info *revs)
>> update_treesame(revs, c);
>> }
>>
>> + free_commit_list(list);
>
> This patch would benefit from more context, but this function is quite
> long. So let me sketch it:
>
> struct commit_list *list = revs->commits;
>
> while (list) {
> struct commit *commit = pop_commit(&list);
> struct object *obj = &commit->object;
>
> if (obj->flags & UNINTERESTING) {
> break;
> }
> }
>
> if (limiting_can_increase_treesame(revs))
> for (list = newlist; list; list = list->next) {
> }
>
> free_commit_list(list);
>
> So the while loop can leave list dangling and you want to free its
> remaining entries. The for loop sometimes overwrites the list pointer,
> though, and you will end up passing NULL to free_commit_list in that
> case. So either the call should be moved between the loops or a fresh
> variable should be used in the second loop instead of reusing list to
> make sure the entries are released in all cases.
Good catch, I did not look closely enough at this one - V1 definitely is
buggy*. I've decided I'll add a new variable for the list in V2, but I
also took the opportunity to rename the original list since I think that
makes it more obvious where that list came from in the first place.
* However I also didn't run into any failures when running the entire
test-suite with this change, so I'm guessing this codepath isn't being
exercised by our tests. I'm hoping to try and investigate this in more
detail when I find a spare moment.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (11 preceding siblings ...)
2021-04-09 18:47 ` [PATCH 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
` (11 more replies)
12 siblings, 12 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt
V2 addresses all the issues brought up during review, and adds a link to
Gabor's analysis of the bloom filter changes.
Andrzej Hunt (12):
revision: free remainder of old commit list in limit_list
wt-status: fix multiple small leaks
ls-files: free max_prefix when done
bloom: clear each bloom_key after use
branch: FREE_AND_NULL instead of NULL'ing real_ref
builtin/bugreport: don't leak prefixed filename
builtin/check-ignore: clear_pathspec before returning
builtin/checkout: clear pending objects after diffing
mailinfo: also free strbuf lists when clearing mailinfo
builtin/for-each-ref: free filter and UNLEAK sorting.
builtin/rebase: release git_format_patch_opt too
builtin/rm: avoid leaking pathspec and seen
bloom.c | 1 +
branch.c | 2 +-
builtin/bugreport.c | 8 +++++---
builtin/check-ignore.c | 1 +
builtin/checkout.c | 1 +
builtin/for-each-ref.c | 3 +++
builtin/ls-files.c | 3 ++-
builtin/rebase.c | 1 +
builtin/rm.c | 2 ++
mailinfo.c | 14 +++-----------
revision.c | 17 ++++++++++-------
strbuf.c | 2 ++
wt-status.c | 4 ++++
13 files changed, 36 insertions(+), 23 deletions(-)
base-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-929%2Fahunt%2Fleaksan-100-part1-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-929/ahunt/leaksan-100-part1-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/929
Range-diff vs v1:
1: 12f0dcaef109 ! 1: d97307edca42 revision: free remainder of old commit list in limit_list
@@ Commit message
to avoid a leak. (revs->commits itself will be an invalid pointer: it
will have been free'd during the first pop_commit.)
+ However the list pointer is later reused to iterate over our new list,
+ but only for the limiting_can_increase_treesame() branch. We therefore
+ need to introduce a new variable for that branch - and while we're here
+ we can rename the original list to original_list as that makes its
+ purpose more obvious.
+
This leak was found while running t0090. It's not likely to be very
impactful, but it can happen quite early during some checkout
invocations, and hence seems to be worth fixing:
@@ Commit message
## revision.c ##
@@ revision.c: static int limit_list(struct rev_info *revs)
+ {
+ int slop = SLOP;
+ timestamp_t date = TIME_MAX;
+- struct commit_list *list = revs->commits;
++ struct commit_list *original_list = revs->commits;
+ struct commit_list *newlist = NULL;
+ struct commit_list **p = &newlist;
+ struct commit_list *bottom = NULL;
+ struct commit *interesting_cache = NULL;
+
+ if (revs->ancestry_path) {
+- bottom = collect_bottom_commits(list);
++ bottom = collect_bottom_commits(original_list);
+ if (!bottom)
+ die("--ancestry-path given but there are no bottom commits");
+ }
+
+- while (list) {
+- struct commit *commit = pop_commit(&list);
++ while (original_list) {
++ struct commit *commit = pop_commit(&original_list);
+ struct object *obj = &commit->object;
+ show_early_output_fn_t show;
+
+@@ revision.c: static int limit_list(struct rev_info *revs)
+
+ if (revs->max_age != -1 && (commit->date < revs->max_age))
+ obj->flags |= UNINTERESTING;
+- if (process_parents(revs, commit, &list, NULL) < 0)
++ if (process_parents(revs, commit, &original_list, NULL) < 0)
+ return -1;
+ if (obj->flags & UNINTERESTING) {
+ mark_parents_uninteresting(commit);
+- slop = still_interesting(list, date, slop, &interesting_cache);
++ slop = still_interesting(original_list, date, slop, &interesting_cache);
+ if (slop)
+ continue;
+ break;
+@@ revision.c: static int limit_list(struct rev_info *revs)
+ * Check if any commits have become TREESAME by some of their parents
+ * becoming UNINTERESTING.
+ */
+- if (limiting_can_increase_treesame(revs))
++ if (limiting_can_increase_treesame(revs)) {
++ struct commit_list *list = NULL;
+ for (list = newlist; list; list = list->next) {
+ struct commit *c = list->item;
+ if (c->object.flags & (UNINTERESTING | TREESAME))
+ continue;
update_treesame(revs, c);
}
++ }
-+ free_commit_list(list);
++ free_commit_list(original_list);
revs->commits = newlist;
return 0;
}
2: 716a21b4ef73 = 2: 9ad3d8e3fbf4 wt-status: fix multiple small leaks
3: beccdb177869 ! 3: 76519acdfee7 ls-files: free max_prefix when done
@@ Commit message
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
## builtin/ls-files.c ##
+@@ builtin/ls-files.c: static int option_parse_exclude_standard(const struct option *opt,
+ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
+ {
+ int require_work_tree = 0, show_tag = 0, i;
+- const char *max_prefix;
++ char *max_prefix;
+ struct dir_struct dir;
+ struct pattern_list *pl;
+ struct string_list exclude_list = STRING_LIST_INIT_NODUP;
@@ builtin/ls-files.c: int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
}
dir_clear(&dir);
-+ free((void *)max_prefix);
++ free(max_prefix);
return 0;
}
4: 9ae15b948813 ! 4: fb64a3dcd0b0 bloom: clear each bloom_key after use
@@ Commit message
fill_bloom_key() allocates memory into bloom_key, we need to clean that
up once the key is no longer needed.
- This fixes the following leak which was found while running t0002-t0099.
- Although this leak is happening in code being called from a test-helper,
- the same code is also used in various locations around git, and could
- presumably happen during normal usage too.
+ This leak was found while running t0002-t0099. Although this leak is
+ happening in code being called from a test-helper, the same code is also
+ used in various locations around git, and can therefore happen during
+ normal usage too. Gabor's analysis shows that peak-memory usage during
+ 'git commit-graph write' is reduced on the order of 10% for a selection
+ of larger repos (along with an even larger reduction if we override
+ modified path bloom filter limits):
+ https://lore.kernel.org/git/20210411072651.GF2947267@szeder.dev/
+
+ LSAN output:
Direct leak of 308 byte(s) in 11 object(s) allocated from:
#0 0x49a5e2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
5: 8c7ba2b83d5d = 5: 154c6714f305 branch: FREE_AND_NULL instead of NULL'ing real_ref
6: 24129e3e633d = 6: 0ae6224e01bc builtin/bugreport: don't leak prefixed filename
7: 563264af39c3 = 7: 693ea82490df builtin/check-ignore: clear_pathspec before returning
8: cdeb4b7875e3 = 8: 20c5f2e68c54 builtin/checkout: clear pending objects after diffing
9: 130ef89218a4 ! 9: 217f571f8ef5 mailinfo: also free strbuf lists when clearing mailinfo
@@ Commit message
array contents but want to reuse the array itself, hence we can't use
strbuf_list_free() there.
+ However, strbuf_list_free() cannot handle a NULL input, and the lists we
+ are freeing might be NULL. Therefore we add a NULL check in
+ strbuf_list_free() to make it safe to use with a NULL input (which is a
+ pattern used by some of the other *_free() functions around git).
+
Leak output from t0023:
Direct leak of 72 byte(s) in 3 object(s) allocated from:
@@ mailinfo.c: void setup_mailinfo(struct mailinfo *mi)
while (mi->content < mi->content_top) {
free(*(mi->content_top));
+
+ ## strbuf.c ##
+@@ strbuf.c: void strbuf_list_free(struct strbuf **sbs)
+ {
+ struct strbuf **s = sbs;
+
++ if (!s)
++ return;
+ while (*s) {
+ strbuf_release(*s);
+ free(*s++);
10: 8f2374ee899d = 10: c4363c212217 builtin/for-each-ref: free filter and UNLEAK sorting.
11: c17e296bcb14 = 11: a67168677477 builtin/rebase: release git_format_patch_opt too
12: db1b151e2a15 = 12: 703cd9656bf8 builtin/rm: avoid leaking pathspec and seen
--
gitgitgadget
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 01/12] revision: free remainder of old commit list in limit_list
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
` (10 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
limit_list() iterates over the original revs->commits list, and consumes
many of its entries via pop_commit. However we might stop iterating over
the list early (e.g. if we realise that the rest of the list is
uninteresting). If we do stop iterating early, list will be pointing to
the unconsumed portion of revs->commits - and we need to free this list
to avoid a leak. (revs->commits itself will be an invalid pointer: it
will have been free'd during the first pop_commit.)
However the list pointer is later reused to iterate over our new list,
but only for the limiting_can_increase_treesame() branch. We therefore
need to introduce a new variable for that branch - and while we're here
we can rename the original list to original_list as that makes its
purpose more obvious.
This leak was found while running t0090. It's not likely to be very
impactful, but it can happen quite early during some checkout
invocations, and hence seems to be worth fixing:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac084 in do_xmalloc wrapper.c:41:8
#2 0x9ac05a in xmalloc wrapper.c:62:9
#3 0x7175d6 in commit_list_insert commit.c:540:33
#4 0x71800f in commit_list_insert_by_date commit.c:604:9
#5 0x8f8d2e in process_parents revision.c:1128:5
#6 0x8f2f2c in limit_list revision.c:1418:7
#7 0x8f210e in prepare_revision_walk revision.c:3577:7
#8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
#9 0x512f05 in switch_branches builtin/checkout.c:1250:3
#10 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#11 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#13 0x4cd91d in run_builtin git.c:467:11
#14 0x4cb5f3 in handle_builtin git.c:719:3
#15 0x4ccf47 in run_argv git.c:808:4
#16 0x4caf49 in cmd_main git.c:939:19
#17 0x69dc0e in main common-main.c:52:11
#18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 48 byte(s) in 3 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac084 in do_xmalloc wrapper.c:41:8
#2 0x9ac05a in xmalloc wrapper.c:62:9
#3 0x717de6 in commit_list_append commit.c:1609:35
#4 0x8f1f9b in prepare_revision_walk revision.c:3554:12
#5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
#6 0x512f05 in switch_branches builtin/checkout.c:1250:3
#7 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#8 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
revision.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/revision.c b/revision.c
index 553c0faa9b38..568d73c0b331 100644
--- a/revision.c
+++ b/revision.c
@@ -1393,20 +1393,20 @@ static int limit_list(struct rev_info *revs)
{
int slop = SLOP;
timestamp_t date = TIME_MAX;
- struct commit_list *list = revs->commits;
+ struct commit_list *original_list = revs->commits;
struct commit_list *newlist = NULL;
struct commit_list **p = &newlist;
struct commit_list *bottom = NULL;
struct commit *interesting_cache = NULL;
if (revs->ancestry_path) {
- bottom = collect_bottom_commits(list);
+ bottom = collect_bottom_commits(original_list);
if (!bottom)
die("--ancestry-path given but there are no bottom commits");
}
- while (list) {
- struct commit *commit = pop_commit(&list);
+ while (original_list) {
+ struct commit *commit = pop_commit(&original_list);
struct object *obj = &commit->object;
show_early_output_fn_t show;
@@ -1415,11 +1415,11 @@ static int limit_list(struct rev_info *revs)
if (revs->max_age != -1 && (commit->date < revs->max_age))
obj->flags |= UNINTERESTING;
- if (process_parents(revs, commit, &list, NULL) < 0)
+ if (process_parents(revs, commit, &original_list, NULL) < 0)
return -1;
if (obj->flags & UNINTERESTING) {
mark_parents_uninteresting(commit);
- slop = still_interesting(list, date, slop, &interesting_cache);
+ slop = still_interesting(original_list, date, slop, &interesting_cache);
if (slop)
continue;
break;
@@ -1452,14 +1452,17 @@ static int limit_list(struct rev_info *revs)
* Check if any commits have become TREESAME by some of their parents
* becoming UNINTERESTING.
*/
- if (limiting_can_increase_treesame(revs))
+ if (limiting_can_increase_treesame(revs)) {
+ struct commit_list *list = NULL;
for (list = newlist; list; list = list->next) {
struct commit *c = list->item;
if (c->object.flags & (UNINTERESTING | TREESAME))
continue;
update_treesame(revs, c);
}
+ }
+ free_commit_list(original_list);
revs->commits = newlist;
return 0;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 02/12] wt-status: fix multiple small leaks
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
` (9 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
rev.prune_data is populated (in multiple functions) via copy_pathspec,
and therefore needs to be cleared after running the diff in those
functions.
rev(_info).pending is populated indirectly via setup_revisions, and also
needs to be cleared once diffing is done.
These leaks were found while running t0008 or t0021. The rev.prune_data
leaks are small (80B) but noisy, hence I won't bother including their
logs - the rev.pending leaks are bigger, and can happen early in the
course of other commands, and therefore possibly more valuable to fix -
see example log from a rebase below:
Direct leak of 2048 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9ac2a6 in xrealloc wrapper.c:126:8
#2 0x83da03 in add_object_array_with_path object.c:337:3
#3 0x8f5d8a in add_pending_object_with_path revision.c:329:2
#4 0x8ea50b in add_pending_object_with_mode revision.c:336:2
#5 0x8ea4fd in add_pending_object revision.c:342:2
#6 0x8ea610 in add_head_to_pending revision.c:354:2
#7 0x9b55f5 in has_uncommitted_changes wt-status.c:2474:2
#8 0x9b58c4 in require_clean_work_tree wt-status.c:2553:6
#9 0x606bcc in cmd_rebase builtin/rebase.c:1970:6
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7f2d18909349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 5 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ac048 in xstrdup wrapper.c:29:14
#2 0x83da8d in add_object_array_with_path object.c:349:17
#3 0x8f5d8a in add_pending_object_with_path revision.c:329:2
#4 0x8ea50b in add_pending_object_with_mode revision.c:336:2
#5 0x8ea4fd in add_pending_object revision.c:342:2
#6 0x8ea610 in add_head_to_pending revision.c:354:2
#7 0x9b55f5 in has_uncommitted_changes wt-status.c:2474:2
#8 0x9b58c4 in require_clean_work_tree wt-status.c:2553:6
#9 0x606bcc in cmd_rebase builtin/rebase.c:1970:6
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7f2d18909349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 2053 byte(s) leaked in 2 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
wt-status.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/wt-status.c b/wt-status.c
index 1aed68c43c26..34886655dbcc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -616,6 +616,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
copy_pathspec(&rev.prune_data, &s->pathspec);
run_diff_files(&rev, 0);
+ clear_pathspec(&rev.prune_data);
}
static void wt_status_collect_changes_index(struct wt_status *s)
@@ -652,6 +653,8 @@ static void wt_status_collect_changes_index(struct wt_status *s)
rev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;
copy_pathspec(&rev.prune_data, &s->pathspec);
run_diff_index(&rev, 1);
+ object_array_clear(&rev.pending);
+ clear_pathspec(&rev.prune_data);
}
static void wt_status_collect_changes_initial(struct wt_status *s)
@@ -2480,6 +2483,7 @@ int has_uncommitted_changes(struct repository *r,
diff_setup_done(&rev_info.diffopt);
result = run_diff_index(&rev_info, 1);
+ object_array_clear(&rev_info.pending);
return diff_result_code(&rev_info.diffopt, result);
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 03/12] ls-files: free max_prefix when done
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
` (8 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
common_prefix() returns a new string, which we store in max_prefix -
this string needs to be freed to avoid a leak. This leak is happening
in cmd_ls_files, hence is of no real consequence - an UNLEAK would be
just as good, but we might as well free the string properly.
Leak found while running t0002, see output below:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ab1b4 in do_xmalloc wrapper.c:41:8
#2 0x9ab248 in do_xmallocz wrapper.c:75:8
#3 0x9ab22a in xmallocz wrapper.c:83:9
#4 0x9ab2d7 in xmemdupz wrapper.c:99:16
#5 0x78d6a4 in common_prefix dir.c:191:15
#6 0x5aca48 in cmd_ls_files builtin/ls-files.c:669:16
#7 0x4cd92d in run_builtin git.c:453:11
#8 0x4cb5fa in handle_builtin git.c:704:3
#9 0x4ccf57 in run_argv git.c:771:4
#10 0x4caf49 in cmd_main git.c:902:19
#11 0x69ce2e in main common-main.c:52:11
#12 0x7f64d4d94349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/ls-files.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 60a2913a01e9..84448b360120 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -603,7 +603,7 @@ static int option_parse_exclude_standard(const struct option *opt,
int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
{
int require_work_tree = 0, show_tag = 0, i;
- const char *max_prefix;
+ char *max_prefix;
struct dir_struct dir;
struct pattern_list *pl;
struct string_list exclude_list = STRING_LIST_INIT_NODUP;
@@ -781,5 +781,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
}
dir_clear(&dir);
+ free(max_prefix);
return 0;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 04/12] bloom: clear each bloom_key after use
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (2 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
` (7 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
fill_bloom_key() allocates memory into bloom_key, we need to clean that
up once the key is no longer needed.
This leak was found while running t0002-t0099. Although this leak is
happening in code being called from a test-helper, the same code is also
used in various locations around git, and can therefore happen during
normal usage too. Gabor's analysis shows that peak-memory usage during
'git commit-graph write' is reduced on the order of 10% for a selection
of larger repos (along with an even larger reduction if we override
modified path bloom filter limits):
https://lore.kernel.org/git/20210411072651.GF2947267@szeder.dev/
LSAN output:
Direct leak of 308 byte(s) in 11 object(s) allocated from:
#0 0x49a5e2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x6f4032 in xcalloc wrapper.c:140:8
#2 0x4f2905 in fill_bloom_key bloom.c:137:28
#3 0x4f34c1 in get_or_compute_bloom_filter bloom.c:284:4
#4 0x4cb484 in get_bloom_filter_for_commit t/helper/test-bloom.c:43:11
#5 0x4cb072 in cmd__bloom t/helper/test-bloom.c:97:3
#6 0x4ca7ef in cmd_main t/helper/test-tool.c:121:11
#7 0x4caace in main common-main.c:52:11
#8 0x7f798af95349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 308 byte(s) leaked in 11 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
bloom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/bloom.c b/bloom.c
index 52b87474c6eb..5e297038bb1f 100644
--- a/bloom.c
+++ b/bloom.c
@@ -283,6 +283,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
struct bloom_key key;
fill_bloom_key(e->path, strlen(e->path), &key, settings);
add_key_to_filter(&key, filter, settings);
+ clear_bloom_key(&key);
}
cleanup:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (3 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
` (6 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
real_ref was previously populated by dwim_ref(), which allocates new
memory. We need to make sure to free real_ref when discarding it.
(real_ref is already being freed at the end of create_branch() - but
if we discard it early then it will leak.)
This fixes the following leak found while running t0002-t0099:
Direct leak of 5 byte(s) in 1 object(s) allocated from:
#0 0x486954 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xdd6484 in xstrdup wrapper.c:29:14
#2 0xc0f658 in expand_ref refs.c:671:12
#3 0xc0ecf1 in repo_dwim_ref refs.c:644:22
#4 0x8b1184 in dwim_ref ./refs.h:162:9
#5 0x8b0b02 in create_branch branch.c:284:10
#6 0x550cbb in update_refs_for_switch builtin/checkout.c:1046:4
#7 0x54e275 in switch_branches builtin/checkout.c:1274:2
#8 0x548828 in checkout_branch builtin/checkout.c:1668:9
#9 0x541306 in checkout_main builtin/checkout.c:2025:9
#10 0x5395fa in cmd_checkout builtin/checkout.c:2077:8
#11 0x4d02a8 in run_builtin git.c:467:11
#12 0x4cbfe9 in handle_builtin git.c:719:3
#13 0x4cf04f in run_argv git.c:808:4
#14 0x4cb85a in cmd_main git.c:939:19
#15 0x820cf6 in main common-main.c:52:11
#16 0x7f30bd9dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
branch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/branch.c b/branch.c
index b71a2de29dbe..2260325d58c0 100644
--- a/branch.c
+++ b/branch.c
@@ -294,7 +294,7 @@ void create_branch(struct repository *r,
if (explicit_tracking)
die(_(upstream_not_branch), start_name);
else
- real_ref = NULL;
+ FREE_AND_NULL(real_ref);
}
break;
default:
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 06/12] builtin/bugreport: don't leak prefixed filename
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (4 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
` (5 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
prefix_filename() returns newly allocated memory, and strbuf_addstr()
doesn't take ownership of its inputs. Therefore we have to make sure to
store and free prefix_filename()'s result.
As this leak is in cmd_bugreport(), we could just as well UNLEAK the
prefix - but there's no good reason not to just free it properly. This
leak was found while running t0091, see output below:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc66 in xrealloc wrapper.c:126:8
#2 0x93baed in strbuf_grow strbuf.c:98:2
#3 0x93c6ea in strbuf_add strbuf.c:295:2
#4 0x69f162 in strbuf_addstr ./strbuf.h:304:2
#5 0x69f083 in prefix_filename abspath.c:277:2
#6 0x4fb275 in cmd_bugreport builtin/bugreport.c:146:9
#7 0x4cd91d in run_builtin git.c:467:11
#8 0x4cb5f3 in handle_builtin git.c:719:3
#9 0x4ccf47 in run_argv git.c:808:4
#10 0x4caf49 in cmd_main git.c:939:19
#11 0x69df9e in main common-main.c:52:11
#12 0x7f523a987349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/bugreport.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/bugreport.c b/builtin/bugreport.c
index ad3cc9c02f62..9915a5841def 100644
--- a/builtin/bugreport.c
+++ b/builtin/bugreport.c
@@ -129,6 +129,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
char *option_output = NULL;
char *option_suffix = "%Y-%m-%d-%H%M";
const char *user_relative_path = NULL;
+ char *prefixed_filename;
const struct option bugreport_options[] = {
OPT_STRING('o', "output-directory", &option_output, N_("path"),
@@ -142,9 +143,9 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
bugreport_usage, 0);
/* Prepare the path to put the result */
- strbuf_addstr(&report_path,
- prefix_filename(prefix,
- option_output ? option_output : ""));
+ prefixed_filename = prefix_filename(prefix,
+ option_output ? option_output : "");
+ strbuf_addstr(&report_path, prefixed_filename);
strbuf_complete(&report_path, '/');
strbuf_addstr(&report_path, "git-bugreport-");
@@ -189,6 +190,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix)
fprintf(stderr, _("Created new report at '%s'.\n"),
user_relative_path);
+ free(prefixed_filename);
UNLEAK(buffer);
UNLEAK(report_path);
return !!launch_editor(report_path.buf, NULL, NULL);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 07/12] builtin/check-ignore: clear_pathspec before returning
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (5 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
` (4 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
parse_pathspec() allocates new memory into pathspec, therefore we need
to free it when we're done.
An UNLEAK would probably be just as good here - but clear_pathspec() is
not much more work so we might as well use it. check_ignore() is either
called once directly from cmd_check_ignore() (in which case the leak
really doesnt matter), or it can be called multiple times in a loop from
check_ignore_stdin_paths(), in which case we're potentially leaking
multiple times - but even in this scenario the leak is so small as to
have no real consequence.
Found while running t0008:
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9aca44 in do_xmalloc wrapper.c:41:8
#2 0x9aca1a in xmalloc wrapper.c:62:9
#3 0x873c17 in parse_pathspec pathspec.c:582:2
#4 0x503eb8 in check_ignore builtin/check-ignore.c:90:2
#5 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17
#6 0x4cd91d in run_builtin git.c:467:11
#7 0x4cb5f3 in handle_builtin git.c:719:3
#8 0x4ccf47 in run_argv git.c:808:4
#9 0x4caf49 in cmd_main git.c:939:19
#10 0x69e43e in main common-main.c:52:11
#11 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc46 in xrealloc wrapper.c:126:8
#2 0x93baed in strbuf_grow strbuf.c:98:2
#3 0x93d696 in strbuf_vaddf strbuf.c:392:3
#4 0x9400c6 in xstrvfmt strbuf.c:979:2
#5 0x940253 in xstrfmt strbuf.c:989:8
#6 0x92b72a in prefix_path_gently setup.c:115:15
#7 0x87442d in init_pathspec_item pathspec.c:439:11
#8 0x873cef in parse_pathspec pathspec.c:589:3
#9 0x503eb8 in check_ignore builtin/check-ignore.c:90:2
#10 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17
#11 0x4cd91d in run_builtin git.c:467:11
#12 0x4cb5f3 in handle_builtin git.c:719:3
#13 0x4ccf47 in run_argv git.c:808:4
#14 0x4caf49 in cmd_main git.c:939:19
#15 0x69e43e in main common-main.c:52:11
#16 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ac9e8 in xstrdup wrapper.c:29:14
#2 0x874542 in init_pathspec_item pathspec.c:468:20
#3 0x873cef in parse_pathspec pathspec.c:589:3
#4 0x503eb8 in check_ignore builtin/check-ignore.c:90:2
#5 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17
#6 0x4cd91d in run_builtin git.c:467:11
#7 0x4cb5f3 in handle_builtin git.c:719:3
#8 0x4ccf47 in run_argv git.c:808:4
#9 0x4caf49 in cmd_main git.c:939:19
#10 0x69e43e in main common-main.c:52:11
#11 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 179 byte(s) leaked in 3 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/check-ignore.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 3c652748d58c..467e92cc7b80 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -118,6 +118,7 @@ static int check_ignore(struct dir_struct *dir,
num_ignored++;
}
free(seen);
+ clear_pathspec(&pathspec);
return num_ignored;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 08/12] builtin/checkout: clear pending objects after diffing
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (6 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
` (3 subsequent siblings)
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.
This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:
Direct leak of 2048 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc46 in xrealloc wrapper.c:126:8
#2 0x83e3a3 in add_object_array_with_path object.c:337:3
#3 0x8f672a in add_pending_object_with_path revision.c:329:2
#4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
#5 0x8eae9d in add_pending_object revision.c:342:2
#6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
#7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#12 0x4cd91d in run_builtin git.c:467:11
#13 0x4cb5f3 in handle_builtin git.c:719:3
#14 0x4ccf47 in run_argv git.c:808:4
#15 0x4caf49 in cmd_main git.c:939:19
#16 0x69e43e in main common-main.c:52:11
#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/checkout.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4c696ef4805b..190153c81571 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -602,6 +602,7 @@ static void show_local_changes(struct object *head,
diff_setup_done(&rev.diffopt);
add_pending_object(&rev, head, NULL);
run_diff_index(&rev, 0);
+ object_array_clear(&rev.pending);
}
static void describe_detached_head(const char *msg, struct commit *commit)
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 09/12] mailinfo: also free strbuf lists when clearing mailinfo
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (7 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-28 0:43 ` Junio C Hamano
2021-04-25 14:16 ` [PATCH v2 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
` (2 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
mailinfo.p_hdr_info/s_hdr_info are null-terminated lists of strbuf's,
with entries pointing either to NULL or an allocated strbuf. Therefore
we need to free those strbuf's (and not just the data they contain)
whenever we're done with a given entry. (See handle_header() where those
new strbufs are malloc'd.)
Once we no longer need the list (and not just its entries) we can switch
over to strbuf_list_free() instead of manually iterating over the list,
which takes care of those additional details for us. We can only do this
in clear_mailinfo() - in handle_commit_message() we are only clearing the
array contents but want to reuse the array itself, hence we can't use
strbuf_list_free() there.
However, strbuf_list_free() cannot handle a NULL input, and the lists we
are freeing might be NULL. Therefore we add a NULL check in
strbuf_list_free() to make it safe to use with a NULL input (which is a
pattern used by some of the other *_free() functions around git).
Leak output from t0023:
Direct leak of 72 byte(s) in 3 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac9f4 in do_xmalloc wrapper.c:41:8
#2 0x9ac9ca in xmalloc wrapper.c:62:9
#3 0x7f6cf7 in handle_header mailinfo.c:205:10
#4 0x7f5abf in check_header mailinfo.c:583:4
#5 0x7f5524 in mailinfo mailinfo.c:1197:3
#6 0x4dcc95 in parse_mail builtin/am.c:1167:6
#7 0x4d9070 in am_run builtin/am.c:1732:12
#8 0x4d5b7a in cmd_am builtin/am.c:2398:3
#9 0x4cd91d in run_builtin git.c:467:11
#10 0x4cb5f3 in handle_builtin git.c:719:3
#11 0x4ccf47 in run_argv git.c:808:4
#12 0x4caf49 in cmd_main git.c:939:19
#13 0x69e43e in main common-main.c:52:11
#14 0x7fc1fadfa349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 72 byte(s) leaked in 3 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
mailinfo.c | 14 +++-----------
strbuf.c | 2 ++
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/mailinfo.c b/mailinfo.c
index 5681d9130db6..95ce191f385b 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -821,7 +821,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
for (i = 0; header[i]; i++) {
if (mi->s_hdr_data[i])
strbuf_release(mi->s_hdr_data[i]);
- mi->s_hdr_data[i] = NULL;
+ FREE_AND_NULL(mi->s_hdr_data[i]);
}
return 0;
}
@@ -1236,22 +1236,14 @@ void setup_mailinfo(struct mailinfo *mi)
void clear_mailinfo(struct mailinfo *mi)
{
- int i;
-
strbuf_release(&mi->name);
strbuf_release(&mi->email);
strbuf_release(&mi->charset);
strbuf_release(&mi->inbody_header_accum);
free(mi->message_id);
- if (mi->p_hdr_data)
- for (i = 0; mi->p_hdr_data[i]; i++)
- strbuf_release(mi->p_hdr_data[i]);
- free(mi->p_hdr_data);
- if (mi->s_hdr_data)
- for (i = 0; mi->s_hdr_data[i]; i++)
- strbuf_release(mi->s_hdr_data[i]);
- free(mi->s_hdr_data);
+ strbuf_list_free(mi->p_hdr_data);
+ strbuf_list_free(mi->s_hdr_data);
while (mi->content < mi->content_top) {
free(*(mi->content_top));
diff --git a/strbuf.c b/strbuf.c
index e3397cc4c72a..4df30b45494d 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -209,6 +209,8 @@ void strbuf_list_free(struct strbuf **sbs)
{
struct strbuf **s = sbs;
+ if (!s)
+ return;
while (*s) {
strbuf_release(*s);
free(*s++);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 10/12] builtin/for-each-ref: free filter and UNLEAK sorting.
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (8 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
sorting might be a list allocated in ref_default_sorting() (in this case
it's a fixed single item list, which has nevertheless been xcalloc'd),
or it might be a list allocated in parse_opt_ref_sorting(). In either
case we could free these lists - but instead we UNLEAK as we're at the
end of cmd_for_each_ref. (There's no existing implementation of
clear_ref_sorting(), and writing a loop to free the list seems more
trouble than it's worth.)
filter.with_commit/no_commit are populated via
OPT_CONTAINS/OPT_NO_CONTAINS, both of which create new entries via
parse_opt_commits(), and also need to be free'd or UNLEAK'd. Because
free_commit_list() already exists, we choose to use that over an UNLEAK.
LSAN output from t0041:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a9d2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x9ac252 in xcalloc wrapper.c:140:8
#2 0x8a4a55 in ref_default_sorting ref-filter.c:2486:32
#3 0x56c6b1 in cmd_for_each_ref builtin/for-each-ref.c:72:13
#4 0x4cd91d in run_builtin git.c:467:11
#5 0x4cb5f3 in handle_builtin git.c:719:3
#6 0x4ccf47 in run_argv git.c:808:4
#7 0x4caf49 in cmd_main git.c:939:19
#8 0x69dabe in main common-main.c:52:11
#9 0x7f2bdc570349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9abf54 in do_xmalloc wrapper.c:41:8
#2 0x9abf2a in xmalloc wrapper.c:62:9
#3 0x717486 in commit_list_insert commit.c:540:33
#4 0x8644cf in parse_opt_commits parse-options-cb.c:98:2
#5 0x869bb5 in get_value parse-options.c:181:11
#6 0x8677dc in parse_long_opt parse-options.c:378:10
#7 0x8659bd in parse_options_step parse-options.c:817:11
#8 0x867fcd in parse_options parse-options.c:870:10
#9 0x56c62b in cmd_for_each_ref builtin/for-each-ref.c:59:2
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dabe in main common-main.c:52:11
#15 0x7f2bdc570349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/for-each-ref.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index cb9c81a04606..84efb71f82fc 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -83,5 +83,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
for (i = 0; i < maxcount; i++)
show_ref_array_item(array.items[i], &format);
ref_array_clear(&array);
+ free_commit_list(filter.with_commit);
+ free_commit_list(filter.no_commit);
+ UNLEAK(sorting);
return 0;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 11/12] builtin/rebase: release git_format_patch_opt too
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (9 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
options.git_format_patch_opt can be populated during cmd_rebase's setup,
and will therefore leak on return. Although we could just UNLEAK all of
options, we choose to strbuf_release() the individual member, which matches
the existing pattern (where we're freeing invidual members of options).
Leak found when running t0021:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9ac296 in xrealloc wrapper.c:126:8
#2 0x93b13d in strbuf_grow strbuf.c:98:2
#3 0x93bd3a in strbuf_add strbuf.c:295:2
#4 0x60ae92 in strbuf_addstr strbuf.h:304:2
#5 0x605f17 in cmd_rebase builtin/rebase.c:1759:3
#6 0x4cd91d in run_builtin git.c:467:11
#7 0x4cb5f3 in handle_builtin git.c:719:3
#8 0x4ccf47 in run_argv git.c:808:4
#9 0x4caf49 in cmd_main git.c:939:19
#10 0x69dbfe in main common-main.c:52:11
#11 0x7f66dae91349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/rebase.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index ed1da1760e4c..a756fba23330 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -2109,6 +2109,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
free(options.head_name);
free(options.gpg_sign_opt);
free(options.cmd);
+ strbuf_release(&options.git_format_patch_opt);
free(squash_onto_name);
return ret;
}
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH v2 12/12] builtin/rm: avoid leaking pathspec and seen
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
` (10 preceding siblings ...)
2021-04-25 14:16 ` [PATCH v2 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
@ 2021-04-25 14:16 ` Andrzej Hunt via GitGitGadget
11 siblings, 0 replies; 35+ messages in thread
From: Andrzej Hunt via GitGitGadget @ 2021-04-25 14:16 UTC (permalink / raw)
To: git; +Cc: René Scharfe, SZEDER Gábor, Andrzej Hunt, Andrzej Hunt
From: Andrzej Hunt <ajrhunt@google.com>
parse_pathspec() populates pathspec, hence we need to clear it once it's
no longer needed. seen is xcalloc'd within the same function and
likewise needs to be freed once its no longer needed.
cmd_rm() has multiple early returns, therefore we need to clear or free
as soon as this data is no longer needed, as opposed to doing a cleanup
at the end.
LSAN output from t0020:
Direct leak of 112 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac0a4 in do_xmalloc wrapper.c:41:8
#2 0x9ac07a in xmalloc wrapper.c:62:9
#3 0x873277 in parse_pathspec pathspec.c:582:2
#4 0x646ffa in cmd_rm builtin/rm.c:266:2
#5 0x4cd91d in run_builtin git.c:467:11
#6 0x4cb5f3 in handle_builtin git.c:719:3
#7 0x4ccf47 in run_argv git.c:808:4
#8 0x4caf49 in cmd_main git.c:939:19
#9 0x69dc0e in main common-main.c:52:11
#10 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9ac2a6 in xrealloc wrapper.c:126:8
#2 0x93b14d in strbuf_grow strbuf.c:98:2
#3 0x93ccf6 in strbuf_vaddf strbuf.c:392:3
#4 0x93f726 in xstrvfmt strbuf.c:979:2
#5 0x93f8b3 in xstrfmt strbuf.c:989:8
#6 0x92ad8a in prefix_path_gently setup.c:115:15
#7 0x873a8d in init_pathspec_item pathspec.c:439:11
#8 0x87334f in parse_pathspec pathspec.c:589:3
#9 0x646ffa in cmd_rm builtin/rm.c:266:2
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ac048 in xstrdup wrapper.c:29:14
#2 0x873ba2 in init_pathspec_item pathspec.c:468:20
#3 0x87334f in parse_pathspec pathspec.c:589:3
#4 0x646ffa in cmd_rm builtin/rm.c:266:2
#5 0x4cd91d in run_builtin git.c:467:11
#6 0x4cb5f3 in handle_builtin git.c:719:3
#7 0x4ccf47 in run_argv git.c:808:4
#8 0x4caf49 in cmd_main git.c:939:19
#9 0x69dc0e in main common-main.c:52:11
#10 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 1 byte(s) in 1 object(s) allocated from:
#0 0x49a9d2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x9ac392 in xcalloc wrapper.c:140:8
#2 0x647108 in cmd_rm builtin/rm.c:294:9
#3 0x4cd91d in run_builtin git.c:467:11
#4 0x4cb5f3 in handle_builtin git.c:719:3
#5 0x4ccf47 in run_argv git.c:808:4
#6 0x4caf49 in cmd_main git.c:939:19
#7 0x69dbfe in main common-main.c:52:11
#8 0x7f4fac1b0349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
builtin/rm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/rm.c b/builtin/rm.c
index 4858631e0f02..2927678d37b6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -327,6 +327,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!seen_any)
exit(0);
}
+ clear_pathspec(&pathspec);
+ free(seen);
if (!index_only)
submodules_absorb_gitdir_if_needed();
--
gitgitgadget
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH v2 09/12] mailinfo: also free strbuf lists when clearing mailinfo
2021-04-25 14:16 ` [PATCH v2 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
@ 2021-04-28 0:43 ` Junio C Hamano
0 siblings, 0 replies; 35+ messages in thread
From: Junio C Hamano @ 2021-04-28 0:43 UTC (permalink / raw)
To: Andrzej Hunt via GitGitGadget
Cc: git, René Scharfe, SZEDER Gábor, Andrzej Hunt,
Andrzej Hunt
"Andrzej Hunt via GitGitGadget" <gitgitgadget@gmail.com> writes:
> However, strbuf_list_free() cannot handle a NULL input, and the lists we
> are freeing might be NULL. Therefore we add a NULL check in
> strbuf_list_free() to make it safe to use with a NULL input (which is a
> pattern used by some of the other *_free() functions around git).
OK.
Thanks.
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2021-04-28 0:43 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09 18:47 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
2021-04-10 7:29 ` René Scharfe
2021-04-25 13:32 ` Andrzej Hunt
2021-04-09 18:47 ` [PATCH 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
2021-04-10 8:12 ` René Scharfe
2021-04-25 13:16 ` Andrzej Hunt
2021-04-09 18:47 ` [PATCH 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
2021-04-11 7:26 ` SZEDER Gábor
2021-04-25 13:17 ` Andrzej Hunt
2021-04-09 18:47 ` [PATCH 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
2021-04-11 11:43 ` Junio C Hamano
2021-04-25 13:15 ` Andrzej Hunt
2021-04-09 18:47 ` [PATCH 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
2021-04-09 18:47 ` [PATCH 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 1 Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 01/12] revision: free remainder of old commit list in limit_list Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 02/12] wt-status: fix multiple small leaks Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 03/12] ls-files: free max_prefix when done Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 04/12] bloom: clear each bloom_key after use Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 05/12] branch: FREE_AND_NULL instead of NULL'ing real_ref Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 06/12] builtin/bugreport: don't leak prefixed filename Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 07/12] builtin/check-ignore: clear_pathspec before returning Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 08/12] builtin/checkout: clear pending objects after diffing Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 09/12] mailinfo: also free strbuf lists when clearing mailinfo Andrzej Hunt via GitGitGadget
2021-04-28 0:43 ` Junio C Hamano
2021-04-25 14:16 ` [PATCH v2 10/12] builtin/for-each-ref: free filter and UNLEAK sorting Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 11/12] builtin/rebase: release git_format_patch_opt too Andrzej Hunt via GitGitGadget
2021-04-25 14:16 ` [PATCH v2 12/12] builtin/rm: avoid leaking pathspec and seen Andrzej Hunt via GitGitGadget
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).