Fix tests that are testing obsoleted functionality
Commit Message
Here is a patch to fix three tests; malloc/tst-mallocstate.c,
math/test-matherr.c, and math/test-matherr-2.c. These tests
test functionality that is no longer in the latest GLIBC but
should still exist for programs linked with an older GLIBC.
If you have a new ABI (like ILP32 aarch64) which never existed
in the older form, these tests will fail. tst-mallocstate.c
and test-matherr.c give link errors and test-matherr-2.c
compiles and links but exits with status 1.
See https://sourceware.org/ml/libc-alpha/2017-08/msg01127.html
and https://sourceware.org/ml/libc-alpha/2016-12/msg00527.html
for some more discussions on this problem.
This patch fixes the tests so that they will compile and
link and, if on a platform that does not support the functionality
being tested return an UNSUPPORTED return code. They continue
to run and pass as before on other platforms. To do this
I created a new macro TEST_COMPAT, similar to SHLIB_COMPAT
that can be used to check for the abi versions available
when building the test.
Tested on aarch64 with LP64 (where they run) and ILP32 (where
they return unsupported). While I tested this with the aarch64
ILP32 ABI, they changes are not specific to any ABI so I would
like to check them in now and not wait for aarch64 ILP32 or some
other new ABI get checked in first.
Steve Ellcey
sellcey@cavium.com
2017-08-24 Steve Ellcey <sellcey@cavium.com>
* include/shlib-compat.h (TEST_COMPAT): New Macro.
* malloc/tst-mallocstate.c: Convert from test-skeleton
to test-driver. Ifdef code using TEST_COMPAT macro.
* math/test-matherr-2.c: Ifdef test using TEST_COMPAT macro.
* math/test-matherr.c: Likewise.
Comments
On 08/24/2017 06:55 PM, Steve Ellcey wrote:
> Here is a patch to fix three tests; malloc/tst-mallocstate.c,
> math/test-matherr.c, and math/test-matherr-2.c. These tests
> test functionality that is no longer in the latest GLIBC but
> should still exist for programs linked with an older GLIBC.
> If you have a new ABI (like ILP32 aarch64) which never existed
> in the older form, these tests will fail. tst-mallocstate.c
> and test-matherr.c give link errors and test-matherr-2.c
> compiles and links but exits with status 1.
>
> See https://sourceware.org/ml/libc-alpha/2017-08/msg01127.html
> and https://sourceware.org/ml/libc-alpha/2016-12/msg00527.html
> for some more discussions on this problem.
>
> This patch fixes the tests so that they will compile and
> link and, if on a platform that does not support the functionality
> being tested return an UNSUPPORTED return code. They continue
> to run and pass as before on other platforms. To do this
> I created a new macro TEST_COMPAT, similar to SHLIB_COMPAT
> that can be used to check for the abi versions available
> when building the test.
>
> Tested on aarch64 with LP64 (where they run) and ILP32 (where
> they return unsupported). While I tested this with the aarch64
> ILP32 ABI, they changes are not specific to any ABI so I would
> like to check them in now and not wait for aarch64 ILP32 or some
> other new ABI get checked in first.
>
> Steve Ellcey
> sellcey@cavium.com
>
>
> 2017-08-24 Steve Ellcey <sellcey@cavium.com>
>
> * include/shlib-compat.h (TEST_COMPAT): New Macro.
> * malloc/tst-mallocstate.c: Convert from test-skeleton
> to test-driver. Ifdef code using TEST_COMPAT macro.
> * math/test-matherr-2.c: Ifdef test using TEST_COMPAT macro.
> * math/test-matherr.c: Likewise.
>
From a high-level perspective this looks good to me.
I know we're not supposed to do anything special on master for
AArch64 ILP32, but this kind of generic cleanup that future-proofs
the test for new arches (we are expecting RISC-V port to turn
up soon and they would have seen this) is a win win.
I'd like one other developer to ACK before you commit this, since
I'd like to hear a few more opinions on how we might handle this.
On Thu, 24 Aug 2017 16:11:21 PDT (-0700), carlos@redhat.com wrote:
> On 08/24/2017 06:55 PM, Steve Ellcey wrote:
>> Here is a patch to fix three tests; malloc/tst-mallocstate.c,
>> math/test-matherr.c, and math/test-matherr-2.c. These tests
>> test functionality that is no longer in the latest GLIBC but
>> should still exist for programs linked with an older GLIBC.
>> If you have a new ABI (like ILP32 aarch64) which never existed
>> in the older form, these tests will fail. tst-mallocstate.c
>> and test-matherr.c give link errors and test-matherr-2.c
>> compiles and links but exits with status 1.
>>
>> See https://sourceware.org/ml/libc-alpha/2017-08/msg01127.html
>> and https://sourceware.org/ml/libc-alpha/2016-12/msg00527.html
>> for some more discussions on this problem.
>>
>> This patch fixes the tests so that they will compile and
>> link and, if on a platform that does not support the functionality
>> being tested return an UNSUPPORTED return code. They continue
>> to run and pass as before on other platforms. To do this
>> I created a new macro TEST_COMPAT, similar to SHLIB_COMPAT
>> that can be used to check for the abi versions available
>> when building the test.
>>
>> Tested on aarch64 with LP64 (where they run) and ILP32 (where
>> they return unsupported). While I tested this with the aarch64
>> ILP32 ABI, they changes are not specific to any ABI so I would
>> like to check them in now and not wait for aarch64 ILP32 or some
>> other new ABI get checked in first.
>>
>> Steve Ellcey
>> sellcey@cavium.com
>>
>>
>> 2017-08-24 Steve Ellcey <sellcey@cavium.com>
>>
>> * include/shlib-compat.h (TEST_COMPAT): New Macro.
>> * malloc/tst-mallocstate.c: Convert from test-skeleton
>> to test-driver. Ifdef code using TEST_COMPAT macro.
>> * math/test-matherr-2.c: Ifdef test using TEST_COMPAT macro.
>> * math/test-matherr.c: Likewise.
>>
>
> From a high-level perspective this looks good to me.
>
> I know we're not supposed to do anything special on master for
> AArch64 ILP32, but this kind of generic cleanup that future-proofs
> the test for new arches (we are expecting RISC-V port to turn
> up soon and they would have seen this) is a win win.
Thanks for the heads up -- our port is still a WIP, I was waiting until we have
Linux a bit farther along before submitting a v2. I'm pretty sure I have a
version of these floating around my tree somewhere, now it can disappear when I
merge.
> I'd like one other developer to ACK before you commit this, since
> I'd like to hear a few more opinions on how we might handle this.
On 25/08/17 00:11, Carlos O'Donell wrote:
> I know we're not supposed to do anything special on master for
> AArch64 ILP32, but this kind of generic cleanup that future-proofs
note that i plan to commit ilp32 changes that are not
depending on the kernel or libc abi, given that partial
ilp32 support is already in since commit
389d1f1b232b3d6b9d73ee2c50e543ace6675621
(and there were some other commits to ease ilp32 merging)
On Thu, 2017-08-24 at 19:11 -0400, Carlos O'Donell wrote:
>
> From a high-level perspective this looks good to me.
>
> I know we're not supposed to do anything special on master for
> AArch64 ILP32, but this kind of generic cleanup that future-proofs
> the test for new arches (we are expecting RISC-V port to turn
> up soon and they would have seen this) is a win win.
>
> I'd like one other developer to ACK before you commit this, since
> I'd like to hear a few more opinions on how we might handle this.
Any other comments on this? Any objections to the patch?
Steve Ellcey
sellcey@cavium.com
On 28/08/17 21:08, Steve Ellcey wrote:
> On Thu, 2017-08-24 at 19:11 -0400, Carlos O'Donell wrote:
>>
>> From a high-level perspective this looks good to me.
>>
>> I know we're not supposed to do anything special on master for
>> AArch64 ILP32, but this kind of generic cleanup that future-proofs
>> the test for new arches (we are expecting RISC-V port to turn
>> up soon and they would have seen this) is a win win.
>>
>> I'd like one other developer to ACK before you commit this, since
>> I'd like to hear a few more opinions on how we might handle this.
>
> Any other comments on this? Any objections to the patch?
>
while you wait for feedback on master,
i committed the current patch to arm/ilp32
since it is needed for clean tests there.
(i'm still not decided on what's the best
way to manage the branch, i plan to do
rebases and then such commits can be
dropped if master have them too, but i'm
open to feedback if there are issues with
rebasing public branches.)
On Thu, 2017-08-24 at 19:11 -0400, Carlos O'Donell wrote:
> On 08/24/2017 06:55 PM, Steve Ellcey wrote:
> >
> > 2017-08-24 Steve Ellcey <sellcey@cavium.com>
> >
> > * include/shlib-compat.h (TEST_COMPAT): New Macro.
> > * malloc/tst-mallocstate.c: Convert from test-skeleton
> > to test-driver. Ifdef code using TEST_COMPAT macro.
> > * math/test-matherr-2.c: Ifdef test using TEST_COMPAT macro.
> > * math/test-matherr.c: Likewise.
> >
> From a high-level perspective this looks good to me.
>
> I know we're not supposed to do anything special on master for
> AArch64 ILP32, but this kind of generic cleanup that future-proofs
> the test for new arches (we are expecting RISC-V port to turn
> up soon and they would have seen this) is a win win.
>
> I'd like one other developer to ACK before you commit this, since
> I'd like to hear a few more opinions on how we might handle this.
Patch ping. Does anyone else have any comments on this approach
to fixing these tests?
Steve Ellcey
sellcey@cavium.com
On 09/05/2017 07:49 PM, Steve Ellcey wrote:
> Patch ping. Does anyone else have any comments on this approach
> to fixing these tests?
Please go ahead if this works for you. We can adjust it later if
necessary (or if we have a more general framework for such matters).
Thanks,
Florian
@@ -97,4 +97,14 @@
compat_symbol (libc, name, aliasname, version);
# endif
+/* The TEST_COMPAT macro acts just like the SHLIB_COMPAT macro except
+ that it does not check IS_IN. It is used by tests that are testing
+ functionality that is only available in specific GLIBC versions. */
+
+# define TEST_COMPAT(lib, introduced, obsoleted) \
+ _TEST_COMPAT (lib, introduced, obsoleted)
+# define _TEST_COMPAT(lib, introduced, obsoleted) \
+ (!(ABI_##lib##_##obsoleted - 0) \
+ || ((ABI_##lib##_##introduced - 0) < (ABI_##lib##_##obsoleted - 0)))
+
#endif /* shlib-compat.h */
@@ -23,19 +23,20 @@
#include <string.h>
#include <libc-symbols.h>
#include <shlib-compat.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/test-driver.h>
#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);
int malloc_set_state (void *);
compat_symbol_reference (libc, malloc_set_state, malloc_set_state, GLIBC_2_0);
-static int do_test (void);
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
-
/* Maximum object size in the fake heap. */
enum { max_size = 64 };
@@ -63,9 +64,9 @@ static size_t *next_heap_chunk;
/* Copied from malloc.c and hooks.c. The version is deliberately
lower than the final version of malloc_set_state. */
-#define NBINS 128
-#define MALLOC_STATE_MAGIC 0x444c4541l
-#define MALLOC_STATE_VERSION (0 * 0x100l + 4l)
+# define NBINS 128
+# define MALLOC_STATE_MAGIC 0x444c4541l
+# define MALLOC_STATE_VERSION (0 * 0x100l + 4l)
static struct
{
long magic;
@@ -117,12 +118,7 @@ dumped_heap_alloc (size_t length)
/* Round up the allocation size to the heap alignment. */
chunk_size += heap_alignment_mask;
chunk_size &= ~heap_alignment_mask;
- if ((chunk_size & 3) != 0)
- {
- /* The lower three bits in the chunk size have to be 0. */
- write_message ("error: dumped_heap_alloc computed invalid chunk size\n");
- _exit (1);
- }
+ TEST_VERIFY_EXIT ((chunk_size & 3) == 0);
if (next_heap_chunk == NULL)
/* Initialize the top of the heap. Add one word of zero padding,
to match existing practice. */
@@ -244,11 +240,7 @@ shuffle_allocation_tasks (void)
/* Pick pair in the tail of the array. */
int j = i + (rand_next (&global_seed)
% ((unsigned) (allocation_task_count - i)));
- if (j < 0 || j >= allocation_task_count)
- {
- write_message ("error: test bug in shuffle\n");
- _exit (1);
- }
+ TEST_VERIFY_EXIT (j >= 0 && j < allocation_task_count);
/* Exchange. */
struct allocation_task tmp = allocation_tasks[i];
allocation_tasks[i] = allocation_tasks[j];
@@ -298,7 +290,8 @@ static volatile bool heap_initialized;
static void
init_heap (void)
{
- write_message ("info: performing heap initialization\n");
+ if (test_verbose)
+ printf ("info: performing heap initialization\n");
heap_initialized = true;
/* Populate the dumped heap. */
@@ -312,11 +305,7 @@ init_heap (void)
save_state.av[2] = (void *) (next_heap_chunk - 1);
/* Integrate the dumped heap into the process heap. */
- if (malloc_set_state (&save_state) != 0)
- {
- write_message ("error: malloc_set_state failed\n");
- _exit (1);
- }
+ TEST_VERIFY_EXIT (malloc_set_state (&save_state) == 0);
}
/* Interpose the initialization callback. */
@@ -389,15 +378,12 @@ static int
do_test (void)
{
my_free (malloc (1));
- if (!heap_initialized)
- {
- printf ("error: heap was not initialized by malloc\n");
- return 1;
- }
+ TEST_VERIFY_EXIT (heap_initialized);
/* The first pass performs the randomly generated allocation
tasks. */
- write_message ("info: first pass through allocation tasks\n");
+ if (test_verbose)
+ printf ("info: first pass through allocation tasks\n");
full_heap_check ();
/* Execute the post-undump tasks in a random order. */
@@ -451,14 +437,15 @@ do_test (void)
break;
case action_count:
- abort ();
+ FAIL_EXIT1 ("task->action should never be action_count");
}
full_heap_check ();
}
/* The second pass frees the objects which were allocated during the
first pass. */
- write_message ("info: second pass through allocation tasks\n");
+ if (test_verbose)
+ printf ("info: second pass through allocation tasks\n");
shuffle_allocation_tasks ();
for (int i = 0; i < allocation_task_count; ++i)
@@ -480,7 +467,7 @@ do_test (void)
break;
case action_count:
- abort ();
+ FAIL_EXIT1 ("task->action should never be action_count");
}
full_heap_check ();
}
@@ -503,3 +490,12 @@ do_test (void)
return errors;
}
+#else
+static int
+do_test (void)
+{
+ return 77;
+}
+#endif
+
+#include <support/test-driver.c>
@@ -22,8 +22,11 @@
#include <math-svid-compat.h>
#include <shlib-compat.h>
-#undef matherr
-#undef _LIB_VERSION
+
+#if TEST_COMPAT (libm, GLIBC_2_0, GLIBC_2_27)
+
+# undef matherr
+# undef _LIB_VERSION
compat_symbol_reference (libm, matherr, matherr, GLIBC_2_0);
compat_symbol_reference (libm, _LIB_VERSION, _LIB_VERSION, GLIBC_2_0);
@@ -45,5 +48,12 @@ do_test (void)
acos (2.0);
return fail;
}
+#else
+static int
+do_test (void)
+{
+ return 77;
+}
+#endif
#include <support/test-driver.c>
@@ -22,8 +22,11 @@
#include <math-svid-compat.h>
#include <shlib-compat.h>
-#undef matherr
-#undef _LIB_VERSION
+
+#if TEST_COMPAT (libm, GLIBC_2_0, GLIBC_2_27)
+
+# undef matherr
+# undef _LIB_VERSION
compat_symbol_reference (libm, matherr, matherr, GLIBC_2_0);
compat_symbol_reference (libm, _LIB_VERSION, _LIB_VERSION, GLIBC_2_0);
@@ -44,5 +47,12 @@ do_test (void)
acos (2.0);
return fail;
}
+#else
+static int
+do_test (void)
+{
+ return 77;
+}
+#endif
#include <support/test-driver.c>