Add tests for atexit/on_exit firing order
Commit Message
Greetings,
While working a on patch for bz14333, I discovered that there are no
tests for ordering of functions registered with atexit/on_exit, and in
particular the case where such function itself registers new exit
handlers.
This patch adds such test. I am using on_exit here because it
conveniently allows passing an argument.
2017-07-10 Paul Pluzhnikov <ppluzhnikov@google.com>
* stdlib/Makefile (tests): Add tst-on_exit
* stdlib/tst-on_exit.c: New.
Comments
On 07/10/2017 11:00 AM, Paul Pluzhnikov wrote:
> Greetings,
>
> While working a on patch for bz14333, I discovered that there are no
> tests for ordering of functions registered with atexit/on_exit, and in
> particular the case where such function itself registers new exit
> handlers.
>
> This patch adds such test. I am using on_exit here because it
> conveniently allows passing an argument.
>
>
> 2017-07-10 Paul Pluzhnikov <ppluzhnikov@google.com>
>
> * stdlib/Makefile (tests): Add tst-on_exit
> * stdlib/tst-on_exit.c: New.
Paul,
Always awesome to see new tests! :-)
* First line of test needs to describe the test, and BZ#.
* Test needs a copyright header.
* Test needs an explanatory paragraph talking about what is
being tested, why, and the expected results.
* Test should use new support driver.
e.g. #include <support/test-driver.c>
Thanks.
On Mon, 10 Jul 2017, Paul Pluzhnikov wrote:
> Greetings,
>
> While working a on patch for bz14333, I discovered that there are no
> tests for ordering of functions registered with atexit/on_exit, and in
> particular the case where such function itself registers new exit
> handlers.
>
> This patch adds such test. I am using on_exit here because it
> conveniently allows passing an argument.
I'd think that it would make sense to test all of atexit, on_exit,
at_quick_exit this way.
On Mon, Jul 10, 2017 at 8:18 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>> This patch adds such test. I am using on_exit here because it
>> conveniently allows passing an argument.
>
> I'd think that it would make sense to test all of atexit, on_exit,
> at_quick_exit this way.
I could be missing something, but I don't see an easy way to test
atexit and at_quick_exit the same way due to them not taking an
argument.
To test atexit, I would have to implement separate fn1 .. fn8 and
hard-code expected call sequence into each of them, wouldn't I?
That would make for a more verbose and more difficult to modify test,
and given that all 3 functions currently share implementation, that
seems like an overkill. Of course if later the implementation is
un-shared, it would be nice to have a test case for each. But it seems
unlikely to ever happen.
Thanks,
On 10/07/17 16:39, Paul Pluzhnikov wrote:
> On Mon, Jul 10, 2017 at 8:18 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>
>>> This patch adds such test. I am using on_exit here because it
>>> conveniently allows passing an argument.
>>
>> I'd think that it would make sense to test all of atexit, on_exit,
>> at_quick_exit this way.
>
> I could be missing something, but I don't see an easy way to test
> atexit and at_quick_exit the same way due to them not taking an
> argument.
>
> To test atexit, I would have to implement separate fn1 .. fn8 and
> hard-code expected call sequence into each of them, wouldn't I?
>
> That would make for a more verbose and more difficult to modify test,
> and given that all 3 functions currently share implementation, that
> seems like an overkill. Of course if later the implementation is
> un-shared, it would be nice to have a test case for each. But it seems
> unlikely to ever happen.
>
you could call __cxa_atexit and __cxa_at_quick_exit
or just use globals.
On Mon, 10 Jul 2017, Paul Pluzhnikov wrote:
> On Mon, Jul 10, 2017 at 8:18 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>
> >> This patch adds such test. I am using on_exit here because it
> >> conveniently allows passing an argument.
> >
> > I'd think that it would make sense to test all of atexit, on_exit,
> > at_quick_exit this way.
>
> I could be missing something, but I don't see an easy way to test
> atexit and at_quick_exit the same way due to them not taking an
> argument.
>
> To test atexit, I would have to implement separate fn1 .. fn8 and
> hard-code expected call sequence into each of them, wouldn't I?
You might have more separate functions, however the call sequence is
encoded. There are various ways with wrapper functions, macros etc. it
should be possible to share the same test structure for all the functions
(e.g. have wrappers for each function that call it with different
arguments, and have my_on_exit select such a wrapper from an array).
> That would make for a more verbose and more difficult to modify test,
> and given that all 3 functions currently share implementation, that
> seems like an overkill. Of course if later the implementation is
> un-shared, it would be nice to have a test case for each. But it seems
> unlikely to ever happen.
Every public interface should have adequate test coverage. All three are
public interfaces, so all three should have test coverage.
On Mon, 10 Jul 2017, Szabolcs Nagy wrote:
> you could call __cxa_atexit and __cxa_at_quick_exit
Well, those should properly have such tests as well.
@@ -80,7 +80,7 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \
tst-strtol-locale tst-strtod-nan-locale tst-strfmon_l \
tst-quick_exit tst-thread-quick_exit tst-width \
tst-width-stdint tst-strfrom tst-strfrom-locale \
- tst-getrandom
+ tst-getrandom tst-on_exit
tests-internal := tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \
tst-tls-atexit tst-tls-atexit-nodelete
tests-static := tst-secure-getenv
new file mode 100644
@@ -0,0 +1,68 @@
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#define MAX_ON_EXIT 10
+static int expected[MAX_ON_EXIT];
+static int next_slot = 0;
+static int next_expected = 0;
+
+static void my_on_exit (void (*fn) (int status, void *));
+
+static void
+fn1 (int status, void *arg)
+{
+ intptr_t k = (intptr_t) arg;
+
+ printf ("fn1:\t\t%p %d\n", fn1, (int) k);
+ if (next_slot < 1 || expected[--next_slot] != k)
+ _exit (1);
+}
+
+static void
+fn2 (int status, void *arg)
+{
+ intptr_t k = (intptr_t) arg;
+
+ printf ("fn2:\t\t%p %d\n", fn2, (int) k);
+ if (next_slot < 1 || expected[--next_slot] != k)
+ _exit (1);
+ my_on_exit (fn1);
+}
+
+static void
+fn3 (int status, void *arg)
+{
+ intptr_t k = (intptr_t) arg;
+
+ printf ("fn3:\t\t%p %d\n", fn3, (int) k);
+ if (next_slot < 1 || expected[--next_slot] != k)
+ _exit (1);
+ my_on_exit (fn2);
+}
+
+static void
+my_on_exit (void (*fn) (int, void *))
+{
+ intptr_t k = ++next_expected;
+
+ printf ("on_exit:\t%p %d\n", fn, (int) k);
+ on_exit (fn, (void *) k);
+ assert (next_slot < MAX_ON_EXIT);
+ expected[next_slot++] = k;
+}
+
+static int
+do_test (void)
+{
+ my_on_exit (fn2);
+ my_on_exit (fn1);
+ my_on_exit (fn2);
+ my_on_exit (fn3);
+ return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"