Re: [PATCH 0/10] towards clean leak-checker output

From: "Martin Ågren" <martin.agren@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 0/10] towards clean leak-checker output
Date: Tue, 5 Sep 2017 22:41:51 +0200	[thread overview]
Message-ID: <CAN0heSrz=tVHj13X+yHJpAR=5o7j8EqbBdw8hvrhcnhbA04QAA@mail.gmail.com> (raw)
In-Reply-To: <20170905190212.4xx4xukx2bbtudij@sigill.intra.peff.net>

On 5 September 2017 at 21:02, Jeff King <peff@peff.net> wrote:
> On Tue, Sep 05, 2017 at 07:50:10PM +0200, Martin Ågren wrote:
>
>> >     That line is the setting of argv0_path, which is a global (and thus
>> >     shouldn't be marked as leaking). Interestingly, it only happens with
>> >     -O2. Compiling with -O0 works fine. I'm not sure if it's a bug or
>> >     what.
>> >
>> >     I did most of my testing with clang-6.0, which gets this case right.
>>
>> Hmmm, I got the same wrong results (IMHO) from Valgrind, which
>> classified this as "definitely lost". Like you I found that -O0 helped.
>> And yes, that was with gcc. Maybe gcc with optimization somehow manages
>> to hide the pointers from these tools. I know too little about the
>> technical details to have any real ideas, though. My searches did not
>> bring up anything useful. (gcc 5.4.0)
>
> Yeah, I think it is just optimizing out the variable entirely. If
> RUNTIME_PREFIX isn't defined (and it's not for non-Windows platforms)
> then we never look at the variable at all, and it's a dead assignment.
> And the compiler can see that easily because it's got static linkage. So
> it drops the variable completely, but it can't drop the call to
> xstrdup() with the information in exec_cmd.c. It has to call the
> function and throw away the result, resulting in the leak.

I see. Yeah, that makes sense.

FWIW, this series (combined with the other series you mentioned) makes
t0000 and t0001 pass for me with gcc/clang. There are actually some
leaks in t0000, they're just silently being reported to stderr, since
the exit statuses from git are hidden by pipes. Maybe you're already
aware of it. Depending on your definition of "running clean" it might be
out of scope for this series, which is of course still very interesting
and enlightening as it stands.

One is in cmd_show (you did mention git show) where we leak data in rev.
The other is some use of strdup. I can't immediately figure out how to
get a useful stacktrace (you mentioned this as well) and it's past
bed-time here. I'll try to play more with this tomorrow.

Note for self: getting rid of all pipes would probably also help flush
out a few leaks (or "introduce" them, depending on your viewpoint).

Martin