Subject: Re: [PATCH] leak tests: free() before die for two API functions

From: Junio C Hamano <[email-masked]>
Date: Thu, 21 Oct 2021 11:51:32 -0700

Andrzej Hunt <[email-masked]> writes:

>> On 21 Oct 2021, at 13:42, Ævar Arnfjörð Bjarmason <[email-masked]> 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.