[v3,02/10] Add mcheck tests to malloc

  Like malloc-check, add generic rules to run all tests in malloc by
linking with libmcheck.a so as to provide coverage for mcheck().
Currently the following 12 tests fail:

FAIL: malloc/tst-malloc-backtrace-mcheck
FAIL: malloc/tst-malloc-fork-deadlock-mcheck
FAIL: malloc/tst-malloc-stats-cancellation-mcheck
FAIL: malloc/tst-malloc-tcache-leak-mcheck
FAIL: malloc/tst-malloc-thread-exit-mcheck
FAIL: malloc/tst-malloc-thread-fail-mcheck
FAIL: malloc/tst-malloc-usable-static-mcheck
FAIL: malloc/tst-malloc-usable-static-tunables-mcheck
FAIL: malloc/tst-malloc-usable-tunables-mcheck
FAIL: malloc/tst-malloc_info-mcheck
FAIL: malloc/tst-memalign-mcheck
FAIL: malloc/tst-posix_memalign-mcheck

and they have been added to tests-exclude-mcheck for now to keep
status quo.  At least the last two can be attributed to bugs in
mcheck() but I haven't fixed them here since they should be fixed by
removing malloc hooks.  Others need to be triaged to check if they're
due to mcheck bugs or due to actual bugs.

Reviewed-by: DJ Delorie <dj@redhat.com>
---
 Rules           | 15 ++++++++++++++-
 malloc/Makefile | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)
  

On 02/07/2021 04:35, Siddhesh Poyarekar via Libc-alpha wrote:
> Like malloc-check, add generic rules to run all tests in malloc by
> linking with libmcheck.a so as to provide coverage for mcheck().
> Currently the following 12 tests fail:
> 
> FAIL: malloc/tst-malloc-backtrace-mcheck
> FAIL: malloc/tst-malloc-fork-deadlock-mcheck
> FAIL: malloc/tst-malloc-stats-cancellation-mcheck
> FAIL: malloc/tst-malloc-tcache-leak-mcheck
> FAIL: malloc/tst-malloc-thread-exit-mcheck
> FAIL: malloc/tst-malloc-thread-fail-mcheck
> FAIL: malloc/tst-malloc-usable-static-mcheck
> FAIL: malloc/tst-malloc-usable-static-tunables-mcheck
> FAIL: malloc/tst-malloc-usable-tunables-mcheck
> FAIL: malloc/tst-malloc_info-mcheck
> FAIL: malloc/tst-memalign-mcheck
> FAIL: malloc/tst-posix_memalign-mcheck
> 
> and they have been added to tests-exclude-mcheck for now to keep
> status quo.  At least the last two can be attributed to bugs in
> mcheck() but I haven't fixed them here since they should be fixed by
> removing malloc hooks.  Others need to be triaged to check if they're
> due to mcheck bugs or due to actual bugs.
> 
> Reviewed-by: DJ Delorie <dj@redhat.com>

Hi Siddhesh,

starting with this commit, malloc/tst-realloc-mcheck fails if build with
gcc 7.5 on s390x:
Error: realloc (NULL, 0) returned NULL.

With gcc 7.5:
 p = realloc (NULL, 0);
realloc is really called, which is using reallochook, which always
returns NULL:
static void *
reallochook (void *ptr, size_t size, const void *caller)
{
  if (size == 0)
    {
      freehook (ptr, caller);
      return NULL;
    }

With e.g. gcc 9.1:
malloc(0) is called, which is using mallochook. It is using
hdr = (struct hdr *) malloc (sizeof (struct hdr) + size + 1);
...
return (void *) (hdr + 1);
=> != NULL

Can you please have a look? And perhaps also filter out this test?

Bye,
Stefan
  

On 7/6/21 8:20 PM, Stefan Liebler via Libc-alpha wrote:
> Hi Siddhesh,
> 
> starting with this commit, malloc/tst-realloc-mcheck fails if build with
> gcc 7.5 on s390x:
> Error: realloc (NULL, 0) returned NULL.
> 
> With gcc 7.5:
>   p = realloc (NULL, 0);
> realloc is really called, which is using reallochook, which always
> returns NULL:
> static void *
> reallochook (void *ptr, size_t size, const void *caller)
> {
>    if (size == 0)
>      {
>        freehook (ptr, caller);
>        return NULL;
>      }
> 
> With e.g. gcc 9.1:
> malloc(0) is called, which is using mallochook. It is using
> hdr = (struct hdr *) malloc (sizeof (struct hdr) + size + 1);
> ...
> return (void *) (hdr + 1);
> => != NULL
> 
> Can you please have a look? And perhaps also filter out this test?

Thanks, that is an mcheck bug that gcc seems to have masked.  I'll push 
a patch filter it out.  I'll also need to update my hooks patch to fix 
this bug in mcheck.

Thanks,
Siddhesh
  

On Tue, Jul 6, 2021 at 8:54 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> On 7/6/21 8:20 PM, Stefan Liebler via Libc-alpha wrote:
> > Hi Siddhesh,
> >
> > starting with this commit, malloc/tst-realloc-mcheck fails if build with
> > gcc 7.5 on s390x:
> > Error: realloc (NULL, 0) returned NULL.
> >
> > With gcc 7.5:
> >   p = realloc (NULL, 0);
> > realloc is really called, which is using reallochook, which always
> > returns NULL:
> > static void *
> > reallochook (void *ptr, size_t size, const void *caller)
> > {
> >    if (size == 0)
> >      {
> >        freehook (ptr, caller);
> >        return NULL;
> >      }
> >
> > With e.g. gcc 9.1:
> > malloc(0) is called, which is using mallochook. It is using
> > hdr = (struct hdr *) malloc (sizeof (struct hdr) + size + 1);
> > ...
> > return (void *) (hdr + 1);
> > => != NULL
> >
> > Can you please have a look? And perhaps also filter out this test?
>
> Thanks, that is an mcheck bug that gcc seems to have masked.  I'll push
> a patch filter it out.  I'll also need to update my hooks patch to fix
> this bug in mcheck.
>
> Thanks,
> Siddhesh

It also caused:

https://sourceware.org/bugzilla/show_bug.cgi?id=28068

on x32.
  

On Thu, Jul 8, 2021 at 4:37 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jul 6, 2021 at 8:54 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> >
> > On 7/6/21 8:20 PM, Stefan Liebler via Libc-alpha wrote:
> > > Hi Siddhesh,
> > >
> > > starting with this commit, malloc/tst-realloc-mcheck fails if build with
> > > gcc 7.5 on s390x:
> > > Error: realloc (NULL, 0) returned NULL.
> > >
> > > With gcc 7.5:
> > >   p = realloc (NULL, 0);
> > > realloc is really called, which is using reallochook, which always
> > > returns NULL:
> > > static void *
> > > reallochook (void *ptr, size_t size, const void *caller)
> > > {
> > >    if (size == 0)
> > >      {
> > >        freehook (ptr, caller);
> > >        return NULL;
> > >      }
> > >
> > > With e.g. gcc 9.1:
> > > malloc(0) is called, which is using mallochook. It is using
> > > hdr = (struct hdr *) malloc (sizeof (struct hdr) + size + 1);
> > > ...
> > > return (void *) (hdr + 1);
> > > => != NULL
> > >
> > > Can you please have a look? And perhaps also filter out this test?
> >
> > Thanks, that is an mcheck bug that gcc seems to have masked.  I'll push
> > a patch filter it out.  I'll also need to update my hooks patch to fix
> > this bug in mcheck.
> >
> > Thanks,
> > Siddhesh
>
> It also caused:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28068
>
> on x32.

mcheck.c doesn't take MALLOC_ALIGNMENT into account.   Will it be removed
or fixed?
  

On 7/9/21 5:36 AM, H.J. Lu wrote:
>> It also caused:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=28068
>>
>> on x32.
> 
> mcheck.c doesn't take MALLOC_ALIGNMENT into account.   Will it be removed
> or fixed?

Very interesting, I think it's best to exclude the test for mcheck on 
x32 for now.  I'll fix mcheck once the malloc hooks patchset[1] is in.

On a separate note, this test probably deserves to be run on all 
architectures for MALLOC_ALIGNMENT.

Siddhesh
  

Siddhesh Poyarekar <siddhesh@gotplt.org> writes:

> On 7/9/21 5:36 AM, H.J. Lu wrote:
>>> It also caused:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=28068
>>>
>>> on x32.
>> mcheck.c doesn't take MALLOC_ALIGNMENT into account.   Will it be removed
>> or fixed?
>
> Very interesting, I think it's best to exclude the test for mcheck on x32 for
> now.  I'll fix mcheck once the malloc hooks patchset[1] is in.
>

I just noticed this test is also failing on powerpc32. Similar to
x32. Does it make sense to exclude it there too?

--
Matheus Castanho
  

On 7/12/21 11:32 PM, Matheus Castanho wrote:
> I just noticed this test is also failing on powerpc32. Similar to
> x32. Does it make sense to exclude it there too?

I just acked HJ's patch that fixes this.

Siddhesh
  

Siddhesh Poyarekar <siddhesh@gotplt.org> writes:

> On 7/12/21 11:32 PM, Matheus Castanho wrote:
>> I just noticed this test is also failing on powerpc32. Similar to
>> x32. Does it make sense to exclude it there too?
>
> I just acked HJ's patch that fixes this.
>
> Siddhesh

Nice, I just saw it. I confirm it fixes the problem on ppc32 as
well. Thanks!

--
Matheus Castanho