* [patch] Add tests for atexit/on_exit firing order @ 2017-07-10 15:01 Paul Pluzhnikov 2017-07-10 15:09 ` Carlos O'Donell 2017-07-10 15:18 ` Joseph Myers 0 siblings, 2 replies; 13+ messages in thread From: Paul Pluzhnikov @ 2017-07-10 15:01 UTC (permalink / raw) To: GLIBC Devel [-- Attachment #1: Type: text/plain, Size: 499 bytes --] 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 Pluzhnikov [-- Attachment #2: glibc-on_exit-20170710.txt --] [-- Type: text/plain, Size: 2120 bytes --] 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" ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-10 15:01 [patch] Add tests for atexit/on_exit firing order Paul Pluzhnikov @ 2017-07-10 15:09 ` Carlos O'Donell 2017-07-10 15:18 ` Joseph Myers 1 sibling, 0 replies; 13+ messages in thread From: Carlos O'Donell @ 2017-07-10 15:09 UTC (permalink / raw) To: Paul Pluzhnikov, GLIBC Devel 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. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-10 15:01 [patch] Add tests for atexit/on_exit firing order Paul Pluzhnikov 2017-07-10 15:09 ` Carlos O'Donell @ 2017-07-10 15:18 ` Joseph Myers 2017-07-10 15:39 ` Paul Pluzhnikov 1 sibling, 1 reply; 13+ messages in thread From: Joseph Myers @ 2017-07-10 15:18 UTC (permalink / raw) To: Paul Pluzhnikov; +Cc: GLIBC Devel 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. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-10 15:18 ` Joseph Myers @ 2017-07-10 15:39 ` Paul Pluzhnikov 2017-07-10 15:45 ` Szabolcs Nagy 2017-07-10 15:55 ` Joseph Myers 0 siblings, 2 replies; 13+ messages in thread From: Paul Pluzhnikov @ 2017-07-10 15:39 UTC (permalink / raw) To: Joseph Myers; +Cc: GLIBC Devel 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, -- Paul Pluzhnikov ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-10 15:39 ` Paul Pluzhnikov @ 2017-07-10 15:45 ` Szabolcs Nagy 2017-07-10 15:56 ` Joseph Myers 2017-07-10 15:55 ` Joseph Myers 1 sibling, 1 reply; 13+ messages in thread From: Szabolcs Nagy @ 2017-07-10 15:45 UTC (permalink / raw) To: Paul Pluzhnikov, Joseph Myers; +Cc: nd, GLIBC Devel 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-10 15:45 ` Szabolcs Nagy @ 2017-07-10 15:56 ` Joseph Myers 2017-07-24 15:50 ` Paul Pluzhnikov 0 siblings, 1 reply; 13+ messages in thread From: Joseph Myers @ 2017-07-10 15:56 UTC (permalink / raw) To: Szabolcs Nagy; +Cc: Paul Pluzhnikov, nd, GLIBC Devel 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. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-10 15:56 ` Joseph Myers @ 2017-07-24 15:50 ` Paul Pluzhnikov 2017-07-24 17:05 ` Paul Pluzhnikov 0 siblings, 1 reply; 13+ messages in thread From: Paul Pluzhnikov @ 2017-07-24 15:50 UTC (permalink / raw) To: Joseph Myers; +Cc: Szabolcs Nagy, nd, GLIBC Devel [-- Attachment #1: Type: text/plain, Size: 664 bytes --] On Mon, Jul 10, 2017 at 8:56 AM, Joseph Myers <joseph@codesourcery.com> wrote: > 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. Revised patch attached. Thanks. 2017-07-24 Paul Pluzhnikov <ppluzhnikov@google.com> * stdlib/Makefile (tst-atexit, tst-at_quick_exit): New tests (tst-cxa_atexit, tst-on_exit): Likewise * stdlib/tst-atexit-common.c: New. * stdlib/tst-atexit.c: New. * stdlib/tst-at_quick_exit.c: New. * stdlib/tst-cxa_atexit.c: New. * stdlib/tst-on_exit.c: New. -- Paul Pluzhnikov [-- Attachment #2: glibc-atexit-20170724.txt --] [-- Type: text/plain, Size: 7963 bytes --] diff --git a/stdlib/Makefile b/stdlib/Makefile index 0314d5926b..94cbd0e479 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -80,7 +80,9 @@ 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-atexit tst-at_quick_exit \ + tst-cxa_atexit 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-at_quick_exit.c b/stdlib/tst-at_quick_exit.c new file mode 100644 index 0000000000..c86bbe3455 --- /dev/null +++ b/stdlib/tst-at_quick_exit.c @@ -0,0 +1,25 @@ +/* Test that functions registered via at_auick_exit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define ATEXIT(fn) at_quick_exit (fn) +#define EXIT(x) quick_exit (x) + +#include <stdlib/tst-atexit-common.c> diff --git a/stdlib/tst-atexit-common.c b/stdlib/tst-atexit-common.c new file mode 100644 index 0000000000..a585614733 --- /dev/null +++ b/stdlib/tst-atexit-common.c @@ -0,0 +1,86 @@ +/* Helper file for tst-{atexit,at_quick_exit,cxa_atexit,on_exit}. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <assert.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#define MAX_ATEXIT 20 +static char crumbs[MAX_ATEXIT]; +static int next_slot = 0; + +static void +fn0 (void) +{ + crumbs[next_slot++] = '0'; +} + +static void +fn1 (void) +{ + crumbs[next_slot++] = '1'; +} + +static void +fn2 (void) +{ + crumbs[next_slot++] = '2'; + ATEXIT (fn1); +} + +static void +fn3 (void) +{ + crumbs[next_slot++] = '3'; + ATEXIT (fn2); + ATEXIT (fn0); +} + +static void +fn_final (void) +{ + const char expected[] = "3021121130211"; + if (strcmp (crumbs, expected) == 0) + _exit (0); + + printf ("crumbs: %s\n", crumbs); + printf ("expected: %s\n", expected); + _exit (1); +} + +static int +do_test (void) +{ + /* Register this first so it can verify expected order of the rest. */ + ATEXIT (fn_final); + + ATEXIT (fn1); + ATEXIT (fn3); + ATEXIT (fn1); + ATEXIT (fn2); + ATEXIT (fn1); + ATEXIT (fn3); + + EXIT (0); +} + +#define TEST_FUNCTION do_test +#include <support/test-driver.c> diff --git a/stdlib/tst-atexit.c b/stdlib/tst-atexit.c new file mode 100644 index 0000000000..8ca401bcbf --- /dev/null +++ b/stdlib/tst-atexit.c @@ -0,0 +1,25 @@ +/* Test that functions registered via atexit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define ATEXIT(fn) atexit (fn) +#define EXIT(x) exit (x) + +#include <stdlib/tst-atexit-common.c> diff --git a/stdlib/tst-cxa_atexit.c b/stdlib/tst-cxa_atexit.c new file mode 100644 index 0000000000..46fa04a1f7 --- /dev/null +++ b/stdlib/tst-cxa_atexit.c @@ -0,0 +1,27 @@ +/* Test that functions registered via __cxa_atexit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +extern int __cxa_atexit (void (*func) (void *), void *arg, void *d); + +#define ATEXIT(fn) __cxa_atexit ((void (*) (void *)) fn, (void *) 0, (void *) 0) +#define EXIT(x) exit (x) + +#include <stdlib/tst-atexit-common.c> diff --git a/stdlib/tst-on_exit.c b/stdlib/tst-on_exit.c new file mode 100644 index 0000000000..8dda872154 --- /dev/null +++ b/stdlib/tst-on_exit.c @@ -0,0 +1,25 @@ +/* Test that functions registered via on_exit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define ATEXIT(fn) on_exit ((void (*) (int, void *)) fn, (void *) 0) +#define EXIT(x) exit (x) + +#include <stdlib/tst-atexit-common.c> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-24 15:50 ` Paul Pluzhnikov @ 2017-07-24 17:05 ` Paul Pluzhnikov 2017-07-31 18:05 ` Paul Pluzhnikov 2017-08-16 16:36 ` Carlos O'Donell 0 siblings, 2 replies; 13+ messages in thread From: Paul Pluzhnikov @ 2017-07-24 17:05 UTC (permalink / raw) To: Joseph Myers; +Cc: Szabolcs Nagy, nd, GLIBC Devel [-- Attachment #1: Type: text/plain, Size: 828 bytes --] On Mon, Jul 24, 2017 at 8:44 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > On Mon, Jul 10, 2017 at 8:56 AM, Joseph Myers <joseph@codesourcery.com> wrote: >> 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. > > > Revised patch attached. Thanks. I forgot to test the case where fn_final doesn't fire at all. Revised (only last line of do_test changed). 2017-07-24 Paul Pluzhnikov <ppluzhnikov@google.com> * stdlib/Makefile (tst-atexit, tst-at_quick_exit): New tests (tst-cxa_atexit, tst-on_exit): Likewise * stdlib/tst-atexit-common.c: New. * stdlib/tst-atexit.c: New. * stdlib/tst-at_quick_exit.c: New. * stdlib/tst-cxa_atexit.c: New. -- Paul Pluzhnikov [-- Attachment #2: glibc-atexit-20170724a.txt --] [-- Type: text/plain, Size: 8028 bytes --] diff --git a/stdlib/Makefile b/stdlib/Makefile index 0314d5926b..94cbd0e479 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -80,7 +80,9 @@ 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-atexit tst-at_quick_exit \ + tst-cxa_atexit 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-at_quick_exit.c b/stdlib/tst-at_quick_exit.c new file mode 100644 index 0000000000..c86bbe3455 --- /dev/null +++ b/stdlib/tst-at_quick_exit.c @@ -0,0 +1,25 @@ +/* Test that functions registered via at_auick_exit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define ATEXIT(fn) at_quick_exit (fn) +#define EXIT(x) quick_exit (x) + +#include <stdlib/tst-atexit-common.c> diff --git a/stdlib/tst-atexit-common.c b/stdlib/tst-atexit-common.c new file mode 100644 index 0000000000..428dba1742 --- /dev/null +++ b/stdlib/tst-atexit-common.c @@ -0,0 +1,86 @@ +/* Helper file for tst-{atexit,at_quick_exit,cxa_atexit,on_exit}. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <assert.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +#define MAX_ATEXIT 20 +static char crumbs[MAX_ATEXIT]; +static int next_slot = 0; + +static void +fn0 (void) +{ + crumbs[next_slot++] = '0'; +} + +static void +fn1 (void) +{ + crumbs[next_slot++] = '1'; +} + +static void +fn2 (void) +{ + crumbs[next_slot++] = '2'; + ATEXIT (fn1); +} + +static void +fn3 (void) +{ + crumbs[next_slot++] = '3'; + ATEXIT (fn2); + ATEXIT (fn0); +} + +static void +fn_final (void) +{ + const char expected[] = "3021121130211"; + if (strcmp (crumbs, expected) == 0) + _exit (0); + + printf ("crumbs: %s\n", crumbs); + printf ("expected: %s\n", expected); + _exit (1); +} + +static int +do_test (void) +{ + /* Register this first so it can verify expected order of the rest. */ + ATEXIT (fn_final); + + ATEXIT (fn1); + ATEXIT (fn3); + ATEXIT (fn1); + ATEXIT (fn2); + ATEXIT (fn1); + ATEXIT (fn3); + + EXIT (2); /* If we see this exit code, fn_final must have not worked. */ +} + +#define TEST_FUNCTION do_test +#include <support/test-driver.c> diff --git a/stdlib/tst-atexit.c b/stdlib/tst-atexit.c new file mode 100644 index 0000000000..8ca401bcbf --- /dev/null +++ b/stdlib/tst-atexit.c @@ -0,0 +1,25 @@ +/* Test that functions registered via atexit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define ATEXIT(fn) atexit (fn) +#define EXIT(x) exit (x) + +#include <stdlib/tst-atexit-common.c> diff --git a/stdlib/tst-cxa_atexit.c b/stdlib/tst-cxa_atexit.c new file mode 100644 index 0000000000..46fa04a1f7 --- /dev/null +++ b/stdlib/tst-cxa_atexit.c @@ -0,0 +1,27 @@ +/* Test that functions registered via __cxa_atexit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +extern int __cxa_atexit (void (*func) (void *), void *arg, void *d); + +#define ATEXIT(fn) __cxa_atexit ((void (*) (void *)) fn, (void *) 0, (void *) 0) +#define EXIT(x) exit (x) + +#include <stdlib/tst-atexit-common.c> diff --git a/stdlib/tst-on_exit.c b/stdlib/tst-on_exit.c new file mode 100644 index 0000000000..8dda872154 --- /dev/null +++ b/stdlib/tst-on_exit.c @@ -0,0 +1,25 @@ +/* Test that functions registered via on_exit are called in correct + (LIFO) order. In particular, exit handlers can themselves register + additional handlers, so we test that. + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#define ATEXIT(fn) on_exit ((void (*) (int, void *)) fn, (void *) 0) +#define EXIT(x) exit (x) + +#include <stdlib/tst-atexit-common.c> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-24 17:05 ` Paul Pluzhnikov @ 2017-07-31 18:05 ` Paul Pluzhnikov 2017-08-07 14:54 ` Paul Pluzhnikov 2017-08-16 16:36 ` Carlos O'Donell 1 sibling, 1 reply; 13+ messages in thread From: Paul Pluzhnikov @ 2017-07-31 18:05 UTC (permalink / raw) To: Joseph Myers; +Cc: Szabolcs Nagy, nd, GLIBC Devel On Mon, Jul 24, 2017 at 8:50 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > 2017-07-24 Paul Pluzhnikov <ppluzhnikov@google.com> > > * stdlib/Makefile (tst-atexit, tst-at_quick_exit): New tests > (tst-cxa_atexit, tst-on_exit): Likewise > * stdlib/tst-atexit-common.c: New. > * stdlib/tst-atexit.c: New. > * stdlib/tst-at_quick_exit.c: New. > * stdlib/tst-cxa_atexit.c: New. Ping? -- Paul Pluzhnikov ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-31 18:05 ` Paul Pluzhnikov @ 2017-08-07 14:54 ` Paul Pluzhnikov 0 siblings, 0 replies; 13+ messages in thread From: Paul Pluzhnikov @ 2017-08-07 14:54 UTC (permalink / raw) To: Joseph Myers; +Cc: Szabolcs Nagy, nd, GLIBC Devel On Mon, Jul 31, 2017 at 10:00 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > On Mon, Jul 24, 2017 at 8:50 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: > >> 2017-07-24 Paul Pluzhnikov <ppluzhnikov@google.com> >> >> * stdlib/Makefile (tst-atexit, tst-at_quick_exit): New tests >> (tst-cxa_atexit, tst-on_exit): Likewise >> * stdlib/tst-atexit-common.c: New. >> * stdlib/tst-atexit.c: New. >> * stdlib/tst-at_quick_exit.c: New. >> * stdlib/tst-cxa_atexit.c: New. > > Ping? Ping x2? -- Paul Pluzhnikov ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-24 17:05 ` Paul Pluzhnikov 2017-07-31 18:05 ` Paul Pluzhnikov @ 2017-08-16 16:36 ` Carlos O'Donell 2017-08-28 2:16 ` Paul Pluzhnikov 1 sibling, 1 reply; 13+ messages in thread From: Carlos O'Donell @ 2017-08-16 16:36 UTC (permalink / raw) To: Paul Pluzhnikov, Joseph Myers; +Cc: Szabolcs Nagy, nd, GLIBC Devel On 07/24/2017 11:50 AM, Paul Pluzhnikov wrote: > On Mon, Jul 24, 2017 at 8:44 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote: >> On Mon, Jul 10, 2017 at 8:56 AM, Joseph Myers <joseph@codesourcery.com> wrote: >>> 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. >> >> Revised patch attached. Thanks. > I forgot to test the case where fn_final doesn't fire at all. > Revised (only last line of do_test changed). Thank you very much for adding new test. They are an important part of accelerating glibc development. Overall the structure of the tests looks good, but you need a few more comments. OK with the changes outlined below. I make some additional testing suggestions but these tests are better than nothing, so I don't hold you to anything but documenting the possible options. > 2017-07-24 Paul Pluzhnikov <ppluzhnikov@google.com> > > * stdlib/Makefile (tst-atexit, tst-at_quick_exit): New tests > (tst-cxa_atexit, tst-on_exit): Likewise * stdlib/Makefile (tests): Add tst-atexit, tst-at_quick_exit, tst-cxa_atexit, and tst-on_exit. > * stdlib/tst-atexit-common.c: New. > * stdlib/tst-atexit.c: New. > * stdlib/tst-at_quick_exit.c: New. > * stdlib/tst-cxa_atexit.c: New. Say "New file." > > -- Paul Pluzhnikov > > > glibc-atexit-20170724a.txt > > > diff --git a/stdlib/Makefile b/stdlib/Makefile > index 0314d5926b..94cbd0e479 100644 > --- a/stdlib/Makefile > +++ b/stdlib/Makefile > @@ -80,7 +80,9 @@ 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-atexit tst-at_quick_exit \ > + tst-cxa_atexit tst-on_exit OK. Add 4 new tests. > + > 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-at_quick_exit.c b/stdlib/tst-at_quick_exit.c > new file mode 100644 > index 0000000000..c86bbe3455 > --- /dev/null > +++ b/stdlib/tst-at_quick_exit.c > @@ -0,0 +1,25 @@ > +/* Test that functions registered via at_auick_exit are called in correct > + (LIFO) order. In particular, exit handlers can themselves register > + additional handlers, so we test that. > + This needs to be a one line summary, the rest can move after the top-level copyright block. No empty line after the first. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#define ATEXIT(fn) at_quick_exit (fn) > +#define EXIT(x) quick_exit (x) > + > +#include <stdlib/tst-atexit-common.c> > diff --git a/stdlib/tst-atexit-common.c b/stdlib/tst-atexit-common.c > new file mode 100644 > index 0000000000..428dba1742 > --- /dev/null > +++ b/stdlib/tst-atexit-common.c > @@ -0,0 +1,86 @@ > +/* Helper file for tst-{atexit,at_quick_exit,cxa_atexit,on_exit}. > + Remove empty line. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <assert.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > + > +#define MAX_ATEXIT 20 Why 20? Provide a comment stating this is arbitrarily chosen or chosen for a specific purpose. POSIX says we need to support at least ATEXIT_MAX? Should this be dynamically computed via sysconf()? It's OK to keep your code as it is, but you should list the various considerations for POSIX and sysconf in the comment. Likewise we don't test that a fork'd child inherits the registrations of the parent, and we could. So that should also be mentioned briefly. > +static char crumbs[MAX_ATEXIT]; > +static int next_slot = 0; > + > +static void > +fn0 (void) > +{ > + crumbs[next_slot++] = '0'; > +} > + > +static void > +fn1 (void) > +{ > + crumbs[next_slot++] = '1'; > +} > + > +static void > +fn2 (void) > +{ > + crumbs[next_slot++] = '2'; > + ATEXIT (fn1); > +} > + > +static void > +fn3 (void) > +{ > + crumbs[next_slot++] = '3'; > + ATEXIT (fn2); > + ATEXIT (fn0); > +} > + > +static void > +fn_final (void) > +{ > + const char expected[] = "3021121130211"; Needs a comment explaining if this order is arbitrarily chosen or specifically chosen to trigger the bug. For example, can this be changed in the future? > + if (strcmp (crumbs, expected) == 0) > + _exit (0); > + > + printf ("crumbs: %s\n", crumbs); > + printf ("expected: %s\n", expected); > + _exit (1); > +} > + Add a comment describing the test in more detail please. > +static int > +do_test (void) > +{ > + /* Register this first so it can verify expected order of the rest. */ > + ATEXIT (fn_final); > + > + ATEXIT (fn1); > + ATEXIT (fn3); > + ATEXIT (fn1); > + ATEXIT (fn2); > + ATEXIT (fn1); > + ATEXIT (fn3); > + > + EXIT (2); /* If we see this exit code, fn_final must have not worked. */ > +} > + > +#define TEST_FUNCTION do_test > +#include <support/test-driver.c> OK. > diff --git a/stdlib/tst-atexit.c b/stdlib/tst-atexit.c > new file mode 100644 > index 0000000000..8ca401bcbf > --- /dev/null > +++ b/stdlib/tst-atexit.c > @@ -0,0 +1,25 @@ > +/* Test that functions registered via atexit are called in correct > + (LIFO) order. In particular, exit handlers can themselves register > + additional handlers, so we test that. > + Same comments as above. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#define ATEXIT(fn) atexit (fn) > +#define EXIT(x) exit (x) > + > +#include <stdlib/tst-atexit-common.c> OK. > diff --git a/stdlib/tst-cxa_atexit.c b/stdlib/tst-cxa_atexit.c > new file mode 100644 > index 0000000000..46fa04a1f7 > --- /dev/null > +++ b/stdlib/tst-cxa_atexit.c > @@ -0,0 +1,27 @@ > +/* Test that functions registered via __cxa_atexit are called in correct > + (LIFO) order. In particular, exit handlers can themselves register > + additional handlers, so we test that. > + Same comments as above. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +extern int __cxa_atexit (void (*func) (void *), void *arg, void *d); > + > +#define ATEXIT(fn) __cxa_atexit ((void (*) (void *)) fn, (void *) 0, (void *) 0) > +#define EXIT(x) exit (x) > + > +#include <stdlib/tst-atexit-common.c> OK. > diff --git a/stdlib/tst-on_exit.c b/stdlib/tst-on_exit.c > new file mode 100644 > index 0000000000..8dda872154 > --- /dev/null > +++ b/stdlib/tst-on_exit.c > @@ -0,0 +1,25 @@ > +/* Test that functions registered via on_exit are called in correct > + (LIFO) order. In particular, exit handlers can themselves register > + additional handlers, so we test that. > + Same comments as above. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#define ATEXIT(fn) on_exit ((void (*) (int, void *)) fn, (void *) 0) > +#define EXIT(x) exit (x) > + > +#include <stdlib/tst-atexit-common.c> OK. -- Cheers, Carlos. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-08-16 16:36 ` Carlos O'Donell @ 2017-08-28 2:16 ` Paul Pluzhnikov 0 siblings, 0 replies; 13+ messages in thread From: Paul Pluzhnikov @ 2017-08-28 2:16 UTC (permalink / raw) To: Carlos O'Donell; +Cc: Joseph Myers, Szabolcs Nagy, nd, GLIBC Devel On Wed, Aug 16, 2017 at 9:36 AM, Carlos O'Donell <carlos@redhat.com> wrote: > OK with the changes outlined below. Thanks. Committed with these changes as https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=5f3b183d198b39ca993a41aadb02bddd9fde078d > I make some additional testing suggestions > but these tests are better than nothing, so I don't hold you to anything but > documenting the possible options. I left this for a follow-up commit. -- Paul Pluzhnikov ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Add tests for atexit/on_exit firing order 2017-07-10 15:39 ` Paul Pluzhnikov 2017-07-10 15:45 ` Szabolcs Nagy @ 2017-07-10 15:55 ` Joseph Myers 1 sibling, 0 replies; 13+ messages in thread From: Joseph Myers @ 2017-07-10 15:55 UTC (permalink / raw) To: Paul Pluzhnikov; +Cc: GLIBC Devel 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. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-08-28 2:16 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-10 15:01 [patch] Add tests for atexit/on_exit firing order Paul Pluzhnikov 2017-07-10 15:09 ` Carlos O'Donell 2017-07-10 15:18 ` Joseph Myers 2017-07-10 15:39 ` Paul Pluzhnikov 2017-07-10 15:45 ` Szabolcs Nagy 2017-07-10 15:56 ` Joseph Myers 2017-07-24 15:50 ` Paul Pluzhnikov 2017-07-24 17:05 ` Paul Pluzhnikov 2017-07-31 18:05 ` Paul Pluzhnikov 2017-08-07 14:54 ` Paul Pluzhnikov 2017-08-16 16:36 ` Carlos O'Donell 2017-08-28 2:16 ` Paul Pluzhnikov 2017-07-10 15:55 ` Joseph Myers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).