Hi Junio
On 27/11/2023 01:51, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
At the moment this is academic as neither of the test scripts changed
by this patch are leak free and so I don't think we need to worry
about it but it raises an interesting question about how we should
handle memory leaks when dying. Leaving the leak when dying means that
a test script that tests an expected failure will never be leak free
but using UNLEAK() would mean we miss a leak being introduced in the
successful case should the call to "free()" ever be removed.
Is there a leak here? The piece of memory is pointed at by an on-stack
variable full_ref when leak sanitizer starts scanning the heap and
the stack just before the process exits due to die, so I do not see
a reason to worry about this particular variable over all the other
on stack variables we accumulated before the control reached this
point of the code.
Oh, good point. I was thinking "we exit without calling free() so it is
leaked" but as you say the leak checker (thankfully) does not consider
it a leak as there is still a reference to the allocation on the stack.
Sorry for the noise
Phillip
Are you worried about optimizing compilers that behave more cleverly
than their own good to somehow lose the on-stack reference to
full_ref while calling die_if_switching_to_a_branch_in_use()? We
might need to squelch them with UNLEAK() but that does not mean we
have to remove the free() we see above, and I suspect a more
productive use of our time to solve that issue is ensure that our
leak-sanitizing build will not triger such an unwanted optimization
anyway.
Thanks.
