Message ID | CALoOobMo+n2qfftAYG6Tyibg7Y7iCDn0PEMuBhvJyzwT4vVxZA@mail.gmail.com |
---|---|
State | New |
Headers | show |
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.
diff --git a/stdlib/Makefile b/stdlib/Makefile index 0314d5926b..cc9f9215e4 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -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 diff --git a/stdlib/tst-on_exit.c b/stdlib/tst-on_exit.c new file mode 100644 index 0000000000..0de3b68525 --- /dev/null +++ b/stdlib/tst-on_exit.c @@ -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"