Re: [PATCH] nptl: Add tst-minstack-cancel, tst-minstack-exit [BZ #22636]

[Carlos O'Donell <carlos at redhat dot com>]

On 01/10/2018 02:21 AM, Florian Weimer wrote:
> I verified that without the guard accounting change in commit
> 630f4cc3aa019ede55976ea561f1a7af2f068639 (Fix stack guard size
> accounting) the tst-minstack-cancel test fails on an AVX-512F machine.
> (tst-minstack-exit still passes.)
> 
> The two tests cannot be merged because lazy binding in libgcc occurs
> only once and contributes significantly to stack usage, so the first
> test would alter stack usage in the second test.
> 

* BIND_NOW vs lazy binding?

- Do the two tests need to be built explicitly with BIND_NOW to avoid
  lazy binding stack usage on systems that do not have compilers that
  default to building binaries with BIND_NOW?

- Should both tests be built and run with and without lazy binding?

* Comment about merging the tests.

- Please add a comment to both tests that explains that the tests
  should not be merged together with any other test, and that this is
  done to avoid impacting the stack of whichever test is run second.

OK if you take both issues into consideration.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 2018-01-10  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #22636]
> 	* nptl/Makefile (tests): Add tst-minstack-cancel, tst-minstack-exit.
> 	* nptl/tst-minstack-cancel.c, nptl/tst-minstack-exit.c: New files.
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 7265c0a53c..12c69f99d8 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -307,7 +307,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
>  	tst-bad-schedattr \
>  	tst-thread_local1 tst-mutex-errorcheck tst-robust10 \
>  	tst-robust-fork tst-create-detached tst-memstream \
> -	tst-thread-exit-clobber
> +	tst-thread-exit-clobber tst-minstack-cancel tst-minstack-exit

OK.

>  
>  tests-internal := tst-rwlock19 tst-rwlock20 \
>  		  tst-sem11 tst-sem12 tst-sem13 \
> diff --git a/nptl/tst-minstack-cancel.c b/nptl/tst-minstack-cancel.c
> new file mode 100644
> index 0000000000..3d8d0dba70
> --- /dev/null
> +++ b/nptl/tst-minstack-cancel.c
> @@ -0,0 +1,45 @@
> +/* Test cancellation with a minimal stack size.
> +   Copyright (C) 2018 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 <limits.h>
> +#include <unistd.h>
> +#include <support/check.h>
> +#include <support/xthread.h>
> +
> +static void *
> +threadfunc (void *closure)
> +{
> +  while (1)
> +    pause ();

OK.

> +  return NULL;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  pthread_attr_t attr;
> +  xpthread_attr_init (&attr);
> +  xpthread_attr_setstacksize (&attr, PTHREAD_STACK_MIN);

OK.

> +  pthread_t thr = xpthread_create (&attr, threadfunc, NULL);
> +  xpthread_cancel (thr);
> +  TEST_VERIFY (xpthread_join (thr) == PTHREAD_CANCELED);

OK.

> +  xpthread_attr_destroy (&attr);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/nptl/tst-minstack-exit.c b/nptl/tst-minstack-exit.c
> new file mode 100644
> index 0000000000..eb6808e889
> --- /dev/null
> +++ b/nptl/tst-minstack-exit.c
> @@ -0,0 +1,43 @@
> +/* Test that pthread_exit works with the minimum stack size.
> +   Copyright (C) 2018 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 <limits.h>
> +#include <unistd.h>
> +#include <support/check.h>
> +#include <support/xthread.h>
> +
> +static void *
> +threadfunc (void *closure)
> +{
> +  pthread_exit (threadfunc);

OK, run through thread exit with minimal stack.

> +  return NULL;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  pthread_attr_t attr;
> +  xpthread_attr_init (&attr);
> +  xpthread_attr_setstacksize (&attr, PTHREAD_STACK_MIN);
> +  pthread_t thr = xpthread_create (&attr, threadfunc, NULL);
> +  TEST_VERIFY (xpthread_join (thr) == threadfunc);

OK.

> +  xpthread_attr_destroy (&attr);
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> 


-- 
Cheers,
Carlos.