[13/13] linux: Return EINVAL for invalid clock for pthread_clockjoin_np

Message ID 20201123195256.3336217-13-adhemerval.zanella@linaro.org
State Committed
Headers
Series [01/13] linux: Remove unused internal futex functions |

Commit Message

Adhemerval Zanella Nov. 23, 2020, 7:52 p.m. UTC
  The align the GNU extension with the others one that accept specify
which clock to wait for (such as pthread_mutex_clocklock).

Check on x86_64-linux-gnu.
---
 nptl/pthread_clockjoin.c     |  4 ++
 sysdeps/pthread/Makefile     |  2 +-
 sysdeps/pthread/tst-join15.c | 80 ++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/pthread/tst-join15.c
  

Comments

Lukasz Majewski Nov. 24, 2020, 9:48 p.m. UTC | #1
Hi Adhemerval,

> The align the GNU extension with the others one that accept specify
> which clock to wait for (such as pthread_mutex_clocklock).
> 
> Check on x86_64-linux-gnu.
> ---
>  nptl/pthread_clockjoin.c     |  4 ++
>  sysdeps/pthread/Makefile     |  2 +-
>  sysdeps/pthread/tst-join15.c | 80
> ++++++++++++++++++++++++++++++++++++ 3 files changed, 85
> insertions(+), 1 deletion(-) create mode 100644
> sysdeps/pthread/tst-join15.c
> 
> diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
> index 0baba1e83d..3d54fe588f 100644
> --- a/nptl/pthread_clockjoin.c
> +++ b/nptl/pthread_clockjoin.c
> @@ -17,12 +17,16 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <time.h>
> +#include <futex-internal.h>
>  #include "pthreadP.h"
>  
>  int
>  __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
>                            clockid_t clockid, const struct
> __timespec64 *abstime) {
> +  if (!futex_abstimed_supported_clockid (clockid))
> +    return EINVAL;
> +
>    return __pthread_clockjoin_ex (threadid, thread_return,
>                                   clockid, abstime, true);
>  }
> diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
> index 45a15b0b1a..8f335c13b5 100644
> --- a/sysdeps/pthread/Makefile
> +++ b/sysdeps/pthread/Makefile
> @@ -77,7 +77,7 @@ tests += tst-cnd-basic tst-mtx-trylock
> tst-cnd-broadcast \ tst-getpid3 \
>  	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6
> tst-join7 \ tst-join8 tst-join9 tst-join10 tst-join11 tst-join12
> tst-join13 \
> -	 tst-join14 \
> +	 tst-join14 tst-join15 \
>  	 tst-key1 tst-key2 tst-key3 tst-key4 \
>  	 tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6
> \ tst-locale1 tst-locale2 \
> diff --git a/sysdeps/pthread/tst-join15.c
> b/sysdeps/pthread/tst-join15.c new file mode 100644
> index 0000000000..4ed767e733
> --- /dev/null
> +++ b/sysdeps/pthread/tst-join15.c
> @@ -0,0 +1,80 @@
> +/* Check pthread_clockjoin_np clock support.
> +   Copyright (C) 2020 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <pthread.h>
> +#include <sys/time.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +#include <array_length.h>
> +#include <support/check.h>
> +#include <support/timespec.h>
> +#include <support/xthread.h>
> +
> +static void *
> +tf (void *arg)
> +{
> +  pause ();
> +  return NULL;
> +}
> +
> +
> +static int
> +do_test (void)
> +{
> +  const clockid_t clocks[] = {
> +    CLOCK_REALTIME,
> +    CLOCK_MONOTONIC,
> +    CLOCK_PROCESS_CPUTIME_ID,
> +    CLOCK_THREAD_CPUTIME_ID,
> +    CLOCK_THREAD_CPUTIME_ID,
> +    CLOCK_MONOTONIC_RAW,
> +    CLOCK_REALTIME_COARSE,
> +    CLOCK_MONOTONIC_COARSE,
> +#ifdef CLOCK_BOOTTIME
> +    CLOCK_BOOTTIME,
> +#endif
> +#ifdef CLOCK_REALTIME_ALARM
> +    CLOCK_REALTIME_ALARM,
> +#endif
> +#ifdef CLOCK_BOOTTIME_ALARM
> +    CLOCK_BOOTTIME_ALARM,
> +#endif
> +#ifdef CLOCK_TAI
> +    CLOCK_TAI
> +#endif
> +  };
> +
> +  pthread_t thr = xpthread_create (NULL, tf, NULL);
> +
> +  for (int t = 0; t < array_length (clocks); t++)
> +    {
> +      /* A valid timeout so valid clock timeout.  */
> +      struct timespec tmo = timespec_add (xclock_now (clocks[t]),
> +					  make_timespec (0,
> 100000000)); +
> +      int ret = clocks[t] == CLOCK_REALTIME || clocks[t] ==
> CLOCK_MONOTONIC
> +		? ETIMEDOUT : EINVAL;
> +
> +      TEST_COMPARE (pthread_clockjoin_np (thr, NULL, clocks[t],
> &tmo), ret);
> +    }
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Reviewed-by: Lukasz Majewski <lukma@denx.de>


When can we expect that those patches will be pulled? It seems like we
mostly remove dead (i.e. non Y2038 supporting) code in this series.

As aio_suspend depends on this patch series I would be great if we
would pull those sooner than latter :-).

Adhemerval, thanks again for refactoring the futex code.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Lukasz Majewski Nov. 24, 2020, 10:58 p.m. UTC | #2
On Tue, 24 Nov 2020 22:48:59 +0100
Lukasz Majewski <lukma@denx.de> wrote:

> Hi Adhemerval,
> 
> > The align the GNU extension with the others one that accept specify
> > which clock to wait for (such as pthread_mutex_clocklock).
> > 
> > Check on x86_64-linux-gnu.
> > ---
> >  nptl/pthread_clockjoin.c     |  4 ++
> >  sysdeps/pthread/Makefile     |  2 +-
> >  sysdeps/pthread/tst-join15.c | 80
> > ++++++++++++++++++++++++++++++++++++ 3 files changed, 85
> > insertions(+), 1 deletion(-) create mode 100644
> > sysdeps/pthread/tst-join15.c
> > 
> > diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
> > index 0baba1e83d..3d54fe588f 100644
> > --- a/nptl/pthread_clockjoin.c
> > +++ b/nptl/pthread_clockjoin.c
> > @@ -17,12 +17,16 @@
> >     <http://www.gnu.org/licenses/>.  */
> >  
> >  #include <time.h>
> > +#include <futex-internal.h>
> >  #include "pthreadP.h"
> >  
> >  int
> >  __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
> >                            clockid_t clockid, const struct
> > __timespec64 *abstime) {
> > +  if (!futex_abstimed_supported_clockid (clockid))
> > +    return EINVAL;
> > +
> >    return __pthread_clockjoin_ex (threadid, thread_return,
> >                                   clockid, abstime, true);
> >  }
> > diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
> > index 45a15b0b1a..8f335c13b5 100644
> > --- a/sysdeps/pthread/Makefile
> > +++ b/sysdeps/pthread/Makefile
> > @@ -77,7 +77,7 @@ tests += tst-cnd-basic tst-mtx-trylock
> > tst-cnd-broadcast \ tst-getpid3 \
> >  	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5
> > tst-join6 tst-join7 \ tst-join8 tst-join9 tst-join10 tst-join11
> > tst-join12 tst-join13 \
> > -	 tst-join14 \
> > +	 tst-join14 tst-join15 \
> >  	 tst-key1 tst-key2 tst-key3 tst-key4 \
> >  	 tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5
> > tst-kill6 \ tst-locale1 tst-locale2 \
> > diff --git a/sysdeps/pthread/tst-join15.c
> > b/sysdeps/pthread/tst-join15.c new file mode 100644
> > index 0000000000..4ed767e733
> > --- /dev/null
> > +++ b/sysdeps/pthread/tst-join15.c
> > @@ -0,0 +1,80 @@
> > +/* Check pthread_clockjoin_np clock support.
> > +   Copyright (C) 2020 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
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#include <pthread.h>
> > +#include <sys/time.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> > +
> > +#include <array_length.h>
> > +#include <support/check.h>
> > +#include <support/timespec.h>
> > +#include <support/xthread.h>
> > +
> > +static void *
> > +tf (void *arg)
> > +{
> > +  pause ();
> > +  return NULL;
> > +}
> > +
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  const clockid_t clocks[] = {
> > +    CLOCK_REALTIME,
> > +    CLOCK_MONOTONIC,
> > +    CLOCK_PROCESS_CPUTIME_ID,
> > +    CLOCK_THREAD_CPUTIME_ID,
> > +    CLOCK_THREAD_CPUTIME_ID,
> > +    CLOCK_MONOTONIC_RAW,
> > +    CLOCK_REALTIME_COARSE,
> > +    CLOCK_MONOTONIC_COARSE,
> > +#ifdef CLOCK_BOOTTIME
> > +    CLOCK_BOOTTIME,
> > +#endif
> > +#ifdef CLOCK_REALTIME_ALARM
> > +    CLOCK_REALTIME_ALARM,
> > +#endif
> > +#ifdef CLOCK_BOOTTIME_ALARM
> > +    CLOCK_BOOTTIME_ALARM,
> > +#endif
> > +#ifdef CLOCK_TAI
> > +    CLOCK_TAI
> > +#endif
> > +  };
> > +
> > +  pthread_t thr = xpthread_create (NULL, tf, NULL);
> > +
> > +  for (int t = 0; t < array_length (clocks); t++)
> > +    {
> > +      /* A valid timeout so valid clock timeout.  */
> > +      struct timespec tmo = timespec_add (xclock_now (clocks[t]),
> > +					  make_timespec (0,
> > 100000000)); +
> > +      int ret = clocks[t] == CLOCK_REALTIME || clocks[t] ==
> > CLOCK_MONOTONIC
> > +		? ETIMEDOUT : EINVAL;
> > +
> > +      TEST_COMPARE (pthread_clockjoin_np (thr, NULL, clocks[t],
> > &tmo), ret);
> > +    }
> > +
> > +  return 0;
> > +}
> > +
> > +#include <support/test-driver.c>  
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>
> 
> 
> When can we expect that those patches will be pulled? It seems like we
> mostly remove dead (i.e. non Y2038 supporting) code in this series.
> 
> As aio_suspend depends on this patch series I would be great if we
> would pull those sooner than latter :-).

I've built tested following branch:
https://github.com/lmajewski/y2038_glibc/commits/y2038_edge-futex-rework

on x86-64-x32, x86, x86-64, arm 32 bits and pthread_* related tests
were OK, so

Tested-by: Lukasz Majewski <lukma@denx.de>


> 
> Adhemerval, thanks again for refactoring the futex code.
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  

Patch

diff --git a/nptl/pthread_clockjoin.c b/nptl/pthread_clockjoin.c
index 0baba1e83d..3d54fe588f 100644
--- a/nptl/pthread_clockjoin.c
+++ b/nptl/pthread_clockjoin.c
@@ -17,12 +17,16 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <futex-internal.h>
 #include "pthreadP.h"
 
 int
 __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
                           clockid_t clockid, const struct __timespec64 *abstime)
 {
+  if (!futex_abstimed_supported_clockid (clockid))
+    return EINVAL;
+
   return __pthread_clockjoin_ex (threadid, thread_return,
                                  clockid, abstime, true);
 }
diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
index 45a15b0b1a..8f335c13b5 100644
--- a/sysdeps/pthread/Makefile
+++ b/sysdeps/pthread/Makefile
@@ -77,7 +77,7 @@  tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \
 	 tst-getpid3 \
 	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 tst-join7 \
 	 tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \
-	 tst-join14 \
+	 tst-join14 tst-join15 \
 	 tst-key1 tst-key2 tst-key3 tst-key4 \
 	 tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \
 	 tst-locale1 tst-locale2 \
diff --git a/sysdeps/pthread/tst-join15.c b/sysdeps/pthread/tst-join15.c
new file mode 100644
index 0000000000..4ed767e733
--- /dev/null
+++ b/sysdeps/pthread/tst-join15.c
@@ -0,0 +1,80 @@ 
+/* Check pthread_clockjoin_np clock support.
+   Copyright (C) 2020 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <array_length.h>
+#include <support/check.h>
+#include <support/timespec.h>
+#include <support/xthread.h>
+
+static void *
+tf (void *arg)
+{
+  pause ();
+  return NULL;
+}
+
+
+static int
+do_test (void)
+{
+  const clockid_t clocks[] = {
+    CLOCK_REALTIME,
+    CLOCK_MONOTONIC,
+    CLOCK_PROCESS_CPUTIME_ID,
+    CLOCK_THREAD_CPUTIME_ID,
+    CLOCK_THREAD_CPUTIME_ID,
+    CLOCK_MONOTONIC_RAW,
+    CLOCK_REALTIME_COARSE,
+    CLOCK_MONOTONIC_COARSE,
+#ifdef CLOCK_BOOTTIME
+    CLOCK_BOOTTIME,
+#endif
+#ifdef CLOCK_REALTIME_ALARM
+    CLOCK_REALTIME_ALARM,
+#endif
+#ifdef CLOCK_BOOTTIME_ALARM
+    CLOCK_BOOTTIME_ALARM,
+#endif
+#ifdef CLOCK_TAI
+    CLOCK_TAI
+#endif
+  };
+
+  pthread_t thr = xpthread_create (NULL, tf, NULL);
+
+  for (int t = 0; t < array_length (clocks); t++)
+    {
+      /* A valid timeout so valid clock timeout.  */
+      struct timespec tmo = timespec_add (xclock_now (clocks[t]),
+					  make_timespec (0, 100000000));
+
+      int ret = clocks[t] == CLOCK_REALTIME || clocks[t] == CLOCK_MONOTONIC
+		? ETIMEDOUT : EINVAL;
+
+      TEST_COMPARE (pthread_clockjoin_np (thr, NULL, clocks[t], &tmo), ret);
+    }
+
+  return 0;
+}
+
+#include <support/test-driver.c>