malloc: Use _int_free_chunk in tcache_thread_shutdown

[Wilco Dijkstra]

Hi Arjun,

Thanks for the detailed review.

> I was looking to find the recursion path, and it looks like:
> tcache_thread_shutdown -> libc_free -> tcache_double_free_verify -> libc_free.

Yes, that's the main one (it was also possible to get back into tcache_init in
the past where it had a special check to catch that recursion).
Note due to other recent changes, we now walk tcache entries N^2 times
during shutdown...

> Not really related to this change but tcache_double_free_verify seems
> a little misleading since it doesn't just verify.

We could call it tcache_double_free_verify_and_free...

> Looking at libc_free, what it does to blocks that eventually get put
> into the tcache is:
>
> 1. tag memory back to libc ownership.
> 2. check for misalignment.
> 3. call tcache_double_free_verify to catch anything that was
> previously freed into the tache, and is now being freed again.
>
> So the second time round (while we are processing already tcached
> blocks during this thread shutdown), we can skip these checks.

Correct.

> If I understand it correctly, doesn't this additional check that does
> occur in libc_free before int_free_chunk now get skipped with this
> change?:
>
>  /* Check size >= MINSIZE and p + size does not overflow.  */
>  if (__glibc_unlikely (INT_ADD_OVERFLOW ((uintptr_t) p,
>                                          size - MINSIZE)))
>    return malloc_printerr_tail ("free(): invalid size");

It does skip this, but we also skip this check when allocating/freeing into tcache
given the check cannot trigger for such blocks when inserting into tcache.
Since far more specific checks are done by _int_free_(merge_)chunk, this is not
an issue. It's not clear why this check was originally added.

Figuring out which checks are actually beneficial, doing them consistently and
efficiently for all possible paths (in tcache and the 4 different bins) is non-trivial
and not the goal here...

Cheers,
Wilco