On Mon, Nov 27, 2023 at 10:51:00AM +0900, Junio C Hamano wrote:
> >> + if (opts->new_branch_force) {
> >> + char *full_ref = xstrfmt("refs/heads/%s", opts->new_branch);
> >> + die_if_switching_to_a_branch_in_use(opts, full_ref);
> >> + free(full_ref);
> >
> > 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.
Right, I think the only reasonable approach here is to not consider this
a leak. We've discussed this in the past. Here's a link into a relevant
thread for reference, but I don't think it's really worth anybody's
time to re-visit:
https://lore.kernel.org/git/Y0+i1G5ybdhUGph2@xxxxxxxxxxxxxxxxxxxxxxx/
> 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.
We did have that problem, but it should no longer be the case after
d3775de074 (Makefile: force -O0 when compiling with SANITIZE=leak,
2022-10-18). If that is not sufficient for some compiler/code combo, I'd
be interested to hear about it.
-Peff
