Re: [patch] tcache double free check

[DJ Delorie <dj at redhat dot com>]

Florian Weimer <fweimer@redhat.com> writes:
>> diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c
>
> This should go into a separate commit.  I'm not sure if this is the
> right fix.  Why is check_free called twice?

I'm not sure either, but I included it because there would otherwise be
an unexplained regression triggered by my malloc patch, and I didn't
want to be blamed for it ;-)

>> +      /* If we get here, it was a coincidence.  We've wasted a few
>> +	 cycles, but don't abort.  */
>> +    }
>
> Should the above land in a separate function?  The code below looks
> pretty similar.

Perhaps an inline function.  I'm wary of adding more call overhead on
fast paths.

>> +  /* Now we mark this chunk as "in the tcache" so the above test will
>> +     detect a double free.  */
>> +  e->key = tcache;
>
> Does this put the address of the tcache control block on the heap?

Only while the blocks are in the tcache itself.  The value is arbitrary,
it can be anything that we can argue won't come up in usual program
flows.

> And this clears it again, so that it does not leak to user code, and
> it's not necessary to check the entire list?  This should be mentioned
> in a comment.

Right, the intention is that *if* the key has the magic value in it,
then we take the time to scan the whole list.

> This will print the malloc error message to the build output, where it
> will confuse QE.  Perhaps you can use support_capture_subprocess to
> capture the error message?

Or I could fprintf to stderr, but I was just following what the other
tests already do.