[PATCH] malloc: Turn tst-mallocstate into a non-internal test

[PATCH] malloc: Turn tst-mallocstate into a non-internal test

[Florian Weimer]

compat_symbol_reference no longer needs tests-internal.  Do not build
the test at all for newer targets, so that no spurious UNSUPPORTED
result is generated.

---
 malloc/Makefile          | 10 ++++++----
 malloc/tst-mallocstate.c | 11 ++---------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/malloc/Makefile b/malloc/Makefile
index 583bbefb0d..641967c512 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -47,7 +47,12 @@ tests-static := \
 	 tst-interpose-static-thread \
 	 tst-malloc-usable-static \
 
-tests-internal := tst-mallocstate tst-scratch_buffer
+# Test for the malloc_set_state symbol removed in glibc 2.25.
+ifeq ($(have-GLIBC_2.24)$(build-shared),yesyes)
+tests += tst-mallocstate
+endif
+
+tests-internal := tst-scratch_buffer
 
 # The dynarray framework is only available inside glibc.
 tests-internal += \
@@ -118,9 +123,6 @@ $(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)
 
-# Export the __malloc_initialize_hook variable to libc.so.
-LDFLAGS-tst-mallocstate = -rdynamic
-
 # These should be removed by `make clean'.
 extra-objs = mcheck-init.o libmcheck.a
 others-extras = mcheck-init.o
diff --git a/malloc/tst-mallocstate.c b/malloc/tst-mallocstate.c
index 776f606863..ba726505e7 100644
--- a/malloc/tst-mallocstate.c
+++ b/malloc/tst-mallocstate.c
@@ -29,8 +29,6 @@
 
 #include "malloc.h"
 
-#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_25)
-
 /* Make the compatibility symbols availabile to this test case.  */
 void *malloc_get_state (void);
 compat_symbol_reference (libc, malloc_get_state, malloc_get_state, GLIBC_2_0);
@@ -310,6 +308,8 @@ init_heap (void)
 
 /* Interpose the initialization callback.  */
 void (*volatile __malloc_initialize_hook) (void) = init_heap;
+compat_symbol_reference (libc, __malloc_initialize_hook,
+                         __malloc_initialize_hook, GLIBC_2_0);
 
 /* Simulate occasional unrelated heap activity in the non-dumped
    heap.  */
@@ -490,12 +490,5 @@ do_test (void)
 
   return errors;
 }
-#else
-static int
-do_test (void)
-{
-  return 77;
-}
-#endif
 
 #include <support/test-driver.c>

Re: [PATCH] malloc: Turn tst-mallocstate into a non-internal test

[DJ Delorie]

Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:
> compat_symbol_reference no longer needs tests-internal.  Do not build
> the test at all for newer targets, so that no spurious UNSUPPORTED
> result is generated.

If this is your only goal, changing "return 77" to "return 0" would have
sufficed ;-)

> -tests-internal := tst-mallocstate tst-scratch_buffer
> +# Test for the malloc_set_state symbol removed in glibc 2.25.
> +ifeq ($(have-GLIBC_2.24)$(build-shared),yesyes)
> +tests += tst-mallocstate
> +endif

If a platform has been around long enough to have been released in glibc
2.24, then we want to add this test.  Ok.

> -# Export the __malloc_initialize_hook variable to libc.so.
> -LDFLAGS-tst-mallocstate = -rdynamic
> -

This is unexplained?  I assume it's part of whatever
"compat_symbol_reference no longer needs tests-internal" encompases.

> -#if TEST_COMPAT (libc, GLIBC_2_0, GLIBC_2_25)

If the test only runs on 2.24-or-older platforms, we don't need to test
for 2.25-or-newer platforms.  Ok.

>  /* Interpose the initialization callback.  */
>  void (*volatile __malloc_initialize_hook) (void) = init_heap;
> +compat_symbol_reference (libc, __malloc_initialize_hook,
> +                         __malloc_initialize_hook, GLIBC_2_0);

How does this work?  On x86-64, that symbol is GLIBC_2.2.5, not
GLIBC_2.0.

Re: [PATCH] malloc: Turn tst-mallocstate into a non-internal test

[Florian Weimer]

* DJ Delorie:

>>  /* Interpose the initialization callback.  */
>>  void (*volatile __malloc_initialize_hook) (void) = init_heap;
>> +compat_symbol_reference (libc, __malloc_initialize_hook,
>> +                         __malloc_initialize_hook, GLIBC_2_0);
>
> How does this work?  On x86-64, that symbol is GLIBC_2.2.5, not
> GLIBC_2.0.

On x86-64, GLIBC_2_0 eventually expands to "GLIBC_2.2.5".  See
abi-versions.h in the build tree.

I'm going to add this to the commit message: “Use
compat_symbol_reference for __malloc_initialize_hook as well,
eliminating the need for -rdynamic.”

Thanks,
Florian

Re: [PATCH] malloc: Turn tst-mallocstate into a non-internal test

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
> On x86-64, GLIBC_2_0 eventually expands to "GLIBC_2.2.5".  See
> abi-versions.h in the build tree.

Ah!  Ok.

> I'm going to add this to the commit message: “Use
> compat_symbol_reference for __malloc_initialize_hook as well,
> eliminating the need for -rdynamic.”

Ok.

LGTM then :-)

Reviewed-by: DJ Delorie <dj@redhat.com>