Re: [PATCH] leak tests: free() before die for two API functions
flat view
From: Martin Ågren <hidden>
Date: 2021-10-21 16:13:17
On Thu, 21 Oct 2021 at 13:43, Ævar Arnfjörð Bjarmason [off-list ref] wrote:
Call free() just before die() in two API functions whose tests are asserted under SANITIZE=leak. Normally this would not be needed due to how SANITIZE=leak works, but in these cases my GCC version (10.2.1-6) will fail tests t0001 and t0017 under SANITIZE=leak depending on the optimization level.
Seems a bit unfortunate. I have to wonder why these in particular trigger this compiler bug or whatever it is, but oh well.
quoted hunk ↗ jump to hunk
- if (check_refname_format(full_ref, 0)) + if (check_refname_format(full_ref, 0)) { + free(ret); + free(full_ref); die(_("invalid branch name: %s = %s"), config_display_key, ret); + } free(full_ref);
This looks like use-after-free. Rather than complicating this by, e.g., first formatting the string, then freeing `ret`, then dying, could we -- if we really want this workaround -- make the workaround be `UNLEAK` instead? Also, if we do something like this patch, I think we should try to avoid this free-before-die then being cargo-culted all across the codebase. How about UNLEAK(ret); /* work around compiler bug */ UNLEAK(full_ref); /* work around compiler bug */ or something? Martin