From: Junio C Hamano <gitster@pobox.com>
To: Andrzej Hunt <andrzej@ahunt.org>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] leak tests: free() before die for two API functions
Date: Thu, 21 Oct 2021 11:51:32 -0700 [thread overview]
Message-ID: <xmqqcznyky8b.fsf@gitster.g> (raw)
In-Reply-To: <FD837FF9-E83F-42AA-AC13-EADD161D20BE@ahunt.org> (Andrzej Hunt's message of "Thu, 21 Oct 2021 17:33:41 +0200")
Andrzej Hunt <andrzej@ahunt.org> writes:
>> On 21 Oct 2021, at 13:42, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>>
>> Call free() just before die() in two API functions whose tests are
>> asserted under SANITIZE=leak. Normally this would not be needed due to
>> how SANITIZE=leak works, but in these cases my GCC version (10.2.1-6)
>> will fail tests t0001 and t0017 under SANITIZE=leak depending on the
>> optimization level.
>
> I’m curious - to me this seems like a compiler/sanitiser bug, can
> it also be reproduced with clang, or even newer versions of gcc?
> Similarly, can it be reproduced with your gcc version, using
> ASAN+LSAN (as opposed to LSAN by itself)? I remember seeing some
> false positives in the past for some permutations of compilers and
> sanitisers, but I’ve lost track of the details.
>
> These kinds of fixes seem noisy if it’s just to work around what
> appears to be a bug (and to be philosophical: we wouldn’t want to
> do the same for all “leaks” up the call stack if a specific
> compiler complained about them after a die() - after all there
> will be many more allocations that didn’t get free’d floating
> around - so why is it OK for these “leaks”?)
Exactly my feeling. I'll leave this patch hanging on the list
without picking it up until we know this is a reasonable "fix" on
our side and not adding noize only to work around the bug in the
tools.
Thanks.
next prev parent reply other threads:[~2021-10-21 18:51 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-21 11:42 [PATCH] leak tests: free() before die for two API functions Ævar Arnfjörð Bjarmason
2021-10-21 15:33 ` Andrzej Hunt
2021-10-21 18:51 ` Junio C Hamano [this message]
2021-10-21 16:13 ` Martin Ågren
2021-10-21 19:54 ` [PATCH v2 0/3] refs.c + config.c: plug memory leaks Ævar Arnfjörð Bjarmason
2021-10-21 19:54 ` [PATCH v2 1/3] refs.c: make "repo_default_branch_name" static, remove xstrfmt() Ævar Arnfjörð Bjarmason
2021-10-21 23:26 ` Junio C Hamano
2021-10-21 19:54 ` [PATCH v2 2/3] config.c: don't leak memory in handle_path_include() Ævar Arnfjörð Bjarmason
2021-10-21 23:30 ` Junio C Hamano
2021-10-22 17:19 ` Ævar Arnfjörð Bjarmason
2021-10-22 21:21 ` Junio C Hamano
2021-10-22 22:30 ` Ævar Arnfjörð Bjarmason
2021-10-21 19:54 ` [PATCH v2 3/3] config.c: free(expanded) before die(), work around GCC oddity Ævar Arnfjörð Bjarmason
2021-10-21 23:32 ` Junio C Hamano
2021-10-22 18:19 ` [PATCH v3 0/6] usage.c: add die_message() & plug memory leaks in refs.c & config.c Ævar Arnfjörð Bjarmason
2021-10-22 18:19 ` [PATCH v3 1/6] usage.c: add a die_message() routine Ævar Arnfjörð Bjarmason
2021-10-24 5:49 ` Junio C Hamano
2021-10-22 18:19 ` [PATCH v3 2/6] usage.c API users: use die_message() where appropriate Ævar Arnfjörð Bjarmason
2021-10-22 18:19 ` [PATCH v3 3/6] usage.c + gc: add and use a die_message_errno() Ævar Arnfjörð Bjarmason
2021-10-24 5:52 ` Junio C Hamano
2021-10-22 18:19 ` [PATCH v3 4/6] config.c: don't leak memory in handle_path_include() Ævar Arnfjörð Bjarmason
2021-10-24 5:53 ` Junio C Hamano
2021-10-22 18:19 ` [PATCH v3 5/6] config.c: free(expanded) before die(), work around GCC oddity Ævar Arnfjörð Bjarmason
2021-10-26 8:53 ` Jeff King
2021-10-22 18:19 ` [PATCH v3 6/6] refs: plug memory leak in repo_default_branch_name() Ævar Arnfjörð Bjarmason
2021-10-27 21:50 ` [PATCH v3 0/6] usage.c: add die_message() & plug memory leaks in refs.c & config.c Jonathan Tan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqcznyky8b.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=andrzej@ahunt.org \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.