[COMMITTED PATCH] Test that pthread_create diagnoses invalid scheduling parameters.

[COMMITTED PATCH] Test that pthread_create diagnoses invalid scheduling parameters.

[Roland McGrath]

Tested x86_64-linux-gnu.


Thanks,
Roland


2014-11-19  Roland McGrath  <roland@hack.frob.com>

	* nptl/tst-bad-schedattr.c: New file.
	* nptl/Makefile (tests): Add it.

--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -270,7 +270,8 @@ tests = tst-typesizes \
 	tst-vfork1 tst-vfork2 tst-vfork1x tst-vfork2x \
 	tst-getpid1 tst-getpid2 tst-getpid3 \
 	tst-setuid3 \
-	tst-initializers1 $(patsubst %,tst-initializers1-%,c89 gnu89 c99 gnu99)
+	tst-initializers1 $(addprefix tst-initializers1-,c89 gnu89 c99 gnu99) \
+	tst-bad-schedattr
 xtests = tst-setuid1 tst-setuid1-static tst-setuid2 \
 	tst-mutexpp1 tst-mutexpp6 tst-mutexpp10
 test-srcs = tst-oddstacklimit
--- /dev/null
+++ b/nptl/tst-bad-schedattr.c
@@ -0,0 +1,97 @@
+/* Test that pthread_create diagnoses invalid scheduling parameters.
+   Copyright (C) 2014 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 <errno.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+
+static void *
+thread_function (void *arg)
+{
+  abort ();
+}
+
+
+static int
+do_test (void)
+{
+#if !defined SCHED_FIFO || !defined SCHED_OTHER
+  puts ("SCHED_FIFO or SCHED_OTHER not available at compile time");
+  return 0; /* 77 */
+#else
+
+  int err;
+
+#define TRY(func, arglist)                              \
+  if ((err = func arglist) != 0)                        \
+    {                                                   \
+      printf ("%s: %s\n", #func, strerror (err));       \
+      return 2;                                         \
+    }
+
+  int fifo_max = sched_get_priority_max (SCHED_FIFO);
+  if (fifo_max == -1)
+    {
+      assert (errno == ENOTSUP || errno == ENOSYS);
+      puts ("SCHED_FIFO not supported, cannot test");
+      return 0; /* 77 */
+    }
+
+  int other_max = sched_get_priority_max (SCHED_OTHER);
+  if (other_max == -1)
+    {
+      assert (errno == ENOTSUP || errno == ENOSYS);
+      puts ("SCHED_OTHER not supported, cannot test");
+      return 0; /* 77 */
+    }
+
+  assert (fifo_max > other_max);
+
+  pthread_attr_t attr;
+  TRY (pthread_attr_init, (&attr));
+  TRY (pthread_attr_setinheritsched, (&attr, PTHREAD_EXPLICIT_SCHED));
+  TRY (pthread_attr_setschedpolicy, (&attr, SCHED_FIFO));
+
+  /* This value is chosen so as to be valid for SCHED_FIFO but invalid for
+     SCHED_OTHER.  */
+  struct sched_param param = { .sched_priority = other_max + 1 };
+  TRY (pthread_attr_setschedparam, (&attr, &param));
+
+  TRY (pthread_attr_setschedpolicy, (&attr, SCHED_OTHER));
+
+  /* Now ATTR has a sched_param that is invalid for its policy.  */
+  pthread_t th;
+  err = pthread_create (&th, &attr, &thread_function, NULL);
+  if (err != EINVAL)
+    {
+      printf ("pthread_create returned %d (%s), expected %d (EINVAL: %s)\n",
+              err, strerror (err), EINVAL, strerror (EINVAL));
+      return 1;
+    }
+
+  return 0;
+#endif
+}
+
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"

Re: [COMMITTED PATCH] Test that pthread_create diagnoses invalid scheduling parameters.

[Joseph Myers]

On Wed, 19 Nov 2014, Roland McGrath wrote:

> Tested x86_64-linux-gnu.
> 
> 
> Thanks,
> Roland
> 
> 
> 2014-11-19  Roland McGrath  <roland@hack.frob.com>
> 
> 	* nptl/tst-bad-schedattr.c: New file.
> 	* nptl/Makefile (tests): Add it.

FWIW, I'm seeing this fail on x86_64 (Ubuntu 14.04, GCC 4.9 branch).

pthread_create returned 0 (Success), expected 22 (EINVAL: Invalid argument)
Didn't expect signal from child: got `Aborted'

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [COMMITTED PATCH] Test that pthread_create diagnoses invalid scheduling parameters.

[H.J. Lu]

On Wed, Nov 19, 2014 at 5:33 PM, Roland McGrath <roland@hack.frob.com> wrote:
> Tested x86_64-linux-gnu.
>
>
> Thanks,
> Roland
>
>
> 2014-11-19  Roland McGrath  <roland@hack.frob.com>
>
>         * nptl/tst-bad-schedattr.c: New file.
>         * nptl/Makefile (tests): Add it.
>

It fails on ia32 and x32:

pthread_create returned 0 (Success), expected 22 (EINVAL: Invalid argument)
Didn't expect signal from child: got `Aborted'

Is this expected?


-- 
H.J.

Re: [COMMITTED PATCH] Test that pthread_create diagnoses invalid scheduling parameters.

[Roland McGrath]

In fact the test itself is fine.  It was my later default-sched.h change
that broke it.  I thought I had tested it properly before Carlos's review,
but apparently I confused myself.

Re: [COMMITTED PATCH] Test that pthread_create diagnoses invalid scheduling parameters.

[Roland McGrath]

Fixed thusly.  This does change things slightly from the status quo ante,
but I suspect the old state was not what we really intended to be doing.
Previously, ATTR_FLAG_{POLICY,SCHED}_SET would not be set in PD->flags at
creation time when IATTR->flags had the bit set (and so PD->schedfoo has
value copied from IATTR->schedfoo) but *would* be set when IATTR->flags had
the bit clear (and so PD->schedfoo has a value gleaned via the sched_getfoo
syscall on the pthread_create caller).  Logically that seems backwards at
best.  Practically, I think the only effect is whether
pthread_getschedparam, __pthread_tpp_change_priority, and
__pthread_current_priority yield values already cached in the calling
thread's struct pthread or refresh that cache by making syscalls (so there
is no effect on the second or later repeated call to one of those).  Per
the comment in pthread_getschedparam, no kosher program could notice the
difference.  I think the status quo ante was just an oversight.


Thanks,
Roland


2014-11-21  Roland McGrath  <roland@hack.frob.com>

	* nptl/pthread_create.c (__pthread_create_2_1): Set
	ATTR_FLAG_POLICY_SET and/or ATTR_FLAG_SCHED_SET in PD->flags
	when copying values from IATTR into PD.

--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -596,10 +596,16 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg)
     {
       /* Use the scheduling parameters the user provided.  */
       if (iattr->flags & ATTR_FLAG_POLICY_SET)
-	pd->schedpolicy = iattr->schedpolicy;
+        {
+          pd->schedpolicy = iattr->schedpolicy;
+          pd->flags |= ATTR_FLAG_POLICY_SET;
+        }
       if (iattr->flags & ATTR_FLAG_SCHED_SET)
-        /* The values were validated in pthread_attr_setschedparam.  */
-        pd->schedparam = iattr->schedparam;
+        {
+          /* The values were validated in pthread_attr_setschedparam.  */
+          pd->schedparam = iattr->schedparam;
+          pd->flags |= ATTR_FLAG_SCHED_SET;
+        }
 
       if ((pd->flags & (ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET))
           != (ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET))