Re: [PATCH 4/5] Fix deadlock in _int_free consistency check

[Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>]

Andreas Schwab wrote:

> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index c00df205c6004ee5b5d0aee9ffd5130b3c8f9e9f..f4f44400d120188c4d0bece996380e04b35c8fac 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -4168,15 +4168,14 @@ _int_free (mstate av, mchunkptr p, int have_lock)
>                             >= av->system_mem, 0))
>        {
>        /* We might not have a lock at this point and concurrent modifications
> -        of system_mem might have let to a false positive.  Redo the test
> -        after getting the lock.  */
> -     if (!have_lock
> -         || ({ __libc_lock_lock (av->mutex);
> -               chunksize_nomask (chunk_at_offset (p, size)) <= 2 * SIZE_SZ
> -               || chunksize (chunk_at_offset (p, size)) >= av->system_mem;
> -             }))
> +        of system_mem might result in a false positive.  Redo the test after
> +        getting the lock.  */
> +     if (!have_lock)
> +       __libc_lock_lock (av->mutex);
> +     if (chunksize_nomask (chunk_at_offset (p, size)) <= 2 * SIZE_SZ
> +         || chunksize (chunk_at_offset (p, size)) >= av->system_mem)

> There is no need to redo the tests if we had the lock.

Well I guess an alternative is to do:

if (have_lock)
   print error
else
{
  lock
  repeat test and print error
  unlock
}

I also wonder whether we should actually unlock again before printing the error -
or do we assume/hope/know no memory allocation is ever required in the error case?

Wilco