Fork Sync #6
Closed
Fork Sync #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since 1d718a5 ("do not overwrite untracked symlinks", 2011-02-20), the comment on top of threaded_check_leading_path() is outdated and no longer reflects the behavior of this function. Let's updated it to avoid confusions. While we are here, also remove some duplicated comments to avoid similar maintenance problems. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
At 1d718a5 ("do not overwrite untracked symlinks", 2011-02-20), symlink.c:check_leading_path() started returning different codes for FL_ENOENT and FL_SYMLINK. But one of its callers, unlink_entry(), was not adjusted for this change, so it started to follow symlinks on the leading path of to-be-removed entries. Fix that and add a regression test. Note that since 1d718a5 check_leading_path() no longer differentiates the case where it found a symlink in the path's leading components from the cases where it found a regular file or failed to lstat() the component. So, a side effect of this current patch is that unlink_entry() now returns early in all of these three cases. And because we no longer try to unlink such paths, we also don't get the warning from remove_or_warn(). For the regular file and symlink cases, it's questionable whether the warning was useful in the first place: unlink_entry() removes tracked paths that should no longer be present in the state we are checking out to. If the path had its leading dir replaced by another file, it means that the basename already doesn't exist, so there is no need for a warning. Sure, we are leaving a regular file or symlink behind at the path's dirname, but this file is either untracked now (so again, no need to warn), or it will be replaced by a tracked file during the next phase of this checkout operation. As for failing to lstat() one of the leading components, the basename might still exist only we cannot unlink it (e.g. due to the lack of the required permissions). Since the user expect it to be removed (especially with checkout's --no-overlay option), add back the warning in this more relevant case. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Note on using Asciidoctor to build documentation suite. Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add missing tests for showing a tree with "git show". Let's test for showing a tree, two trees, and that doing so doesn't recurse. The only tests for this code added in 5d7eeee (git-show: grok blobs, trees and tags, too, 2006-12-14) were the tests in t7701-repack-unpack-unreachable.sh added in ccc1297 (repack: modify behavior of -A option to leave unreferenced objects unpacked, 2008-05-09). Let's add this common mode of operation to the "show" tests themselves. It's more obvious, and the tests in t7701-repack-unpack-unreachable.sh happily pass if we start buggily emitting trees recursively. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add tests for "ls-files --with-tree". There was effectively no coverage for any normal usage of this command, only the tests added in 54e1abc (Add test case for ls-files --with-tree, 2007-10-03) for an obscure bug. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since the read_tree() API was added around the same time as read_tree_recursive() in 94537c7 (Move "read_tree()" to "tree.c"[...], 2005-04-22) and b12ec37 ([PATCH] Teach read-tree about commit objects, 2005-04-20) things have gradually migrated over to the read_tree_recursive() version. Now builtin/ls-files.c is the last user of this code, let's move all the relevant code there. This allows for subsequent simplification of it, and an eventual move to read_tree_recursive(). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that read_tree() has been moved to ls-files.c we can get rid of the stage != 1 case that'll never happen. Let's not use read_tree_recursive() as a pass-through to pass "stage = 1" either. For now we'll pass an unused "stage = 0" for consistency with other read_tree_recursive() callers, that argument will be removed in a follow-up commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor away the read_tree() function into its only user, overlay_tree_on_index(). First, change read_one_entry_opt() to use the strbuf parameter read_tree_recursive() passes down in place. This finishes up a partial refactoring started in 6a0b0b6 (tree.c: update read_tree_recursive callback to pass strbuf as base, 2014-11-30). Moving the rest into overlay_tree_on_index() makes this index juggling we're doing easier to read. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "stage" variable being passed around in the archive code has only ever been an elaborate way to hardcode the value "0". This code was added in its original form in e4fbbfe (Add git-zip-tree, 2006-08-26), at which point a hardcoded "0" would be passed down through read_tree_recursive() to write_zip_entry(). It was then diligently added to the "struct directory" in ed22b41 (archive: support filtering paths with glob, 2014-09-21), but we were still not doing anything except passing it around as-is. Let's stop doing that in the code internal to archive.c, we'll still feed "0" to read_tree_recursive() itself, but won't use it. That we're providing it at all to read_tree_recursive() will be changed in a follow-up commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rename the static read_tree_1() function to read_tree_at(). This function works just like read_tree_recursive(), except you provide your own strbuf. This step doesn't make much sense now, but in follow-up commits I'll remove the base/baselen/stage arguments to read_tree_recursive(). At that point an anticipated in-tree user[1] for the old read_tree_recursive() couldn't provide a path to start the traversal. Let's give them a function to do so with an API that makes more sense for them, by taking a strbuf we should be able to avoid more casting and/or reallocations in the future. 1. https://lore.kernel.org/git/xmqqft106sok.fsf@gitster.g Signed-off-by: Junio C Hamano <gitster@pobox.com>
Simplify the signature of read_tree_recursive() to omit the "base", "baselen" and "stage" arguments. No callers of it use these parameters for anything anymore. The last function to call read_tree_recursive() with a non-"" path was read_tree_recursive() itself, but that was changed in ffd31f6 (Reimplement read_tree_recursive() using tree_entry_interesting(), 2011-03-25). The last user of the "stage" parameter went away in the last commit, and even that use was mere boilerplate. So let's remove those and rename the read_tree_recursive() function to just read_tree(). We had another read_tree() function that I've refactored away in preceding commits, since all in-tree users read trees recursively with a callback we can change the name to signify that this is the norm. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a test for --exit-code working with --no-index. There's no reason to suppose it wouldn't, but we weren't testing for it anywhere in our tests. Let's fix that blind spot. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the username and password are supplied in a url like this https://myuser:secret@git.exampe/myrepo.git and the server supports the negotiate authenticaten method, git does not fall back to basic auth and libcurl hardly tries to authenticate with the negotiate method. Stop using the Negotiate authentication method after the first failure because if it fails on the first try it will never succeed. Signed-off-by: Christopher Schenk <christopher@cschenk.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When "git diff --no-index X Y" is run the modes of the files being differ are normalized by canon_mode() in fill_filespec(). I recently broke that behavior in a patch of mine[1] which would pass all tests, or not, depending on the umask of the git.git checkout. Let's test for this explicitly. Arguably this should not be the behavior of "git diff --no-index". We aren't diffing our own objects or the index, so it might be useful to show mode differences between files. On the other hand diff(1) does not do that, and it would be needlessly distracting when e.g. diffing an extracted tar archive whose contents is the same, but whose file modes are different. 1. https://lore.kernel.org/git/20210316155829.31242-2-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the rebase.useBuiltin setting and the now-obsolete GIT_TEST_REBASE_USE_BUILTIN test flag. This was left in place after my d03ebd4 (rebase: remove the rebase.useBuiltin setting, 2019-03-18) to help anyone who'd used the experimental flag and wanted to know that it was the default, or that they should transition their test environment to use the builtin rebase unconditionally. It's been more than long enough for those users to get a headsup about this. So remove all the scaffolding that was left inplace after d03ebd4. I'm also removing the documentation entry, if anyone still has this left in their configuration they can do some source archaeology to figure out what it used to do, which makes more sense than exposing every git user reading the documentation to this legacy configuration switch. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
get-send-email currently makes the assumption that the 'sendemail-validate' hook exists inside of the repository. Since the introduction of 'core.hooksPath' configuration option in 867ad08 (hooks: allow customizing where the hook directory is, 2016-05-04), this is no longer true. Instead of assuming a hardcoded repo relative path, query git for the actual path of the hooks directory. Signed-off-by: Robert Foss <robert.foss@linaro.org> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove a stray "xb" I inadvertently introduced in 780aa0a (tests: remove last uses of GIT_TEST_GETTEXT_POISON=false, 2021-02-11). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The text says something called a "patch" is prepared one for each commit, it is suitable for e-mail submission, and "am" is the command to use it, but does not say what the "patch" really is. The description in the page also refers to the "three-dash" line, but it is unclear what it is, unless the reader is given a more detailed overview of what the "patch" is. Add a brief paragraph to give an overview of what the output looks like. Signed-off-by: Junio C Hamano <gitster@pobox.com>
As record_reused_object(offset, offset - hashfile_total(out)) said, reused_chunk.difference should be the offset of original packfile minus the offset of the generated packfile. But the comment presented an opposite way. Signed-off-by: Han Xin <hanxin.hx@alibaba-inc.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When "git checkout" removes a path that does not exist in the commit it is checking out, it wasn't careful enough not to follow symbolic links, which has been corrected. * mt/checkout-remove-nofollow: checkout: don't follow symlinks when removing entries symlinks: update comment on threaded_check_leading_path()
Doc update. * bs/asciidoctor-installation-hints: INSTALL: note on using Asciidoctor to build doc
Code simplification by removing support for a caller that is long gone. * ab/read-tree: tree.h API: simplify read_tree_recursive() signature tree.h API: expose read_tree_1() as read_tree_at() archive: stop passing "stage" through read_tree_recursive() ls-files: refactor away read_tree() ls-files: don't needlessly pass around stage variable tree.c API: move read_tree() into builtin/ls-files.c ls-files tests: add meaningful --with-tree tests show tests: add test for "git show <tree>"
More test coverage over "diff --no-index". * ab/diff-no-index-tests: diff --no-index tests: test mode normalization diff --no-index tests: add test for --exit-code
When accessing a server with a URL like https://user:pass@site/, we did not to fall back to the basic authentication with the credential material embedded in the URL after the "Negotiate" authentication failed. Now we do. * cs/http-use-basic-after-failed-negotiate: remote-curl: fall back to basic auth if Negotiate fails
Remove the final hint that we used to have a scripted "git rebase". * ab/remove-rebase-usebuiltin: rebase: remove transitory rebase.useBuiltin setting & env
"git send-email" learned to honor the core.hooksPath configuration. * rf/send-email-hookspath: git-send-email: Respect core.hooksPath setting
Comment update. * hx/pack-objects-chunk-comment: pack-objects: fix comment of reused_chunk.difference
Testfix. * ab/detox-gettext-tests: mktag tests: fix broken "&&" chain
Explain pieces of the format-patch output upfront before the rest of the documentation starts referring to them. * jc/doc-format-patch-clarify: format-patch: give an overview of what a "patch" message is
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
origin starts off pointing to somewhere within line, which is owned by
the caller. Later we might allocate a new string using xmemdupz() or
xstrfmt(). To avoid leaking these new strings, we introduce a to_free
pointer - which allows us to safely free the newly allocated string when
we're done (we cannot just free origin directly as it might still be
pointing to line).
LSAN output from t0090:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa71f49 in do_xmalloc wrapper.c:41:8
#2 0xa720b0 in do_xmallocz wrapper.c:75:8
#3 0xa720b0 in xmallocz wrapper.c:83:9
#4 0xa720b0 in xmemdupz wrapper.c:99:16
#5 0x8092ba in handle_line fmt-merge-msg.c:187:23
#6 0x8092ba in fmt_merge_msg fmt-merge-msg.c:666:7
#7 0x5ce2e6 in prepare_merge_message builtin/merge.c:1119:2
git#8 0x5ce2e6 in collect_parents builtin/merge.c:1215:3
git#9 0x5c9c1e in cmd_merge builtin/merge.c:1454:16
git#10 0x4ce83e in run_builtin git.c:475:11
git#11 0x4ccafe in handle_builtin git.c:729:3
git#12 0x4cb01c in run_argv git.c:818:4
git#13 0x4cb01c in cmd_main git.c:949:19
git#14 0x6b3fad in main common-main.c:52:11
git#15 0x7fb929620349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
realpath is only populated if we execute the git_work_tree_initialized
block. However that block also causes us to return early, meaning we
never actually release the strbuf in the case where we populated it.
Therefore we move all strbuf related code into the block to guarantee
that we can't leak it.
LSAN output from t0095:
Direct leak of 129 byte(s) in 1 object(s) allocated from:
#0 0x49a9b9 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x78f585 in xrealloc wrapper.c:126:8
#2 0x713ff4 in strbuf_grow strbuf.c:98:2
#3 0x713ff4 in strbuf_getcwd strbuf.c:597:3
#4 0x4f0c18 in strbuf_realpath_1 abspath.c:99:7
#5 0x5ae4a4 in set_git_work_tree environment.c:259:3
#6 0x6fdd8a in setup_discovered_git_dir setup.c:931:2
#7 0x6fdd8a in setup_git_directory_gently setup.c:1235:12
git#8 0x4cb50d in get_bloom_filter_for_commit t/helper/test-bloom.c:41:2
git#9 0x4cb50d in cmd__bloom t/helper/test-bloom.c:95:3
git#10 0x4caa1f in cmd_main t/helper/test-tool.c:124:11
git#11 0x4caded in main common-main.c:52:11
git#12 0x7f0869f02349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 129 byte(s) leaked in 1 allocation(s).
It looks like this leak has existed since realpath was first added to
set_git_work_tree() in:
3d7747e (real_path: remove unsafe API, 2020-03-10)
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
relative_url() populates sb. In the normal return path, its buffer is
detached using strbuf_detach(). However the early return path does
nothing with sb, which means that sb's memory is leaked - therefore
we add a release to avoid this leak.
The reset is also only necessary for the normal return path, hence we
move it down to after the early-return to avoid unnecessary work.
LSAN output from t0060:
Direct leak of 121 byte(s) in 1 object(s) allocated from:
#0 0x7f31246f28b0 in realloc (/usr/lib64/libasan.so.4+0xdc8b0)
#1 0x98d7d6 in xrealloc wrapper.c:126
#2 0x909a60 in strbuf_grow strbuf.c:98
#3 0x90bf00 in strbuf_vaddf strbuf.c:401
#4 0x90c321 in strbuf_addf strbuf.c:335
#5 0x5cb78d in relative_url builtin/submodule--helper.c:182
#6 0x5cbe46 in resolve_relative_url_test builtin/submodule--helper.c:248
#7 0x410dcd in run_builtin git.c:475
git#8 0x410dcd in handle_builtin git.c:729
git#9 0x414087 in run_argv git.c:818
git#10 0x414087 in cmd_main git.c:949
git#11 0x40e9ec in main common-main.c:52
git#12 0x7f3123c41349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 121 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
cmd_for_each_repo() copies argv into args (a strvec), which is later
passed into run_command_on_repo(), which in turn copies that strvec onto
the end of child.args. The initial copy is unnecessary (we never modify
args). We therefore choose to just pass argv directly into
run_command_on_repo(), which lets us avoid the copy and fixes the leak.
LSAN output from t0068:
Direct leak of 192 byte(s) in 1 object(s) allocated from:
#0 0x7f63bd4ab8b0 in realloc (/usr/lib64/libasan.so.4+0xdc8b0)
#1 0x98d7e6 in xrealloc wrapper.c:126
#2 0x916914 in strvec_push_nodup strvec.c:19
#3 0x916a6e in strvec_push strvec.c:26
#4 0x4be4eb in cmd_for_each_repo builtin/for-each-repo.c:49
#5 0x410dcd in run_builtin git.c:475
#6 0x410dcd in handle_builtin git.c:729
#7 0x414087 in run_argv git.c:818
git#8 0x414087 in cmd_main git.c:949
git#9 0x40e9ec in main common-main.c:52
git#10 0x7f63bc9fa349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 22 byte(s) in 2 object(s) allocated from:
#0 0x7f63bd445e30 in __interceptor_strdup (/usr/lib64/libasan.so.4+0x76e30)
#1 0x98d698 in xstrdup wrapper.c:29
#2 0x916a63 in strvec_push strvec.c:26
#3 0x4be4eb in cmd_for_each_repo builtin/for-each-repo.c:49
#4 0x410dcd in run_builtin git.c:475
#5 0x410dcd in handle_builtin git.c:729
#6 0x414087 in run_argv git.c:818
#7 0x414087 in cmd_main git.c:949
git#8 0x40e9ec in main common-main.c:52
git#9 0x7f63bc9fa349 in __libc_start_main (/lib64/libc.so.6+0x24349)
See also discussion about the original implementation below - this code
appears to have evolved from a callback explaining the double-strvec-copy
pattern, but there's no strong reason to keep that now:
https://lore.kernel.org/git/68bbeca5-314b-08ee-ef36-040e3f3814e9@gmail.com/
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
old_dir/new_dir are free()'d at the end of update_dir_rename_counts, however if we return early we'll never free those strings. Therefore we should move all new allocations after the possible early return, avoiding a leak. This seems like a fairly recent leak, that started happening since the early-return was added in: 1ad69eb (diffcore-rename: compute dir_rename_counts in stages, 2021-02-27) LSAN output from t0022: Direct leak of 7 byte(s) in 1 object(s) allocated from: #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0xa71e48 in xstrdup wrapper.c:29:14 #2 0x7db9c7 in update_dir_rename_counts diffcore-rename.c:464:12 #3 0x7db6ae in find_renames diffcore-rename.c:1062:3 #4 0x7d76c3 in diffcore_rename_extended diffcore-rename.c:1472:18 #5 0x7b4cfc in diffcore_std diff.c:6705:4 #6 0x855e46 in log_tree_diff_flush log-tree.c:846:2 #7 0x856574 in log_tree_diff log-tree.c:955:3 git#8 0x856574 in log_tree_commit log-tree.c:986:10 git#9 0x9a9c67 in print_commit_summary sequencer.c:1329:7 git#10 0x52e623 in cmd_commit builtin/commit.c:1862:3 git#11 0x4ce83e in run_builtin git.c:475:11 git#12 0x4ccafe in handle_builtin git.c:729:3 git#13 0x4cb01c in run_argv git.c:818:4 git#14 0x4cb01c in cmd_main git.c:949:19 git#15 0x6b3f3d in main common-main.c:52:11 git#16 0x7fe397c7a349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 7 byte(s) in 1 object(s) allocated from: #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0xa71e48 in xstrdup wrapper.c:29:14 #2 0x7db9bc in update_dir_rename_counts diffcore-rename.c:463:12 #3 0x7db6ae in find_renames diffcore-rename.c:1062:3 #4 0x7d76c3 in diffcore_rename_extended diffcore-rename.c:1472:18 #5 0x7b4cfc in diffcore_std diff.c:6705:4 #6 0x855e46 in log_tree_diff_flush log-tree.c:846:2 #7 0x856574 in log_tree_diff log-tree.c:955:3 git#8 0x856574 in log_tree_commit log-tree.c:986:10 git#9 0x9a9c67 in print_commit_summary sequencer.c:1329:7 git#10 0x52e623 in cmd_commit builtin/commit.c:1862:3 git#11 0x4ce83e in run_builtin git.c:475:11 git#12 0x4ccafe in handle_builtin git.c:729:3 git#13 0x4cb01c in run_argv git.c:818:4 git#14 0x4cb01c in cmd_main git.c:949:19 git#15 0x6b3f3d in main common-main.c:52:11 git#16 0x7fe397c7a349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 14 byte(s) leaked in 2 allocation(s). Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
u.head is populated using resolve_refdup(), which returns a newly
allocated string - hence we also need to free() it.
Found while running t0041 with LSAN:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xa8be98 in xstrdup wrapper.c:29:14
#2 0x9481db in head_atom_parser ref-filter.c:549:17
#3 0x9408c7 in parse_ref_filter_atom ref-filter.c:703:30
#4 0x9400e3 in verify_ref_format ref-filter.c:974:8
#5 0x4f9e8b in print_ref_list builtin/branch.c:439:6
#6 0x4f9e8b in cmd_branch builtin/branch.c:757:3
#7 0x4ce83e in run_builtin git.c:475:11
git#8 0x4ccafe in handle_builtin git.c:729:3
git#9 0x4cb01c in run_argv git.c:818:4
git#10 0x4cb01c in cmd_main git.c:949:19
git#11 0x6bdc2d in main common-main.c:52:11
git#12 0x7f96edf86349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
repo_diff_setup() calls through to diff.c's static prep_parse_options(),
which in turn allocates a new array into diff_opts.parseopts.
diff_setup_done() is responsible for freeing that array, and has the
benefit of verifying diff_opts too - hence we add a call to
diff_setup_done() to avoid leaking parseopts.
Output from the leak as found while running t0090 with LSAN:
Direct leak of 7120 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa8bf89 in do_xmalloc wrapper.c:41:8
#2 0x7a7bae in prep_parse_options diff.c:5636:2
#3 0x7a7bae in repo_diff_setup diff.c:4611:2
#4 0x93716c in repo_index_has_changes read-cache.c:2518:3
#5 0x872233 in unclean merge-ort-wrappers.c:12:14
#6 0x872233 in merge_ort_recursive merge-ort-wrappers.c:53:6
#7 0x5d5b11 in try_merge_strategy builtin/merge.c:752:12
git#8 0x5d0b6b in cmd_merge builtin/merge.c:1666:9
git#9 0x4ce83e in run_builtin git.c:475:11
git#10 0x4ccafe in handle_builtin git.c:729:3
git#11 0x4cb01c in run_argv git.c:818:4
git#12 0x4cb01c in cmd_main git.c:949:19
git#13 0x6bdc2d in main common-main.c:52:11
git#14 0x7f551eb51349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 7120 byte(s) leaked in 1 allocation(s)
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
apply_multi_file_filter and async_query_available_blobs both query
subprocess output using subprocess_read_status, which writes data into
the identically named filter_status strbuf. We add a strbuf_release to
avoid leaking their contents.
Leak output seen when running t0021 with LSAN:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c2b5 in xrealloc wrapper.c:126:8
#2 0x9ff99d in strbuf_grow strbuf.c:98:2
#3 0x9ff99d in strbuf_addbuf strbuf.c:304:2
#4 0xa101d6 in subprocess_read_status sub-process.c:45:5
#5 0x77793c in apply_multi_file_filter convert.c:886:8
#6 0x77793c in apply_filter convert.c:1042:10
#7 0x77a0b5 in convert_to_git_filter_fd convert.c:1492:7
git#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
git#9 0x8b48cd in index_fd object-file.c:2248:9
git#10 0x597411 in hash_fd builtin/hash-object.c:43:9
git#11 0x596be1 in hash_object builtin/hash-object.c:59:2
git#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
git#13 0x4ce83e in run_builtin git.c:475:11
git#14 0x4ccafe in handle_builtin git.c:729:3
git#15 0x4cb01c in run_argv git.c:818:4
git#16 0x4cb01c in cmd_main git.c:949:19
git#17 0x6bdc2d in main common-main.c:52:11
git#18 0x7f42acf79349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1 allocation(s).
Direct leak of 120 byte(s) in 5 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c295 in xrealloc wrapper.c:126:8
#2 0x9ff97d in strbuf_grow strbuf.c:98:2
#3 0x9ff97d in strbuf_addbuf strbuf.c:304:2
#4 0xa101b6 in subprocess_read_status sub-process.c:45:5
#5 0x775c73 in async_query_available_blobs convert.c:960:8
#6 0x80029d in finish_delayed_checkout entry.c:183:9
#7 0xa65d1e in check_updates unpack-trees.c:493:10
git#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
git#9 0x525971 in checkout builtin/clone.c:815:6
git#10 0x525971 in cmd_clone builtin/clone.c:1409:8
git#11 0x4ce83e in run_builtin git.c:475:11
git#12 0x4ccafe in handle_builtin git.c:729:3
git#13 0x4cb01c in run_argv git.c:818:4
git#14 0x4cb01c in cmd_main git.c:949:19
git#15 0x6bdc2d in main common-main.c:52:11
git#16 0x7fa253fce349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 120 byte(s) leaked in 5 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
These leaks all happen at the end of cmd_mv, hence don't matter in any
way. But we still fix the easy ones and squash the rest to get us closer
to being able to run tests without leaks.
LSAN output from t0050:
Direct leak of 384 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c015 in xrealloc wrapper.c:126:8
#2 0xa0a7e1 in add_entry string-list.c:44:2
#3 0xa0a7e1 in string_list_insert string-list.c:58:14
#4 0x5dac03 in cmd_mv builtin/mv.c:248:4
#5 0x4ce83e in run_builtin git.c:475:11
#6 0x4ccafe in handle_builtin git.c:729:3
#7 0x4cb01c in run_argv git.c:818:4
git#8 0x4cb01c in cmd_main git.c:949:19
git#9 0x6bd9ad in main common-main.c:52:11
git#10 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa8bd09 in do_xmalloc wrapper.c:41:8
#2 0x5dbc34 in internal_prefix_pathspec builtin/mv.c:32:2
#3 0x5da575 in cmd_mv builtin/mv.c:158:14
#4 0x4ce83e in run_builtin git.c:475:11
#5 0x4ccafe in handle_builtin git.c:729:3
#6 0x4cb01c in run_argv git.c:818:4
#7 0x4cb01c in cmd_main git.c:949:19
git#8 0x6bd9ad in main common-main.c:52:11
git#9 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa8bd09 in do_xmalloc wrapper.c:41:8
#2 0x5dbc34 in internal_prefix_pathspec builtin/mv.c:32:2
#3 0x5da4e4 in cmd_mv builtin/mv.c:148:11
#4 0x4ce83e in run_builtin git.c:475:11
#5 0x4ccafe in handle_builtin git.c:729:3
#6 0x4cb01c in run_argv git.c:818:4
#7 0x4cb01c in cmd_main git.c:949:19
git#8 0x6bd9ad in main common-main.c:52:11
git#9 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x49a9a2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0xa8c119 in xcalloc wrapper.c:140:8
#2 0x5da585 in cmd_mv builtin/mv.c:159:22
#3 0x4ce83e in run_builtin git.c:475:11
#4 0x4ccafe in handle_builtin git.c:729:3
#5 0x4cb01c in run_argv git.c:818:4
#6 0x4cb01c in cmd_main git.c:949:19
#7 0x6bd9ad in main common-main.c:52:11
git#8 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 4 byte(s) in 1 object(s) allocated from:
#0 0x49a9a2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0xa8c119 in xcalloc wrapper.c:140:8
#2 0x5da4f8 in cmd_mv builtin/mv.c:149:10
#3 0x4ce83e in run_builtin git.c:475:11
#4 0x4ccafe in handle_builtin git.c:729:3
#5 0x4cb01c in run_argv git.c:818:4
#6 0x4cb01c in cmd_main git.c:949:19
#7 0x6bd9ad in main common-main.c:52:11
git#8 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c015 in xrealloc wrapper.c:126:8
#2 0xa00226 in strbuf_grow strbuf.c:98:2
#3 0xa00226 in strbuf_vaddf strbuf.c:394:3
#4 0xa065c7 in xstrvfmt strbuf.c:981:2
#5 0xa065c7 in xstrfmt strbuf.c:991:8
#6 0x9e7ce7 in prefix_path_gently setup.c:115:15
#7 0x9e7fa6 in prefix_path setup.c:128:12
git#8 0x5dbdbf in internal_prefix_pathspec builtin/mv.c:55:23
git#9 0x5da575 in cmd_mv builtin/mv.c:158:14
git#10 0x4ce83e in run_builtin git.c:475:11
git#11 0x4ccafe in handle_builtin git.c:729:3
git#12 0x4cb01c in run_argv git.c:818:4
git#13 0x4cb01c in cmd_main git.c:949:19
git#14 0x6bd9ad in main common-main.c:52:11
git#15 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c015 in xrealloc wrapper.c:126:8
#2 0xa00226 in strbuf_grow strbuf.c:98:2
#3 0xa00226 in strbuf_vaddf strbuf.c:394:3
#4 0xa065c7 in xstrvfmt strbuf.c:981:2
#5 0xa065c7 in xstrfmt strbuf.c:991:8
#6 0x9e7ce7 in prefix_path_gently setup.c:115:15
#7 0x9e7fa6 in prefix_path setup.c:128:12
git#8 0x5dbdbf in internal_prefix_pathspec builtin/mv.c:55:23
git#9 0x5da4e4 in cmd_mv builtin/mv.c:148:11
git#10 0x4ce83e in run_builtin git.c:475:11
git#11 0x4ccafe in handle_builtin git.c:729:3
git#12 0x4cb01c in run_argv git.c:818:4
git#13 0x4cb01c in cmd_main git.c:949:19
git#14 0x6bd9ad in main common-main.c:52:11
git#15 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 558 byte(s) leaked in 7 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
merge_name() calls dwim_ref(), which allocates a new string into
found_ref. Therefore add a free() to avoid leaking found_ref.
LSAN output from t0021:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xa8beb8 in xstrdup wrapper.c:29:14
#2 0x954054 in expand_ref refs.c:671:12
#3 0x953cb6 in repo_dwim_ref refs.c:644:22
#4 0x5d3759 in dwim_ref refs.h:162:9
#5 0x5d3759 in merge_name builtin/merge.c:517:6
#6 0x5d3759 in collect_parents builtin/merge.c:1214:5
#7 0x5cf60d in cmd_merge builtin/merge.c:1458:16
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6bdbfd in main common-main.c:52:11
git#13 0x7f0430502349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
- cmd_rebase populates rebase_options.strategy with newly allocated
strings, hence we need to free those strings at the end of cmd_rebase
to avoid a leak.
- In some cases: get_replay_opts() is called, which prepares replay_opts
using data from rebase_options. We used to simply copy the pointer,
however that would now result in a double-free because
sequencer_remove_state() is eventually used to free
replay_opts.strategy. To avoid this we xstrdup() strategy when
adding it to replay_opts. (The original leak happens because we always
populate rebase_options.strategy, but we don't always enter the path
that calls get_replay_opts() and later sequencer_remove_state() - in
other words we'd sometimes but not always free rebase_options.strategy.
Relying on such indirect free'ing would be impossible to understand
anyhow.)
This was first seen when running t0021 with LSAN, but t2012 helped catch
the fact that we can't just free(options.strategy) at the end of
cmd_rebase (as that can cause a double-free). LSAN output from t0021:
LSAN output from t0021:
Direct leak of 4 byte(s) in 1 object(s) allocated from:
#0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xa71eb8 in xstrdup wrapper.c:29:14
#2 0x61b1cc in cmd_rebase builtin/rebase.c:1779:22
#3 0x4ce83e in run_builtin git.c:475:11
#4 0x4ccafe in handle_builtin git.c:729:3
#5 0x4cb01c in run_argv git.c:818:4
#6 0x4cb01c in cmd_main git.c:949:19
#7 0x6b3fad in main common-main.c:52:11
git#8 0x7f267b512349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 4 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
setup_unpack_trees_porcelain() populates various fields on
unpack_tree_opts, we need to call clear_unpack_trees_porcelain() to
avoid leaking them. Specifically, we used to leak
unpack_tree_opts.msgs_to_free.
We have to do this in leave_reset_head because there are multiple
scenarios where unpack_tree_opts has already been configured, followed
by a 'goto leave_reset_head'. But we can also 'goto leave_reset_head'
prior to having initialised unpack_tree_opts via memset(..., 0, ...).
Therefore we also move unpack_tree_opts initialisation to the start of
reset_head(), and convert it to use brace initialisation - which
guarantees that we can never clear an uninitialised unpack_tree_opts.
clear_unpack_tree_opts() is always safe to call as long as
unpack_tree_opts is at least zero-initialised, i.e. it does not depend
on a previous call to setup_unpack_trees_porcelain().
LSAN output from t0021:
Direct leak of 192 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9f7861 in strvec_push_nodup strvec.c:19:2
#3 0x9f7861 in strvec_pushf strvec.c:39:2
#4 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
#5 0x97e011 in reset_head reset.c:53:2
#6 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
#7 0x4ce83e in run_builtin git.c:475:11
git#8 0x4ccafe in handle_builtin git.c:729:3
git#9 0x4cb01c in run_argv git.c:818:4
git#10 0x4cb01c in cmd_main git.c:949:19
git#11 0x6b3f3d in main common-main.c:52:11
git#12 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 147 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 134 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43fe4 in setup_unpack_trees_porcelain unpack-trees.c:168:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 130 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43f20 in setup_unpack_trees_porcelain unpack-trees.c:150:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 603 byte(s) leaked in 4 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
- cmd_rebase populates rebase_options.strategy with newly allocated
strings, hence we need to free those strings at the end of cmd_rebase
to avoid a leak.
- In some cases: get_replay_opts() is called, which prepares replay_opts
using data from rebase_options. We used to simply copy the pointer
from rebase_options.strategy, however that would now result in a
double-free because sequencer_remove_state() is eventually used to
free replay_opts.strategy. To avoid this we xstrdup() strategy when
adding it to replay_opts.
The original leak happens because we always populate
rebase_options.strategy, but we don't always enter the path that calls
get_replay_opts() and later sequencer_remove_state() - in other words
we'd always allocate a new string into rebase_options.strategy but
only sometimes did we free it. We now make sure that rebse_options
and replay_opts both own their own copies of strategy, and each copy
is free'd independently.
This was first seen when running t0021 with LSAN, but t2012 helped catch
the fact that we can't just free(options.strategy) at the end of
cmd_rebase (as that can cause a double-free). LSAN output from t0021:
LSAN output from t0021:
Direct leak of 4 byte(s) in 1 object(s) allocated from:
#0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xa71eb8 in xstrdup wrapper.c:29:14
#2 0x61b1cc in cmd_rebase builtin/rebase.c:1779:22
#3 0x4ce83e in run_builtin git.c:475:11
#4 0x4ccafe in handle_builtin git.c:729:3
#5 0x4cb01c in run_argv git.c:818:4
#6 0x4cb01c in cmd_main git.c:949:19
#7 0x6b3fad in main common-main.c:52:11
git#8 0x7f267b512349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 4 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 25, 2021
setup_unpack_trees_porcelain() populates various fields on
unpack_tree_opts, we need to call clear_unpack_trees_porcelain() to
avoid leaking them. Specifically, we used to leak
unpack_tree_opts.msgs_to_free.
We have to do this in leave_reset_head because there are multiple
scenarios where unpack_tree_opts has already been configured, followed
by a 'goto leave_reset_head'. But we can also 'goto leave_reset_head'
prior to having initialised unpack_tree_opts via memset(..., 0, ...).
Therefore we also move unpack_tree_opts initialisation to the start of
reset_head(), and convert it to use brace initialisation - which
guarantees that we can never clear an uninitialised unpack_tree_opts.
clear_unpack_tree_opts() is always safe to call as long as
unpack_tree_opts is at least zero-initialised, i.e. it does not depend
on a previous call to setup_unpack_trees_porcelain().
LSAN output from t0021:
Direct leak of 192 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9f7861 in strvec_push_nodup strvec.c:19:2
#3 0x9f7861 in strvec_pushf strvec.c:39:2
#4 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
#5 0x97e011 in reset_head reset.c:53:2
#6 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
#7 0x4ce83e in run_builtin git.c:475:11
git#8 0x4ccafe in handle_builtin git.c:729:3
git#9 0x4cb01c in run_argv git.c:818:4
git#10 0x4cb01c in cmd_main git.c:949:19
git#11 0x6b3f3d in main common-main.c:52:11
git#12 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 147 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 134 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43fe4 in setup_unpack_trees_porcelain unpack-trees.c:168:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 130 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43f20 in setup_unpack_trees_porcelain unpack-trees.c:150:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 603 byte(s) leaked in 4 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
origin starts off pointing to somewhere within line, which is owned by
the caller. Later we might allocate a new string using xmemdupz() or
xstrfmt(). To avoid leaking these new strings, we introduce a to_free
pointer - which allows us to safely free the newly allocated string when
we're done (we cannot just free origin directly as it might still be
pointing to line).
LSAN output from t0090:
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa71f49 in do_xmalloc wrapper.c:41:8
#2 0xa720b0 in do_xmallocz wrapper.c:75:8
#3 0xa720b0 in xmallocz wrapper.c:83:9
#4 0xa720b0 in xmemdupz wrapper.c:99:16
#5 0x8092ba in handle_line fmt-merge-msg.c:187:23
#6 0x8092ba in fmt_merge_msg fmt-merge-msg.c:666:7
#7 0x5ce2e6 in prepare_merge_message builtin/merge.c:1119:2
git#8 0x5ce2e6 in collect_parents builtin/merge.c:1215:3
git#9 0x5c9c1e in cmd_merge builtin/merge.c:1454:16
git#10 0x4ce83e in run_builtin git.c:475:11
git#11 0x4ccafe in handle_builtin git.c:729:3
git#12 0x4cb01c in run_argv git.c:818:4
git#13 0x4cb01c in cmd_main git.c:949:19
git#14 0x6b3fad in main common-main.c:52:11
git#15 0x7fb929620349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 8 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
realpath is only populated if we execute the git_work_tree_initialized
block. However that block also causes us to return early, meaning we
never actually release the strbuf in the case where we populated it.
Therefore we move all strbuf related code into the block to guarantee
that we can't leak it.
LSAN output from t0095:
Direct leak of 129 byte(s) in 1 object(s) allocated from:
#0 0x49a9b9 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x78f585 in xrealloc wrapper.c:126:8
#2 0x713ff4 in strbuf_grow strbuf.c:98:2
#3 0x713ff4 in strbuf_getcwd strbuf.c:597:3
#4 0x4f0c18 in strbuf_realpath_1 abspath.c:99:7
#5 0x5ae4a4 in set_git_work_tree environment.c:259:3
#6 0x6fdd8a in setup_discovered_git_dir setup.c:931:2
#7 0x6fdd8a in setup_git_directory_gently setup.c:1235:12
git#8 0x4cb50d in get_bloom_filter_for_commit t/helper/test-bloom.c:41:2
git#9 0x4cb50d in cmd__bloom t/helper/test-bloom.c:95:3
git#10 0x4caa1f in cmd_main t/helper/test-tool.c:124:11
git#11 0x4caded in main common-main.c:52:11
git#12 0x7f0869f02349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 129 byte(s) leaked in 1 allocation(s).
It looks like this leak has existed since realpath was first added to
set_git_work_tree() in:
3d7747e (real_path: remove unsafe API, 2020-03-10)
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
relative_url() populates sb. In the normal return path, its buffer is
detached using strbuf_detach(). However the early return path does
nothing with sb, which means that sb's memory is leaked - therefore
we add a release to avoid this leak.
The reset is also only necessary for the normal return path, hence we
move it down to after the early-return to avoid unnecessary work.
LSAN output from t0060:
Direct leak of 121 byte(s) in 1 object(s) allocated from:
#0 0x7f31246f28b0 in realloc (/usr/lib64/libasan.so.4+0xdc8b0)
#1 0x98d7d6 in xrealloc wrapper.c:126
#2 0x909a60 in strbuf_grow strbuf.c:98
#3 0x90bf00 in strbuf_vaddf strbuf.c:401
#4 0x90c321 in strbuf_addf strbuf.c:335
#5 0x5cb78d in relative_url builtin/submodule--helper.c:182
#6 0x5cbe46 in resolve_relative_url_test builtin/submodule--helper.c:248
#7 0x410dcd in run_builtin git.c:475
git#8 0x410dcd in handle_builtin git.c:729
git#9 0x414087 in run_argv git.c:818
git#10 0x414087 in cmd_main git.c:949
git#11 0x40e9ec in main common-main.c:52
git#12 0x7f3123c41349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 121 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
cmd_for_each_repo() copies argv into args (a strvec), which is later
passed into run_command_on_repo(), which in turn copies that strvec onto
the end of child.args. The initial copy is unnecessary (we never modify
args). We therefore choose to just pass argv directly into
run_command_on_repo(), which lets us avoid the copy and fixes the leak.
LSAN output from t0068:
Direct leak of 192 byte(s) in 1 object(s) allocated from:
#0 0x7f63bd4ab8b0 in realloc (/usr/lib64/libasan.so.4+0xdc8b0)
#1 0x98d7e6 in xrealloc wrapper.c:126
#2 0x916914 in strvec_push_nodup strvec.c:19
#3 0x916a6e in strvec_push strvec.c:26
#4 0x4be4eb in cmd_for_each_repo builtin/for-each-repo.c:49
#5 0x410dcd in run_builtin git.c:475
#6 0x410dcd in handle_builtin git.c:729
#7 0x414087 in run_argv git.c:818
git#8 0x414087 in cmd_main git.c:949
git#9 0x40e9ec in main common-main.c:52
git#10 0x7f63bc9fa349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 22 byte(s) in 2 object(s) allocated from:
#0 0x7f63bd445e30 in __interceptor_strdup (/usr/lib64/libasan.so.4+0x76e30)
#1 0x98d698 in xstrdup wrapper.c:29
#2 0x916a63 in strvec_push strvec.c:26
#3 0x4be4eb in cmd_for_each_repo builtin/for-each-repo.c:49
#4 0x410dcd in run_builtin git.c:475
#5 0x410dcd in handle_builtin git.c:729
#6 0x414087 in run_argv git.c:818
#7 0x414087 in cmd_main git.c:949
git#8 0x40e9ec in main common-main.c:52
git#9 0x7f63bc9fa349 in __libc_start_main (/lib64/libc.so.6+0x24349)
See also discussion about the original implementation below - this code
appears to have evolved from a callback explaining the double-strvec-copy
pattern, but there's no strong reason to keep that now:
https://lore.kernel.org/git/68bbeca5-314b-08ee-ef36-040e3f3814e9@gmail.com/
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
old_dir/new_dir are free()'d at the end of update_dir_rename_counts, however if we return early we'll never free those strings. Therefore we should move all new allocations after the possible early return, avoiding a leak. This seems like a fairly recent leak, that started happening since the early-return was added in: 1ad69eb (diffcore-rename: compute dir_rename_counts in stages, 2021-02-27) LSAN output from t0022: Direct leak of 7 byte(s) in 1 object(s) allocated from: #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0xa71e48 in xstrdup wrapper.c:29:14 #2 0x7db9c7 in update_dir_rename_counts diffcore-rename.c:464:12 #3 0x7db6ae in find_renames diffcore-rename.c:1062:3 #4 0x7d76c3 in diffcore_rename_extended diffcore-rename.c:1472:18 #5 0x7b4cfc in diffcore_std diff.c:6705:4 #6 0x855e46 in log_tree_diff_flush log-tree.c:846:2 #7 0x856574 in log_tree_diff log-tree.c:955:3 git#8 0x856574 in log_tree_commit log-tree.c:986:10 git#9 0x9a9c67 in print_commit_summary sequencer.c:1329:7 git#10 0x52e623 in cmd_commit builtin/commit.c:1862:3 git#11 0x4ce83e in run_builtin git.c:475:11 git#12 0x4ccafe in handle_builtin git.c:729:3 git#13 0x4cb01c in run_argv git.c:818:4 git#14 0x4cb01c in cmd_main git.c:949:19 git#15 0x6b3f3d in main common-main.c:52:11 git#16 0x7fe397c7a349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 7 byte(s) in 1 object(s) allocated from: #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0xa71e48 in xstrdup wrapper.c:29:14 #2 0x7db9bc in update_dir_rename_counts diffcore-rename.c:463:12 #3 0x7db6ae in find_renames diffcore-rename.c:1062:3 #4 0x7d76c3 in diffcore_rename_extended diffcore-rename.c:1472:18 #5 0x7b4cfc in diffcore_std diff.c:6705:4 #6 0x855e46 in log_tree_diff_flush log-tree.c:846:2 #7 0x856574 in log_tree_diff log-tree.c:955:3 git#8 0x856574 in log_tree_commit log-tree.c:986:10 git#9 0x9a9c67 in print_commit_summary sequencer.c:1329:7 git#10 0x52e623 in cmd_commit builtin/commit.c:1862:3 git#11 0x4ce83e in run_builtin git.c:475:11 git#12 0x4ccafe in handle_builtin git.c:729:3 git#13 0x4cb01c in run_argv git.c:818:4 git#14 0x4cb01c in cmd_main git.c:949:19 git#15 0x6b3f3d in main common-main.c:52:11 git#16 0x7fe397c7a349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 14 byte(s) leaked in 2 allocation(s). Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
u.head is populated using resolve_refdup(), which returns a newly
allocated string - hence we also need to free() it.
Found while running t0041 with LSAN:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xa8be98 in xstrdup wrapper.c:29:14
#2 0x9481db in head_atom_parser ref-filter.c:549:17
#3 0x9408c7 in parse_ref_filter_atom ref-filter.c:703:30
#4 0x9400e3 in verify_ref_format ref-filter.c:974:8
#5 0x4f9e8b in print_ref_list builtin/branch.c:439:6
#6 0x4f9e8b in cmd_branch builtin/branch.c:757:3
#7 0x4ce83e in run_builtin git.c:475:11
git#8 0x4ccafe in handle_builtin git.c:729:3
git#9 0x4cb01c in run_argv git.c:818:4
git#10 0x4cb01c in cmd_main git.c:949:19
git#11 0x6bdc2d in main common-main.c:52:11
git#12 0x7f96edf86349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
repo_diff_setup() calls through to diff.c's static prep_parse_options(),
which in turn allocates a new array into diff_opts.parseopts.
diff_setup_done() is responsible for freeing that array, and has the
benefit of verifying diff_opts too - hence we add a call to
diff_setup_done() to avoid leaking parseopts.
Output from the leak as found while running t0090 with LSAN:
Direct leak of 7120 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa8bf89 in do_xmalloc wrapper.c:41:8
#2 0x7a7bae in prep_parse_options diff.c:5636:2
#3 0x7a7bae in repo_diff_setup diff.c:4611:2
#4 0x93716c in repo_index_has_changes read-cache.c:2518:3
#5 0x872233 in unclean merge-ort-wrappers.c:12:14
#6 0x872233 in merge_ort_recursive merge-ort-wrappers.c:53:6
#7 0x5d5b11 in try_merge_strategy builtin/merge.c:752:12
git#8 0x5d0b6b in cmd_merge builtin/merge.c:1666:9
git#9 0x4ce83e in run_builtin git.c:475:11
git#10 0x4ccafe in handle_builtin git.c:729:3
git#11 0x4cb01c in run_argv git.c:818:4
git#12 0x4cb01c in cmd_main git.c:949:19
git#13 0x6bdc2d in main common-main.c:52:11
git#14 0x7f551eb51349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 7120 byte(s) leaked in 1 allocation(s)
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
apply_multi_file_filter and async_query_available_blobs both query
subprocess output using subprocess_read_status, which writes data into
the identically named filter_status strbuf. We add a strbuf_release to
avoid leaking their contents.
Leak output seen when running t0021 with LSAN:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c2b5 in xrealloc wrapper.c:126:8
#2 0x9ff99d in strbuf_grow strbuf.c:98:2
#3 0x9ff99d in strbuf_addbuf strbuf.c:304:2
#4 0xa101d6 in subprocess_read_status sub-process.c:45:5
#5 0x77793c in apply_multi_file_filter convert.c:886:8
#6 0x77793c in apply_filter convert.c:1042:10
#7 0x77a0b5 in convert_to_git_filter_fd convert.c:1492:7
git#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
git#9 0x8b48cd in index_fd object-file.c:2248:9
git#10 0x597411 in hash_fd builtin/hash-object.c:43:9
git#11 0x596be1 in hash_object builtin/hash-object.c:59:2
git#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
git#13 0x4ce83e in run_builtin git.c:475:11
git#14 0x4ccafe in handle_builtin git.c:729:3
git#15 0x4cb01c in run_argv git.c:818:4
git#16 0x4cb01c in cmd_main git.c:949:19
git#17 0x6bdc2d in main common-main.c:52:11
git#18 0x7f42acf79349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1 allocation(s).
Direct leak of 120 byte(s) in 5 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c295 in xrealloc wrapper.c:126:8
#2 0x9ff97d in strbuf_grow strbuf.c:98:2
#3 0x9ff97d in strbuf_addbuf strbuf.c:304:2
#4 0xa101b6 in subprocess_read_status sub-process.c:45:5
#5 0x775c73 in async_query_available_blobs convert.c:960:8
#6 0x80029d in finish_delayed_checkout entry.c:183:9
#7 0xa65d1e in check_updates unpack-trees.c:493:10
git#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
git#9 0x525971 in checkout builtin/clone.c:815:6
git#10 0x525971 in cmd_clone builtin/clone.c:1409:8
git#11 0x4ce83e in run_builtin git.c:475:11
git#12 0x4ccafe in handle_builtin git.c:729:3
git#13 0x4cb01c in run_argv git.c:818:4
git#14 0x4cb01c in cmd_main git.c:949:19
git#15 0x6bdc2d in main common-main.c:52:11
git#16 0x7fa253fce349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 120 byte(s) leaked in 5 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
These leaks all happen at the end of cmd_mv, hence don't matter in any
way. But we still fix the easy ones and squash the rest to get us closer
to being able to run tests without leaks.
LSAN output from t0050:
Direct leak of 384 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c015 in xrealloc wrapper.c:126:8
#2 0xa0a7e1 in add_entry string-list.c:44:2
#3 0xa0a7e1 in string_list_insert string-list.c:58:14
#4 0x5dac03 in cmd_mv builtin/mv.c:248:4
#5 0x4ce83e in run_builtin git.c:475:11
#6 0x4ccafe in handle_builtin git.c:729:3
#7 0x4cb01c in run_argv git.c:818:4
git#8 0x4cb01c in cmd_main git.c:949:19
git#9 0x6bd9ad in main common-main.c:52:11
git#10 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa8bd09 in do_xmalloc wrapper.c:41:8
#2 0x5dbc34 in internal_prefix_pathspec builtin/mv.c:32:2
#3 0x5da575 in cmd_mv builtin/mv.c:158:14
#4 0x4ce83e in run_builtin git.c:475:11
#5 0x4ccafe in handle_builtin git.c:729:3
#6 0x4cb01c in run_argv git.c:818:4
#7 0x4cb01c in cmd_main git.c:949:19
git#8 0x6bd9ad in main common-main.c:52:11
git#9 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0xa8bd09 in do_xmalloc wrapper.c:41:8
#2 0x5dbc34 in internal_prefix_pathspec builtin/mv.c:32:2
#3 0x5da4e4 in cmd_mv builtin/mv.c:148:11
#4 0x4ce83e in run_builtin git.c:475:11
#5 0x4ccafe in handle_builtin git.c:729:3
#6 0x4cb01c in run_argv git.c:818:4
#7 0x4cb01c in cmd_main git.c:949:19
git#8 0x6bd9ad in main common-main.c:52:11
git#9 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x49a9a2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0xa8c119 in xcalloc wrapper.c:140:8
#2 0x5da585 in cmd_mv builtin/mv.c:159:22
#3 0x4ce83e in run_builtin git.c:475:11
#4 0x4ccafe in handle_builtin git.c:729:3
#5 0x4cb01c in run_argv git.c:818:4
#6 0x4cb01c in cmd_main git.c:949:19
#7 0x6bd9ad in main common-main.c:52:11
git#8 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 4 byte(s) in 1 object(s) allocated from:
#0 0x49a9a2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0xa8c119 in xcalloc wrapper.c:140:8
#2 0x5da4f8 in cmd_mv builtin/mv.c:149:10
#3 0x4ce83e in run_builtin git.c:475:11
#4 0x4ccafe in handle_builtin git.c:729:3
#5 0x4cb01c in run_argv git.c:818:4
#6 0x4cb01c in cmd_main git.c:949:19
#7 0x6bd9ad in main common-main.c:52:11
git#8 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c015 in xrealloc wrapper.c:126:8
#2 0xa00226 in strbuf_grow strbuf.c:98:2
#3 0xa00226 in strbuf_vaddf strbuf.c:394:3
#4 0xa065c7 in xstrvfmt strbuf.c:981:2
#5 0xa065c7 in xstrfmt strbuf.c:991:8
#6 0x9e7ce7 in prefix_path_gently setup.c:115:15
#7 0x9e7fa6 in prefix_path setup.c:128:12
git#8 0x5dbdbf in internal_prefix_pathspec builtin/mv.c:55:23
git#9 0x5da575 in cmd_mv builtin/mv.c:158:14
git#10 0x4ce83e in run_builtin git.c:475:11
git#11 0x4ccafe in handle_builtin git.c:729:3
git#12 0x4cb01c in run_argv git.c:818:4
git#13 0x4cb01c in cmd_main git.c:949:19
git#14 0x6bd9ad in main common-main.c:52:11
git#15 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 65 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c015 in xrealloc wrapper.c:126:8
#2 0xa00226 in strbuf_grow strbuf.c:98:2
#3 0xa00226 in strbuf_vaddf strbuf.c:394:3
#4 0xa065c7 in xstrvfmt strbuf.c:981:2
#5 0xa065c7 in xstrfmt strbuf.c:991:8
#6 0x9e7ce7 in prefix_path_gently setup.c:115:15
#7 0x9e7fa6 in prefix_path setup.c:128:12
git#8 0x5dbdbf in internal_prefix_pathspec builtin/mv.c:55:23
git#9 0x5da4e4 in cmd_mv builtin/mv.c:148:11
git#10 0x4ce83e in run_builtin git.c:475:11
git#11 0x4ccafe in handle_builtin git.c:729:3
git#12 0x4cb01c in run_argv git.c:818:4
git#13 0x4cb01c in cmd_main git.c:949:19
git#14 0x6bd9ad in main common-main.c:52:11
git#15 0x7fbfeffc4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 558 byte(s) leaked in 7 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
merge_name() calls dwim_ref(), which allocates a new string into
found_ref. Therefore add a free() to avoid leaking found_ref.
LSAN output from t0021:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xa8beb8 in xstrdup wrapper.c:29:14
#2 0x954054 in expand_ref refs.c:671:12
#3 0x953cb6 in repo_dwim_ref refs.c:644:22
#4 0x5d3759 in dwim_ref refs.h:162:9
#5 0x5d3759 in merge_name builtin/merge.c:517:6
#6 0x5d3759 in collect_parents builtin/merge.c:1214:5
#7 0x5cf60d in cmd_merge builtin/merge.c:1458:16
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6bdbfd in main common-main.c:52:11
git#13 0x7f0430502349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
- cmd_rebase populates rebase_options.strategy with newly allocated
strings, hence we need to free those strings at the end of cmd_rebase
to avoid a leak.
- In some cases: get_replay_opts() is called, which prepares replay_opts
using data from rebase_options. We used to simply copy the pointer
from rebase_options.strategy, however that would now result in a
double-free because sequencer_remove_state() is eventually used to
free replay_opts.strategy. To avoid this we xstrdup() strategy when
adding it to replay_opts.
The original leak happens because we always populate
rebase_options.strategy, but we don't always enter the path that calls
get_replay_opts() and later sequencer_remove_state() - in other words
we'd always allocate a new string into rebase_options.strategy but
only sometimes did we free it. We now make sure that rebase_options
and replay_opts both own their own copies of strategy, and each copy
is free'd independently.
This was first seen when running t0021 with LSAN, but t2012 helped catch
the fact that we can't just free(options.strategy) at the end of
cmd_rebase (as that can cause a double-free). LSAN output from t0021:
LSAN output from t0021:
Direct leak of 4 byte(s) in 1 object(s) allocated from:
#0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xa71eb8 in xstrdup wrapper.c:29:14
#2 0x61b1cc in cmd_rebase builtin/rebase.c:1779:22
#3 0x4ce83e in run_builtin git.c:475:11
#4 0x4ccafe in handle_builtin git.c:729:3
#5 0x4cb01c in run_argv git.c:818:4
#6 0x4cb01c in cmd_main git.c:949:19
#7 0x6b3fad in main common-main.c:52:11
git#8 0x7f267b512349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 4 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Jul 31, 2021
setup_unpack_trees_porcelain() populates various fields on
unpack_tree_opts, we need to call clear_unpack_trees_porcelain() to
avoid leaking them. Specifically, we used to leak
unpack_tree_opts.msgs_to_free.
We have to do this in leave_reset_head because there are multiple
scenarios where unpack_tree_opts has already been configured, followed
by a 'goto leave_reset_head'. But we can also 'goto leave_reset_head'
prior to having initialised unpack_tree_opts via memset(..., 0, ...).
Therefore we also move unpack_tree_opts initialisation to the start of
reset_head(), and convert it to use brace initialisation - which
guarantees that we can never clear an uninitialised unpack_tree_opts.
clear_unpack_tree_opts() is always safe to call as long as
unpack_tree_opts is at least zero-initialised, i.e. it does not depend
on a previous call to setup_unpack_trees_porcelain().
LSAN output from t0021:
Direct leak of 192 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9f7861 in strvec_push_nodup strvec.c:19:2
#3 0x9f7861 in strvec_pushf strvec.c:39:2
#4 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
#5 0x97e011 in reset_head reset.c:53:2
#6 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
#7 0x4ce83e in run_builtin git.c:475:11
git#8 0x4ccafe in handle_builtin git.c:729:3
git#9 0x4cb01c in run_argv git.c:818:4
git#10 0x4cb01c in cmd_main git.c:949:19
git#11 0x6b3f3d in main common-main.c:52:11
git#12 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 147 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 134 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43fe4 in setup_unpack_trees_porcelain unpack-trees.c:168:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 130 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa721e5 in xrealloc wrapper.c:126:8
#2 0x9e8d54 in strbuf_grow strbuf.c:98:2
#3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
#4 0x9f7774 in strvec_pushf strvec.c:36:2
#5 0xa43f20 in setup_unpack_trees_porcelain unpack-trees.c:150:3
#6 0x97e011 in reset_head reset.c:53:2
#7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
git#8 0x4ce83e in run_builtin git.c:475:11
git#9 0x4ccafe in handle_builtin git.c:729:3
git#10 0x4cb01c in run_argv git.c:818:4
git#11 0x4cb01c in cmd_main git.c:949:19
git#12 0x6b3f3d in main common-main.c:52:11
git#13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 603 byte(s) leaked in 4 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
that referenced
this pull request
Sep 18, 2021
cmd_show populate rev with lots of data, and later returns without
cleaning it up. That's reasonable - we need this data until the process
completes - so we take the easy way out and UNLEAK it.
We have to add the UNLEAK early on, as cmd_show might return via
cmd_log_walk() in the next few lines, or it might continue to the
no-walk implementation below.
This patch silences the following leaks which were found when running
t0000 against LSAN:
Direct leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ab168 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14
#2 0x83cced in add_object_array_with_path /home/ahunt/oss-fuzz/git/object.c:349:17
#3 0x8f4f5a in add_pending_object_with_path /home/ahunt/oss-fuzz/git/revision.c:329:2
#4 0x8eb2b6 in handle_revision_arg_1 /home/ahunt/oss-fuzz/git/revision.c:2082:2
#5 0x8eadad in handle_revision_arg /home/ahunt/oss-fuzz/git/revision.c:2089:12
#6 0x8eea99 in setup_revisions /home/ahunt/oss-fuzz/git/revision.c:2756:7
#7 0x59c024 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:206:9
git#8 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2
git#9 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2
git#10 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
git#11 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
git#12 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
git#13 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
git#14 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
git#15 0x7f7c56197349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x49a9d2 in calloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x9ab4c2 in xcalloc /home/ahunt/oss-fuzz/git/wrapper.c:140:8
#2 0x59c269 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:233:18
#3 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2
#4 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2
#5 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
#6 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
#7 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
git#8 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
git#9 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
git#10 0x7f7c56197349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ab168 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14
#2 0x8f5e30 in add_rev_cmdline /home/ahunt/oss-fuzz/git/revision.c:1482:23
#3 0x8eb26d in handle_revision_arg_1 /home/ahunt/oss-fuzz/git/revision.c:2081:2
#4 0x8eadad in handle_revision_arg /home/ahunt/oss-fuzz/git/revision.c:2089:12
#5 0x8eea99 in setup_revisions /home/ahunt/oss-fuzz/git/revision.c:2756:7
#6 0x59c024 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:206:9
#7 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2
git#8 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2
git#9 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
git#10 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
git#11 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
git#12 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
git#13 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
git#14 0x7fc4b3f06349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Sep 18, 2021
cmd_show() uses objects to point to rev.pending's objects. Later, it might
detach rev.pending's objects: when handling an OBJ_COMMIT it will reset
rev.pending (without freeing its objects). Detaching (as opposed to freeing)
is necessary because cmd_show() continues iterating over the original objects
array.
We choose to UNLEAK because there's no real advantage to cleaning up
properly (cmd_show() exits immediately after looping over these
objects). A number of alternatives exist, but are all significantly more
complex for no gain:
Alternative 1:
Convert objects into an object_array, and memcpy rev.pending into it
(followed by detaching rev.pending immediately - making objects the
owner of what used to be rev.pending). Then we could safely
objects_array_clear() at the end of cmd_show(). And we can rely on
a preexisting UNLEAK(rev) to avoid having to clean up rev.pending.
This is a more complex and riskier approach vs a simple UNLEAK,
and doesn't add value.
Alternative 2:
A variation on alternative 1. We make objects own the object_array as
before. Once we're done, we free the new rev.pending array (which
might be empty), and we memcpy objects back into rev.pending, relying
on the existin UNLEAK(rev) to avoid having to free rev.pending.
ASAN output from t0000:
Direct leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x487504 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:437:3
#1 0x9e4ef8 in xstrdup wrapper.c:29:14
#2 0x86395d in add_object_array_with_path object.c:366:17
#3 0x9264fc in add_pending_object_with_path revision.c:330:2
#4 0x91c4e0 in handle_revision_arg_1 revision.c:2086:2
#5 0x91bfcd in handle_revision_arg revision.c:2093:12
#6 0x91ff5a in setup_revisions revision.c:2780:7
#7 0x5a7678 in cmd_log_init_finish builtin/log.c:206:9
git#8 0x5a4f18 in cmd_log_init builtin/log.c:278:2
git#9 0x5a55d1 in cmd_show builtin/log.c:646:2
git#10 0x4cff30 in run_builtin git.c:461:11
git#11 0x4cdb00 in handle_builtin git.c:713:3
git#12 0x4cf527 in run_argv git.c:780:4
git#13 0x4cd426 in cmd_main git.c:911:19
git#14 0x6b2eb5 in main common-main.c:52:11
git#15 0x7f74fc9bd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 41 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Sep 18, 2021
cmd_show puts a lot of data into rev, and doesn't clean it up before
returning. That's reasonable - we use most if not all of rev up until
cmd_show is finished - there's not much value in doing a proper cleanup.
Therefore we take the easy way out and UNLEAK rev.
The UNLEAK has to be performed early on, as cmd_show might return via
cmd_log_walk() in the next few lines, or it might continue to the
no-walk implementation below.
This patch silences the following leaks which were found when running
t0000 against LSAN:
Direct leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ab168 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14
#2 0x83cced in add_object_array_with_path /home/ahunt/oss-fuzz/git/object.c:349:17
#3 0x8f4f5a in add_pending_object_with_path /home/ahunt/oss-fuzz/git/revision.c:329:2
#4 0x8eb2b6 in handle_revision_arg_1 /home/ahunt/oss-fuzz/git/revision.c:2082:2
#5 0x8eadad in handle_revision_arg /home/ahunt/oss-fuzz/git/revision.c:2089:12
#6 0x8eea99 in setup_revisions /home/ahunt/oss-fuzz/git/revision.c:2756:7
#7 0x59c024 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:206:9
git#8 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2
git#9 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2
git#10 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
git#11 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
git#12 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
git#13 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
git#14 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
git#15 0x7f7c56197349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x49a9d2 in calloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
#1 0x9ab4c2 in xcalloc /home/ahunt/oss-fuzz/git/wrapper.c:140:8
#2 0x59c269 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:233:18
#3 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2
#4 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2
#5 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
#6 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
#7 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
git#8 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
git#9 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
git#10 0x7f7c56197349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x486834 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0x9ab168 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14
#2 0x8f5e30 in add_rev_cmdline /home/ahunt/oss-fuzz/git/revision.c:1482:23
#3 0x8eb26d in handle_revision_arg_1 /home/ahunt/oss-fuzz/git/revision.c:2081:2
#4 0x8eadad in handle_revision_arg /home/ahunt/oss-fuzz/git/revision.c:2089:12
#5 0x8eea99 in setup_revisions /home/ahunt/oss-fuzz/git/revision.c:2756:7
#6 0x59c024 in cmd_log_init_finish /home/ahunt/oss-fuzz/git/builtin/log.c:206:9
#7 0x5998d8 in cmd_log_init /home/ahunt/oss-fuzz/git/builtin/log.c:275:2
git#8 0x599f9b in cmd_show /home/ahunt/oss-fuzz/git/builtin/log.c:641:2
git#9 0x4cd92d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
git#10 0x4cb5fa in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
git#11 0x4ccf57 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
git#12 0x4caf49 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
git#13 0x69ce3e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
git#14 0x7fc4b3f06349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt
added a commit
that referenced
this pull request
Sep 18, 2021
cmd_show() uses objects to point to rev.pending's objects. Later, it might
detach rev.pending's objects: when handling an OBJ_COMMIT it will reset
rev.pending (without freeing its objects). Detaching (as opposed to freeing)
is necessary because cmd_show() continues iterating over the original objects
array.
We choose to UNLEAK because there's no real advantage to cleaning up
properly (cmd_show() exits immediately after looping over these
objects). A number of alternatives exist, but are all significantly more
complex for no gain:
Alternative 1:
Convert objects into an object_array, and memcpy rev.pending into it
(followed by detaching rev.pending immediately - making objects the
owner of what used to be rev.pending). Then we could safely
objects_array_clear() at the end of cmd_show(). And we can rely on
a preexisting UNLEAK(rev) to avoid having to clean up rev.pending.
This is a more complex and riskier approach vs a simple UNLEAK,
and doesn't add any user-visible value.
Alternative 2:
A variation on alternative 1. We make objects own the object_array as
before. Once we're done, we free the new rev.pending array (which
might be empty), and we memcpy objects back into rev.pending, relying
on the existin UNLEAK(rev) to avoid having to free rev.pending.
ASAN output from t0000:
Direct leak of 41 byte(s) in 1 object(s) allocated from:
#0 0x487504 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:437:3
#1 0x9e4ef8 in xstrdup wrapper.c:29:14
#2 0x86395d in add_object_array_with_path object.c:366:17
#3 0x9264fc in add_pending_object_with_path revision.c:330:2
#4 0x91c4e0 in handle_revision_arg_1 revision.c:2086:2
#5 0x91bfcd in handle_revision_arg revision.c:2093:12
#6 0x91ff5a in setup_revisions revision.c:2780:7
#7 0x5a7678 in cmd_log_init_finish builtin/log.c:206:9
git#8 0x5a4f18 in cmd_log_init builtin/log.c:278:2
git#9 0x5a55d1 in cmd_show builtin/log.c:646:2
git#10 0x4cff30 in run_builtin git.c:461:11
git#11 0x4cdb00 in handle_builtin git.c:713:3
git#12 0x4cf527 in run_argv git.c:780:4
git#13 0x4cd426 in cmd_main git.c:911:19
git#14 0x6b2eb5 in main common-main.c:52:11
git#15 0x7f74fc9bd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 41 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.