Re: [PATCH] leak tests: add an interface to the LSAN_OPTIONS "suppressions"

From: Taylor Blau <me@ttaylorr.com>
To: Jeff King <peff@peff.net>
Cc: "Taylor Blau" <me@ttaylorr.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Derrick Stolee" <dstolee@microsoft.com>
Subject: Re: [PATCH] leak tests: add an interface to the LSAN_OPTIONS "suppressions"
Date: Tue, 26 Oct 2021 17:30:47 -0400	[thread overview]
Message-ID: <YXhzh+pW5KvU7MEA@nand.local> (raw)
In-Reply-To: <YXhvAQgxKpq65qcg@coredump.intra.peff.net>

On Tue, Oct 26, 2021 at 05:11:29PM -0400, Jeff King wrote:
> On Tue, Oct 26, 2021 at 04:23:14PM -0400, Taylor Blau wrote:
>
> > So this all feels like a bug in ASan to me. I'm curious if it works on
> > your system, but in the meantime I think the best path forward is to
> > drop the last patch of my original series (the one with the three
> > UNLEAK() calls) and to avoid relying on this patch for the time being.
>
> Bugs aside, I'd much rather see UNLEAK() annotations than external ones,
> for all the reasons we introduced UNLEAK() in the first place:
>
>   - it keeps the annotations near the code. Yes, that creates conflicts
>     when the code is changed (or the leak is actually fixed), but that's
>     a feature. It keeps them from going stale.

I agree completely. I noted as much in my message here:

    https://lore.kernel.org/git/YXJAfICQN8s5Gm7s@nand.local/

but Ævar made it sound like his work would be made much easier without
the conflict. Since I'm not in any kind of rush to make t5319 leak-free,
I figured that queueing the parts of that series that wouldn't conflict
with Ævar's ongoing work would be a net-positive.

>   - leak-checkers only know where things are allocated, not who is
>     supposed to own them. So you're often annotating the wrong thing;
>     it's not a strdup() call which is buggy and leaking, but the
>     function five layers up the stack which was supposed to take
>     ownership and didn't.

TBH, I find this much more compelling. Either way, I don't feel strongly
enough to deviate from v2 much, and I don't want to create more work for
Ævar along the way.

Thanks,
Taylor