[v3,02/10] Add mcheck tests to malloc
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
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(-)
Comments
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
@@ -155,6 +155,7 @@ xtests: tests $(xtests-special)
else
tests: $(tests:%=$(objpfx)%.out) $(tests-internal:%=$(objpfx)%.out) \
$(tests-container:%=$(objpfx)%.out) \
+ $(tests-mcheck:%=$(objpfx)%-mcheck.out) \
$(tests-malloc-check:%=$(objpfx)%-malloc-check.out) \
$(tests-special) $(tests-printers-out)
xtests: tests $(xtests:%=$(objpfx)%.out) $(xtests-special)
@@ -166,7 +167,8 @@ ifeq ($(run-built-tests),no)
tests-expected =
else
tests-expected = $(tests) $(tests-internal) $(tests-printers) \
- $(tests-container) $(tests-malloc-check:%=%-malloc-check)
+ $(tests-container) $(tests-malloc-check:%=%-malloc-check) \
+ $(tests-mcheck:%=%-mcheck)
endif
tests:
$(..)scripts/merge-test-results.sh -s $(objpfx) $(subdir) \
@@ -192,6 +194,7 @@ else
binaries-pie-tests =
binaries-pie-notests =
endif
+binaries-mcheck-tests = $(tests-mcheck:%=%-mcheck)
binaries-malloc-check-tests = $(tests-malloc-check:%=%-malloc-check)
else
binaries-all-notests =
@@ -202,6 +205,7 @@ binaries-static-tests =
binaries-static =
binaries-pie-tests =
binaries-pie-notests =
+binaries-mcheck-tests =
binaries-malloc-check-tests =
endif
@@ -226,6 +230,15 @@ $(addprefix $(objpfx),$(binaries-shared-tests)): %: %.o \
$(+link-tests)
endif
+ifneq "$(strip $(binaries-mcheck-tests))" ""
+$(addprefix $(objpfx),$(binaries-mcheck-tests)): %-mcheck: %.o \
+ $(link-extra-libs-tests) \
+ $(sort $(filter $(common-objpfx)lib%,$(link-libc))) \
+ $(common-objpfx)malloc/libmcheck.a \
+ $(addprefix $(csu-objpfx),start.o) $(+preinit) $(+postinit)
+ $(+link-tests)
+endif
+
ifneq "$(strip $(binaries-malloc-check-tests))" ""
$(addprefix $(objpfx),$(binaries-malloc-check-tests)): %-malloc-check: %.o \
$(link-extra-libs-tests) \
@@ -77,6 +77,26 @@ tests-exclude-malloc-check = tst-malloc-check tst-malloc-usable \
# Run all tests with MALLOC_CHECK_=3
tests-malloc-check = $(filter-out $(tests-exclude-malloc-check),$(tests))
+# Tests that don't play well with mcheck. They are either bugs in mcheck or
+# the tests expect specific internal behavior that is changed due to linking to
+# libmcheck.a.
+tests-exclude-mcheck = tst-mallocstate \
+ tst-safe-linking tst-malloc-usable \
+ tst-malloc-backtrace \
+ tst-malloc-fork-deadlock \
+ tst-malloc-stats-cancellation \
+ tst-malloc-tcache-leak \
+ tst-malloc-thread-exit \
+ tst-malloc-thread-fail \
+ tst-malloc-usable-static \
+ tst-malloc-usable-static-tunables \
+ tst-malloc-usable-tunables \
+ tst-malloc_info \
+ tst-memalign \
+ tst-posix_memalign
+
+tests-mcheck = $(filter-out $(tests-exclude-mcheck), $(tests))
+
routines = malloc morecore mcheck mtrace obstack reallocarray \
scratch_buffer_dupfree \
scratch_buffer_grow scratch_buffer_grow_preserve \
@@ -118,6 +138,11 @@ $(objpfx)tst-mallocfork3: $(shared-thread-library)
$(objpfx)tst-mallocfork3-mcheck: $(shared-thread-library)
$(objpfx)tst-malloc-fork-deadlock: $(shared-thread-library)
$(objpfx)tst-malloc-stats-cancellation: $(shared-thread-library)
+$(objpfx)tst-malloc-backtrace-mcheck: $(shared-thread-library)
+$(objpfx)tst-malloc-thread-exit-mcheck: $(shared-thread-library)
+$(objpfx)tst-malloc-thread-fail-mcheck: $(shared-thread-library)
+$(objpfx)tst-malloc-fork-deadlock-mcheck: $(shared-thread-library)
+$(objpfx)tst-malloc-stats-cancellation-mcheck: $(shared-thread-library)
$(objpfx)tst-malloc-backtrace-malloc-check: $(shared-thread-library)
$(objpfx)tst-malloc-thread-exit-malloc-check: $(shared-thread-library)
$(objpfx)tst-malloc-thread-fail-malloc-check: $(shared-thread-library)
@@ -253,17 +278,24 @@ $(foreach o,$(all-object-suffixes),$(objpfx)malloc$(o)): arena.c hooks.c
$(tests:%=$(objpfx)%.o): CPPFLAGS += -DTEST_NO_MALLOPT
$(objpfx)tst-interpose-nothread: $(objpfx)tst-interpose-aux-nothread.o
+$(objpfx)tst-interpose-nothread-mcheck: $(objpfx)tst-interpose-aux-nothread.o
$(objpfx)tst-interpose-nothread-malloc-check: \
$(objpfx)tst-interpose-aux-nothread.o
$(objpfx)tst-interpose-thread: \
$(objpfx)tst-interpose-aux-thread.o $(shared-thread-library)
+$(objpfx)tst-interpose-thread-mcheck: \
+ $(objpfx)tst-interpose-aux-thread.o $(shared-thread-library)
$(objpfx)tst-interpose-thread-malloc-check: \
$(objpfx)tst-interpose-aux-thread.o $(shared-thread-library)
$(objpfx)tst-interpose-static-nothread: $(objpfx)tst-interpose-aux-nothread.o
+$(objpfx)tst-interpose-static-nothread-mcheck: \
+ $(objpfx)tst-interpose-aux-nothread.o
$(objpfx)tst-interpose-static-nothread-malloc-check: \
$(objpfx)tst-interpose-aux-nothread.o
$(objpfx)tst-interpose-static-thread: \
$(objpfx)tst-interpose-aux-thread.o $(static-thread-library)
+$(objpfx)tst-interpose-static-thread-mcheck: \
+ $(objpfx)tst-interpose-aux-thread.o $(static-thread-library)
$(objpfx)tst-interpose-static-thread-malloc-check: \
$(objpfx)tst-interpose-aux-thread.o $(static-thread-library)
@@ -280,6 +312,9 @@ $(objpfx)tst-dynarray-fail-mem.out: $(objpfx)tst-dynarray-fail.out
$(objpfx)tst-malloc-tcache-leak: $(shared-thread-library)
$(objpfx)tst-malloc_info: $(shared-thread-library)
$(objpfx)tst-mallocfork2: $(shared-thread-library)
+$(objpfx)tst-malloc-tcache-leak-mcheck: $(shared-thread-library)
+$(objpfx)tst-malloc_info-mcheck: $(shared-thread-library)
+$(objpfx)tst-mallocfork2-mcheck: $(shared-thread-library)
$(objpfx)tst-malloc-tcache-leak-malloc-check: $(shared-thread-library)
$(objpfx)tst-malloc_info-malloc-check: $(shared-thread-library)
$(objpfx)tst-mallocfork2-malloc-check: $(shared-thread-library)