[BZ,#19402] Clear list of acquired robust mutexes in the child process after forking.

On 12/23/2016 10:31 PM, Torvald Riegel wrote:
>> Can we add a test case for this?

> What do you have in mind?  Checking that the list is reset to zero after

> fork?  Do we need a test for that if we have documented the need to do

> that in the code?


What about the attached patch?

Thanks,
Florian

On Fri, 2017-01-13 at 14:11 +0100, Florian Weimer wrote:
> On 12/23/2016 10:31 PM, Torvald Riegel wrote:

> >> Can we add a test case for this?

> > What do you have in mind?  Checking that the list is reset to zero after

> > fork?  Do we need a test for that if we have documented the need to do

> > that in the code?

> 

> What about the attached patch?


That looks fine generally, with the following exceptions / comments:

According to the feedback so far we got for
http://austingroupbugs.net/view.php?id=1112 the trylock-based test is
actually UB.  There should be a comment making that clear (ie, that we
rely on something we know about our current implementation to test
this).

If this was a multi-threaded test program, it would be invalid because
non-async-signal-safe functions are executed in the child before exec()
is called.  In a single-threaded program, it's not quite clear to me
what POSIX really wants.  There's indication that only AS-safe functions
should be called, but IIRC that's not explicitly stated.
Florian says we still need to support this as an extension.  If so, this
should be made clear in the test (using a suitable comment).

What about putting the x* wrappers as static inline functions just in
the header?  Then, we wouldn't need to add all those trivial C files.

On 01/13/2017 04:37 PM, Torvald Riegel wrote:
> On Fri, 2017-01-13 at 14:11 +0100, Florian Weimer wrote:

>> On 12/23/2016 10:31 PM, Torvald Riegel wrote:

>>>> Can we add a test case for this?

>>> What do you have in mind?  Checking that the list is reset to zero after

>>> fork?  Do we need a test for that if we have documented the need to do

>>> that in the code?

>>

>> What about the attached patch?

>

> That looks fine generally, with the following exceptions / comments:

>

> According to the feedback so far we got for

> http://austingroupbugs.net/view.php?id=1112 the trylock-based test is

> actually UB.  There should be a comment making that clear (ie, that we

> rely on something we know about our current implementation to test

> this).


Yeah, I'll move the check to the parent process and add another bit for 
error-checking mutexes (and make the test conditional on that).

> If this was a multi-threaded test program, it would be invalid because

> non-async-signal-safe functions are executed in the child before exec()

> is called.  In a single-threaded program, it's not quite clear to me

> what POSIX really wants.  There's indication that only AS-safe functions

> should be called, but IIRC that's not explicitly stated.

> Florian says we still need to support this as an extension.  If so, this

> should be made clear in the test (using a suitable comment).


I'll add a comment, yes.

> What about putting the x* wrappers as static inline functions just in

> the header?  Then, we wouldn't need to add all those trivial C files.


We did that before and it caused problems because it restricted what 
feature macros we could assume in the implementation of those inline 
functions.

I verified your patch with this test, so I think it is okay to check it 
in.  I'll commit my updated patch afterwards.

Thanks,
Florian

On 01/13/2017 04:37 PM, Torvald Riegel wrote:
> On Fri, 2017-01-13 at 14:11 +0100, Florian Weimer wrote:

>> On 12/23/2016 10:31 PM, Torvald Riegel wrote:

>>>> Can we add a test case for this?

>>> What do you have in mind?  Checking that the list is reset to zero after

>>> fork?  Do we need a test for that if we have documented the need to do

>>> that in the code?

>>

>> What about the attached patch?

>

> That looks fine generally, with the following exceptions / comments:

>

> According to the feedback so far we got for

> http://austingroupbugs.net/view.php?id=1112 the trylock-based test is

> actually UB.  There should be a comment making that clear (ie, that we

> rely on something we know about our current implementation to test

> this).

>

> If this was a multi-threaded test program, it would be invalid because

> non-async-signal-safe functions are executed in the child before exec()

> is called.  In a single-threaded program, it's not quite clear to me

> what POSIX really wants.  There's indication that only AS-safe functions

> should be called, but IIRC that's not explicitly stated.

> Florian says we still need to support this as an extension.  If so, this

> should be made clear in the test (using a suitable comment).


I added a comment and an additional test which uses an error-checking mutex.

Test case still passes on current master.  Okay to commit?

Thanks,
Floriannptl: Add tst-robust-fork

2017-01-23  Florian Weimer  <fweimer@redhat.com>

	* nptl/Makefile (tests): Add tst-robust-fork.
	* nptl/tst-robust-fork.c: New file.
	* support/Makefile (libsupport-routines): Add xmmap, xmunmap,
	xpthread_mutex_consistent, xpthread_mutex_destroy,
	xpthread_mutex_init, xpthread_mutexattr_destroy,
	xpthread_mutexattr_init, xpthread_mutexattr_setprotocol,
	xpthread_mutexattr_setpshared, xpthread_mutexattr_setrobust,
	xpthread_mutexattr_settype.
	* support/xmmap.c: New file.
	* support/xmunmap.c: Likewise.
	* support/xpthread_mutex_consistent.c: Likewise.
	* support/xpthread_mutex_destroy.c: Likewise.
	* support/xpthread_mutex_init.c: Likewise.
	* support/xpthread_mutexattr_destroy.c: Likewise.
	* support/xpthread_mutexattr_init.c: Likewise.
	* support/xpthread_mutexattr_setprotocol.c: Likewise.
	* support/xpthread_mutexattr_setpshared.c: Likewise.
	* support/xpthread_mutexattr_setrobust.c: Likewise.
	* support/xpthread_mutexattr_settype.c: Likewise.
	* support/xthread.h (xpthread_mutexattr_destroy)
	(xpthread_mutexattr_init, xpthread_mutexattr_setprotocol)
	(xpthread_mutexattr_setpshared, xpthread_mutexattr_setrobust)
	(xpthread_mutexattr_settype, xpthread_mutex_init)
	(xpthread_mutex_destroy, xpthread_mutex_consistent): Declare.
	* support/xunistd.h (xmmap, xmunmap): Likewise.

diff --git a/nptl/Makefile b/nptl/Makefile
index dcb5953..2d38316 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -295,7 +295,8 @@ tests = tst-typesizes \
 	tst-initializers1 $(addprefix tst-initializers1-,\
 			    c89 gnu89 c99 gnu99 c11 gnu11) \
 	tst-bad-schedattr \
-	tst-thread_local1 tst-mutex-errorcheck tst-robust10
+	tst-thread_local1 tst-mutex-errorcheck tst-robust10 \
+	tst-robust-fork
 xtests = tst-setuid1 tst-setuid1-static tst-setuid2 \
 	tst-mutexpp1 tst-mutexpp6 tst-mutexpp10
 test-srcs = tst-oddstacklimit
diff --git a/nptl/tst-robust-fork.c b/nptl/tst-robust-fork.c
new file mode 100644
index 0000000..4a12ff0
--- /dev/null
+++ b/nptl/tst-robust-fork.c
@@ -0,0 +1,184 @@
+/* Test the interaction of fork and robust mutexes.
+   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 <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/test-driver.h>
+#include <support/xthread.h>
+#include <support/xunistd.h>
+#include <sys/mman.h>
+
+/* Data shared between processes. */
+struct shared
+{
+  pthread_mutex_t parent_mutex;
+  pthread_mutex_t child_mutex;
+};
+
+/* These flags control which mutex settings are enabled in the parent
+   and child (separately).  */
+enum mutex_bits
+  {
+    mutex_pshared = 1,
+    mutex_robust = 2,
+    mutex_pi = 4,
+    mutex_check = 8,
+
+    /* All bits combined.  */
+    mutex_all_bits = 15,
+  };
+
+static void
+mutex_init (pthread_mutex_t *mutex, int bits)
+{
+  pthread_mutexattr_t attr;
+  xpthread_mutexattr_init (&attr);
+  if (bits & mutex_pshared)
+    xpthread_mutexattr_setpshared (&attr, PTHREAD_PROCESS_SHARED);
+  if (bits & mutex_robust)
+    xpthread_mutexattr_setrobust (&attr, PTHREAD_MUTEX_ROBUST);
+  if (bits & mutex_pi)
+    xpthread_mutexattr_setprotocol (&attr, PTHREAD_PRIO_INHERIT);
+  if (bits & mutex_check)
+    xpthread_mutexattr_settype (&attr, PTHREAD_MUTEX_ERRORCHECK);
+  xpthread_mutex_init (mutex, &attr);
+  xpthread_mutexattr_destroy (&attr);
+}
+
+static void
+one_test (int parent_bits, int child_bits, int nonshared_bits,
+          bool lock_nonshared, bool lock_child)
+{
+
+  struct shared *shared = xmmap (NULL, sizeof (*shared),
+                                 PROT_READ | PROT_WRITE,
+                                 MAP_ANONYMOUS | MAP_SHARED, -1);
+  mutex_init (&shared->parent_mutex, parent_bits);
+  mutex_init (&shared->child_mutex, child_bits);
+
+  /* Acquire the parent mutex in the parent.  */
+  xpthread_mutex_lock (&shared->parent_mutex);
+
+  pthread_mutex_t nonshared_mutex;
+  mutex_init (&nonshared_mutex, nonshared_bits);
+  if (lock_nonshared)
+    xpthread_mutex_lock (&nonshared_mutex);
+
+  pid_t pid = xfork ();
+  if (pid == 0)
+    {
+      /* Child process.  */
+      if (lock_child)
+        xpthread_mutex_lock (&shared->child_mutex);
+      else
+        xmunmap (shared, sizeof (*shared));
+      if (lock_nonshared)
+        /* Reinitialize the non-shared mutex if it was locked in the
+           parent.  */
+        mutex_init (&nonshared_mutex, nonshared_bits);
+      xpthread_mutex_lock (&nonshared_mutex);
+      /* For robust mutexes, the _exit call will perform the unlock
+         instead.  */
+      if (lock_child && !(child_bits & mutex_robust))
+        xpthread_mutex_unlock (&shared->child_mutex);
+      _exit (0);
+    }
+  /* Parent process. */
+  {
+    int status;
+    xwaitpid (pid, &status, 0);
+    TEST_VERIFY (status == 0);
+  }
+
+  if (parent_bits & mutex_check)
+    /* Test for expected self-deadlock.  This is only possible to
+       detect if the mutex is error-checking.  */
+    TEST_VERIFY_EXIT (pthread_mutex_lock (&shared->parent_mutex) == EDEADLK);
+
+  pid = xfork ();
+  if (pid == 0)
+    {
+      /* Child process.  We can perform some checks only if we are
+         dealing with process-shared mutexes.  */
+      if (parent_bits & mutex_pshared)
+        /* It must not be possible to acquire the parent mutex.
+
+           NB: This check touches a mutex which has been acquired in
+           the parent at fork time, so it might be deemed undefined
+           behavior, pending the resolution of Austin Groups issue
+           1112.  */
+        TEST_VERIFY_EXIT (pthread_mutex_trylock (&shared->parent_mutex)
+                          == EBUSY);
+      if (lock_child && (child_bits & mutex_robust))
+        {
+          if (!(child_bits & mutex_pshared))
+            /* No further tests possible.  */
+            _exit (0);
+          TEST_VERIFY_EXIT (pthread_mutex_lock (&shared->child_mutex)
+                            == EOWNERDEAD);
+          xpthread_mutex_consistent (&shared->child_mutex);
+        }
+      else
+        /* We did not acquire the lock in the first child process, or
+           we unlocked the mutex again because the mutex is not a
+           robust mutex.  */
+        xpthread_mutex_lock (&shared->child_mutex);
+      xpthread_mutex_unlock (&shared->child_mutex);
+      _exit (0);
+    }
+  /* Parent process. */
+  {
+    int status;
+    xwaitpid (pid, &status, 0);
+    TEST_VERIFY (status == 0);
+  }
+
+  if (lock_nonshared)
+    xpthread_mutex_unlock (&nonshared_mutex);
+  xpthread_mutex_unlock (&shared->parent_mutex);
+  xpthread_mutex_destroy (&shared->parent_mutex);
+  xpthread_mutex_destroy (&shared->child_mutex);
+  xpthread_mutex_destroy (&nonshared_mutex);
+  xmunmap (shared, sizeof (*shared));
+}
+
+static int
+do_test (void)
+{
+  for (int parent_bits = 0; parent_bits <= mutex_all_bits; ++parent_bits)
+    for (int child_bits = 0; child_bits <= mutex_all_bits; ++child_bits)
+      for (int nonshared_bits = 0; nonshared_bits <= mutex_all_bits;
+           ++nonshared_bits)
+        for (int lock_nonshared = 0; lock_nonshared < 2; ++lock_nonshared)
+          for (int lock_child = 0; lock_child < 2; ++lock_child)
+            {
+              if (test_verbose)
+                printf ("info: parent_bits=0x%x child_bits=0x%x"
+                        " nonshared_bits=0x%x%s%s\n",
+                        parent_bits, child_bits, nonshared_bits,
+                        lock_nonshared ? " lock_nonshared" : "",
+                        lock_child ? " lock_child" : "");
+              one_test (parent_bits, child_bits, nonshared_bits,
+                        lock_nonshared, lock_child);
+            }
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/support/Makefile b/support/Makefile
index 7114855..45aa7fc 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -61,6 +61,8 @@ libsupport-routines = \
   xlisten \
   xmalloc \
   xmemstream \
+  xmmap \
+  xmunmap \
   xpoll \
   xpthread_barrier_destroy \
   xpthread_barrier_init \
@@ -71,8 +73,17 @@ libsupport-routines = \
   xpthread_create \
   xpthread_detach \
   xpthread_join \
+  xpthread_mutex_consistent \
+  xpthread_mutex_destroy \
+  xpthread_mutex_init \
   xpthread_mutex_lock \
   xpthread_mutex_unlock \
+  xpthread_mutexattr_destroy \
+  xpthread_mutexattr_init \
+  xpthread_mutexattr_setprotocol \
+  xpthread_mutexattr_setpshared \
+  xpthread_mutexattr_setrobust \
+  xpthread_mutexattr_settype \
   xpthread_once \
   xpthread_sigmask \
   xpthread_spin_lock \
diff --git a/support/xmmap.c b/support/xmmap.c
new file mode 100644
index 0000000..435b1eb
--- /dev/null
+++ b/support/xmmap.c
@@ -0,0 +1,31 @@
+/* mmap with error checking.
+   Copyright (C) 2016-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 <support/check.h>
+#include <support/xunistd.h>
+#include <sys/mman.h>
+
+void *
+xmmap (void *addr, size_t length, int prot, int flags, int fd)
+{
+  void *result = mmap (addr, length, prot, flags, fd, 0);
+  if (result == MAP_FAILED)
+    FAIL_EXIT1 ("mmap of %zu bytes, prot=0x%x, flags=0x%x: %m",
+                length, prot, flags);
+  return result;
+}
diff --git a/support/xmunmap.c b/support/xmunmap.c
new file mode 100644
index 0000000..6ef5a4a
--- /dev/null
+++ b/support/xmunmap.c
@@ -0,0 +1,28 @@
+/* munmap with error checking.
+   Copyright (C) 2016-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 <support/check.h>
+#include <support/xunistd.h>
+#include <sys/mman.h>
+
+void
+xmunmap (void *addr, size_t length)
+{
+  if (munmap (addr, length) != 0)
+    FAIL_EXIT1 ("munmap of %zu bytes: %m", length);
+}
diff --git a/support/xpthread_mutex_consistent.c b/support/xpthread_mutex_consistent.c
new file mode 100644
index 0000000..52364be
--- /dev/null
+++ b/support/xpthread_mutex_consistent.c
@@ -0,0 +1,26 @@
+/* pthread_mutex_consistent with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutex_consistent (pthread_mutex_t *mutex)
+{
+  xpthread_check_return ("pthread_mutex_consistent",
+                         pthread_mutex_consistent (mutex));
+}
diff --git a/support/xpthread_mutex_destroy.c b/support/xpthread_mutex_destroy.c
new file mode 100644
index 0000000..f11f8f0
--- /dev/null
+++ b/support/xpthread_mutex_destroy.c
@@ -0,0 +1,26 @@
+/* pthread_mutex_destroy with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutex_destroy (pthread_mutex_t *mutex)
+{
+  xpthread_check_return ("pthread_mutex_destroy",
+                         pthread_mutex_destroy (mutex));
+}
diff --git a/support/xpthread_mutex_init.c b/support/xpthread_mutex_init.c
new file mode 100644
index 0000000..2d16d1b
--- /dev/null
+++ b/support/xpthread_mutex_init.c
@@ -0,0 +1,26 @@
+/* pthread_mutex_init with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutex_init (pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
+{
+  xpthread_check_return ("pthread_mutex_init",
+                         pthread_mutex_init (mutex, attr));
+}
diff --git a/support/xpthread_mutexattr_destroy.c b/support/xpthread_mutexattr_destroy.c
new file mode 100644
index 0000000..c699e32
--- /dev/null
+++ b/support/xpthread_mutexattr_destroy.c
@@ -0,0 +1,26 @@
+/* pthread_mutexattr_destroy with error checking.
+   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 <support/xthread.h>
+
+void
+xpthread_mutexattr_destroy (pthread_mutexattr_t *attr)
+{
+  xpthread_check_return ("pthread_mutexattr_destroy",
+                         pthread_mutexattr_destroy (attr));
+}
diff --git a/support/xpthread_mutexattr_init.c b/support/xpthread_mutexattr_init.c
new file mode 100644
index 0000000..fa93fab
--- /dev/null
+++ b/support/xpthread_mutexattr_init.c
@@ -0,0 +1,25 @@
+/* pthread_mutexattr_init with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutexattr_init (pthread_mutexattr_t *attr)
+{
+  xpthread_check_return ("pthread_mutexattr_init", pthread_mutexattr_init (attr));
+}
diff --git a/support/xpthread_mutexattr_setprotocol.c b/support/xpthread_mutexattr_setprotocol.c
new file mode 100644
index 0000000..353f75e
--- /dev/null
+++ b/support/xpthread_mutexattr_setprotocol.c
@@ -0,0 +1,26 @@
+/* pthread_mutexattr_setprotocol with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutexattr_setprotocol (pthread_mutexattr_t *attr, int flag)
+{
+  xpthread_check_return ("pthread_mutexattr_setprotocol",
+                         pthread_mutexattr_setprotocol (attr, flag));
+}
diff --git a/support/xpthread_mutexattr_setpshared.c b/support/xpthread_mutexattr_setpshared.c
new file mode 100644
index 0000000..242da1a
--- /dev/null
+++ b/support/xpthread_mutexattr_setpshared.c
@@ -0,0 +1,26 @@
+/* pthread_mutexattr_setpshared with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutexattr_setpshared (pthread_mutexattr_t *attr, int flag)
+{
+  xpthread_check_return ("pthread_mutexattr_setpshared",
+                         pthread_mutexattr_setpshared (attr, flag));
+}
diff --git a/support/xpthread_mutexattr_setrobust.c b/support/xpthread_mutexattr_setrobust.c
new file mode 100644
index 0000000..d7d6fa8
--- /dev/null
+++ b/support/xpthread_mutexattr_setrobust.c
@@ -0,0 +1,26 @@
+/* pthread_mutexattr_setrobust with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutexattr_setrobust (pthread_mutexattr_t *attr, int flag)
+{
+  xpthread_check_return ("pthread_mutexattr_setrobust",
+                         pthread_mutexattr_setrobust (attr, flag));
+}
diff --git a/support/xpthread_mutexattr_settype.c b/support/xpthread_mutexattr_settype.c
new file mode 100644
index 0000000..cf22170
--- /dev/null
+++ b/support/xpthread_mutexattr_settype.c
@@ -0,0 +1,26 @@
+/* pthread_mutexattr_settype with error checking.
+   Copyright (C) 2016-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 <support/xthread.h>
+
+void
+xpthread_mutexattr_settype (pthread_mutexattr_t *attr, int flag)
+{
+  xpthread_check_return ("pthread_mutexattr_settype",
+                         pthread_mutexattr_settype (attr, flag));
+}
diff --git a/support/xthread.h b/support/xthread.h
index e5c896b..0eb54fd 100644
--- a/support/xthread.h
+++ b/support/xthread.h
@@ -41,8 +41,17 @@ void xpthread_check_return (const char *function, int value);
 void xpthread_barrier_init (pthread_barrier_t *barrier,
                             pthread_barrierattr_t *attr, unsigned int count);
 void xpthread_barrier_destroy (pthread_barrier_t *barrier);
+void xpthread_mutexattr_destroy (pthread_mutexattr_t *);
+void xpthread_mutexattr_init (pthread_mutexattr_t *);
+void xpthread_mutexattr_setprotocol (pthread_mutexattr_t *, int);
+void xpthread_mutexattr_setpshared (pthread_mutexattr_t *, int);
+void xpthread_mutexattr_setrobust (pthread_mutexattr_t *, int);
+void xpthread_mutexattr_settype (pthread_mutexattr_t *, int);
+void xpthread_mutex_init (pthread_mutex_t *, const pthread_mutexattr_t *);
+void xpthread_mutex_destroy (pthread_mutex_t *);
 void xpthread_mutex_lock (pthread_mutex_t *mutex);
 void xpthread_mutex_unlock (pthread_mutex_t *mutex);
+void xpthread_mutex_consistent (pthread_mutex_t *);
 void xpthread_spin_lock (pthread_spinlock_t *lock);
 void xpthread_spin_unlock (pthread_spinlock_t *lock);
 void xpthread_cond_wait (pthread_cond_t * cond, pthread_mutex_t * mutex);
diff --git a/support/xunistd.h b/support/xunistd.h
index 11fbc9f..a83b1f4 100644
--- a/support/xunistd.h
+++ b/support/xunistd.h
@@ -33,6 +33,11 @@ pid_t xwaitpid (pid_t, int *status, int flags);
 /* Write the buffer.  Retry on short writes.  */
 void xwrite (int, const void *, size_t);
 
+/* Invoke mmap with a zero file offset.  */
+void *xmmap (void *addr, size_t length, int prot, int flags, int fd);
+
+void xmunmap (void *addr, size_t length);
+
 __END_DECLS
 
 #endif /* SUPPORT_XUNISTD_H */

On Mon, 2017-01-23 at 15:57 +0100, Florian Weimer wrote:
> On 01/13/2017 04:37 PM, Torvald Riegel wrote:

> > On Fri, 2017-01-13 at 14:11 +0100, Florian Weimer wrote:

> >> On 12/23/2016 10:31 PM, Torvald Riegel wrote:

> >>>> Can we add a test case for this?

> >>> What do you have in mind?  Checking that the list is reset to zero after

> >>> fork?  Do we need a test for that if we have documented the need to do

> >>> that in the code?

> >>

> >> What about the attached patch?

> >

> > That looks fine generally, with the following exceptions / comments:

> >

> > According to the feedback so far we got for

> > http://austingroupbugs.net/view.php?id=1112 the trylock-based test is

> > actually UB.  There should be a comment making that clear (ie, that we

> > rely on something we know about our current implementation to test

> > this).

> >

> > If this was a multi-threaded test program, it would be invalid because

> > non-async-signal-safe functions are executed in the child before exec()

> > is called.  In a single-threaded program, it's not quite clear to me

> > what POSIX really wants.  There's indication that only AS-safe functions

> > should be called, but IIRC that's not explicitly stated.

> > Florian says we still need to support this as an extension.  If so, this

> > should be made clear in the test (using a suitable comment).

> 

> I added a comment and an additional test which uses an error-checking mutex.

> 

> Test case still passes on current master.  Okay to commit?


LGTM.

On 01/24/2017 09:27 PM, Torvald Riegel wrote:
> On Mon, 2017-01-23 at 15:57 +0100, Florian Weimer wrote:

>> On 01/13/2017 04:37 PM, Torvald Riegel wrote:

>>> On Fri, 2017-01-13 at 14:11 +0100, Florian Weimer wrote:

>>>> On 12/23/2016 10:31 PM, Torvald Riegel wrote:

>>>>>> Can we add a test case for this?

>>>>> What do you have in mind?  Checking that the list is reset to zero after

>>>>> fork?  Do we need a test for that if we have documented the need to do

>>>>> that in the code?

>>>>

>>>> What about the attached patch?

>>>

>>> That looks fine generally, with the following exceptions / comments:

>>>

>>> According to the feedback so far we got for

>>> http://austingroupbugs.net/view.php?id=1112 the trylock-based test is

>>> actually UB.  There should be a comment making that clear (ie, that we

>>> rely on something we know about our current implementation to test

>>> this).

>>>

>>> If this was a multi-threaded test program, it would be invalid because

>>> non-async-signal-safe functions are executed in the child before exec()

>>> is called.  In a single-threaded program, it's not quite clear to me

>>> what POSIX really wants.  There's indication that only AS-safe functions

>>> should be called, but IIRC that's not explicitly stated.

>>> Florian says we still need to support this as an extension.  If so, this

>>> should be made clear in the test (using a suitable comment).

>>

>> I added a comment and an additional test which uses an error-checking mutex.

>>

>> Test case still passes on current master.  Okay to commit?

>

> LGTM.


Siddhesh, is this okay despite the freeze?  It's a test-only change:

   <https://sourceware.org/ml/libc-alpha/2017-01/msg00423.html>

Thanks,
Florian

Yes, this is fine.

Siddhesh