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

From: Jeff King <peff@peff.net>
Date: Oct 26, 2021, 21:11 UTC
Message-ID: <YXhvAQgxKpq65qcg@coredump.intra.peff.net>
In-Reply-To: <YXhjstW2XAnixEqh@nand.local>

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.

  - 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.

And if we avoid any annotation bugs by doing so, that's icing on the cake. :)

-Peff