New condvar implementation that provides stronger ordering guarantees.

Message ID 1482510792.14990.820.camel@redhat.com
State New
Headers show

Commit Message

Torvald Riegel Dec. 23, 2016, 4:33 p.m.
On Tue, 2016-06-14 at 22:53 +0200, Torvald Riegel wrote:
> I've now tested this patch successfully using our existing tests on ppc,

> ppc64, ppc64le, s390x, and aarch64.  I couldn't test on s390 due to

> https://www.sourceware.org/ml/libc-alpha/2016-06/msg00545.html.

> 

> The attached patch is a minor revision that just fixes some formatting

> and adds an include (revealing by testing on s390x).


I have attached a followup patch that adds support for the pretty
printers added in the meantime.  For ease-of-review, I'm sending this as
a separate path and not a revision of the previous patch; I will commit
it as a single patch though.

Comments

Carlos O'Donell Dec. 31, 2016, 11:10 a.m. | #1
On 12/23/2016 11:33 AM, Torvald Riegel wrote:
> On Tue, 2016-06-14 at 22:53 +0200, Torvald Riegel wrote:

>> I've now tested this patch successfully using our existing tests on ppc,

>> ppc64, ppc64le, s390x, and aarch64.  I couldn't test on s390 due to

>> https://www.sourceware.org/ml/libc-alpha/2016-06/msg00545.html.

>>

>> The attached patch is a minor revision that just fixes some formatting

>> and adds an include (revealing by testing on s390x).

> I have attached a followup patch that adds support for the pretty

> printers added in the meantime.  For ease-of-review, I'm sending this as

> a separate path and not a revision of the previous patch; I will commit

> it as a single patch though.


This looks good to me, though the implementation is fairly rudimentary.

I would like us to continue to make this better _before_ the release to
help developers working on the various ports that need to make this work.
When I say "better" I mean expose more algorithm details.

So this should get checked in with the new condvar, but we should cricle
back to this before the release.

Cheers,
Carlos.

> commit 157cc3cc3e7c6656337f23079272d4247dd53814

> Author: Torvald Riegel <triegel@redhat.com>

> Date:   Fri Dec 23 14:52:08 2016 +0100

> 

>     Additional support for pretty printers.

> 

> diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py

> index e402f23..c207e9c 100644

> --- a/nptl/nptl-printers.py

> +++ b/nptl/nptl-printers.py

> @@ -293,16 +293,6 @@ class MutexAttributesPrinter(object):

>          elif protocol == PTHREAD_PRIO_PROTECT:

>              self.values.append(('Protocol', 'Priority protect'))

>  

> -CLOCK_IDS = {

> -    CLOCK_REALTIME: 'CLOCK_REALTIME',

> -    CLOCK_MONOTONIC: 'CLOCK_MONOTONIC',

> -    CLOCK_PROCESS_CPUTIME_ID: 'CLOCK_PROCESS_CPUTIME_ID',

> -    CLOCK_THREAD_CPUTIME_ID: 'CLOCK_THREAD_CPUTIME_ID',

> -    CLOCK_MONOTONIC_RAW: 'CLOCK_MONOTONIC_RAW',

> -    CLOCK_REALTIME_COARSE: 'CLOCK_REALTIME_COARSE',

> -    CLOCK_MONOTONIC_COARSE: 'CLOCK_MONOTONIC_COARSE'

> -}

> -

>  class ConditionVariablePrinter(object):

>      """Pretty printer for pthread_cond_t."""

>  

> @@ -313,24 +303,8 @@ class ConditionVariablePrinter(object):

>              cond: A gdb.value representing a pthread_cond_t.

>          """

>  

> -        # Since PTHREAD_COND_SHARED is an integer, we need to cast it to void *

> -        # to be able to compare it to the condvar's __data.__mutex member.

> -        #

> -        # While it looks like self.shared_value should be a class variable,

> -        # that would result in it having an incorrect size if we're loading

> -        # these printers through .gdbinit for a 64-bit objfile in AMD64.

> -        # This is because gdb initially assumes the pointer size to be 4 bytes,

> -        # and only sets it to 8 after loading the 64-bit objfiles.  Since

> -        # .gdbinit runs before any objfiles are loaded, this would effectively

> -        # make self.shared_value have a size of 4, thus breaking later

> -        # comparisons with pointers whose types are looked up at runtime.

> -        void_ptr_type = gdb.lookup_type('void').pointer()

> -        self.shared_value = gdb.Value(PTHREAD_COND_SHARED).cast(void_ptr_type)

> -

>          data = cond['__data']

> -        self.total_seq = data['__total_seq']

> -        self.mutex = data['__mutex']

> -        self.nwaiters = data['__nwaiters']

> +        self.wrefs = data['__wrefs']

>          self.values = []

>  

>          self.read_values()

> @@ -360,7 +334,6 @@ class ConditionVariablePrinter(object):

>  

>          self.read_status()

>          self.read_attributes()

> -        self.read_mutex_info()

>  

>      def read_status(self):

>          """Read the status of the condvar.

> @@ -369,41 +342,22 @@ class ConditionVariablePrinter(object):

>          are waiting for it.

>          """

>  

> -        if self.total_seq == PTHREAD_COND_DESTROYED:

> -            self.values.append(('Status', 'Destroyed'))

> -

> -        self.values.append(('Threads waiting for this condvar',

> -                            self.nwaiters >> COND_NWAITERS_SHIFT))

> +        self.values.append(('Threads known to still execute a wait function',

> +                            self.wrefs >> PTHREAD_COND_WREFS_SHIFT))

>  

>      def read_attributes(self):

>          """Read the condvar's attributes."""

>  

> -        clock_id = self.nwaiters & ((1 << COND_NWAITERS_SHIFT) - 1)

> -

> -        # clock_id must be casted to int because it's a gdb.Value

> -        self.values.append(('Clock ID', CLOCK_IDS[int(clock_id)]))

> +	if (self.wrefs & PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0:

> +        	self.values.append(('Clock ID', 'CLOCK_MONOTONIC'))

> +	else:

> +        	self.values.append(('Clock ID', 'CLOCK_REALTIME'))

>  

> -        shared = (self.mutex == self.shared_value)

> -

> -        if shared:

> +        if (self.wrefs & PTHREAD_COND_SHARED_MASK) != 0:

>              self.values.append(('Shared', 'Yes'))

>          else:

>              self.values.append(('Shared', 'No'))

>  

> -    def read_mutex_info(self):

> -        """Read the data of the mutex this condvar is bound to.

> -

> -        A pthread_cond_t's __data.__mutex member is a void * which

> -        must be casted to pthread_mutex_t *.  For shared condvars, this

> -        member isn't recorded and has a special value instead.

> -        """

> -

> -        if self.mutex and self.mutex != self.shared_value:

> -            mutex_type = gdb.lookup_type('pthread_mutex_t')

> -            mutex = self.mutex.cast(mutex_type.pointer()).dereference()

> -

> -            self.values.append(('Mutex', mutex))

> -

>  class ConditionVariableAttributesPrinter(object):

>      """Pretty printer for pthread_condattr_t.

>  

> @@ -453,10 +407,12 @@ class ConditionVariableAttributesPrinter(object):

>          created in self.children.

>          """

>  

> -        clock_id = self.condattr & ((1 << COND_NWAITERS_SHIFT) - 1)

> +        clock_id = (self.condattr >> 1) & ((1 << COND_CLOCK_BITS) - 1)

>  

> -        # clock_id must be casted to int because it's a gdb.Value

> -        self.values.append(('Clock ID', CLOCK_IDS[int(clock_id)]))

> +	if clock_id != 0:

> +        	self.values.append(('Clock ID', 'CLOCK_MONOTONIC'))

> +	else:

> +        	self.values.append(('Clock ID', 'CLOCK_REALTIME'))

>  

>          if self.condattr & 1:

>              self.values.append(('Shared', 'Yes'))

> diff --git a/nptl/nptl_lock_constants.pysym b/nptl/nptl_lock_constants.pysym

> index 303ec61..2ab3179 100644

> --- a/nptl/nptl_lock_constants.pysym

> +++ b/nptl/nptl_lock_constants.pysym

> @@ -44,26 +44,13 @@ PTHREAD_PRIO_NONE

>  PTHREAD_PRIO_INHERIT

>  PTHREAD_PRIO_PROTECT

>  

> --- These values are hardcoded as well:

> --- Value of __mutex for shared condvars.

> -PTHREAD_COND_SHARED             (void *)~0l

> -

> --- Value of __total_seq for destroyed condvars.

> -PTHREAD_COND_DESTROYED          -1ull

> -

> --- __nwaiters encodes the number of threads waiting on a condvar

> --- and the clock ID.

> --- __nwaiters >> COND_NWAITERS_SHIFT gives us the number of waiters.

> -COND_NWAITERS_SHIFT

> -

> --- Condvar clock IDs

> -CLOCK_REALTIME

> -CLOCK_MONOTONIC

> -CLOCK_PROCESS_CPUTIME_ID

> -CLOCK_THREAD_CPUTIME_ID

> -CLOCK_MONOTONIC_RAW

> -CLOCK_REALTIME_COARSE

> -CLOCK_MONOTONIC_COARSE

> +-- Condition variable

> +-- FIXME Why do macros prefixed with __ cannot be used directly?

> +PTHREAD_COND_SHARED_MASK          __PTHREAD_COND_SHARED_MASK

> +PTHREAD_COND_CLOCK_MONOTONIC_MASK __PTHREAD_COND_CLOCK_MONOTONIC_MASK

> +COND_CLOCK_BITS

> +-- These values are hardcoded:

> +PTHREAD_COND_WREFS_SHIFT          3

>  

>  -- Rwlock attributes

>  PTHREAD_RWLOCK_PREFER_READER_NP

> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h

> index 6e0dd09..92a9992 100644

> --- a/nptl/pthreadP.h

> +++ b/nptl/pthreadP.h

> @@ -167,6 +167,13 @@ enum

>  #define __PTHREAD_ONCE_FORK_GEN_INCR	4

>  

>  

> +/* Condition variable definitions.  See __pthread_cond_wait_common.

> +   Need to be defined here so there is one place from which

> +   nptl_lock_constants can grab them.  */

> +#define __PTHREAD_COND_CLOCK_MONOTONIC_MASK 2

> +#define __PTHREAD_COND_SHARED_MASK 1

> +

> +

>  /* Internal variables.  */

>  

>  

> diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c

> index cdd9b7d..c1eac5f 100644

> --- a/nptl/pthread_cond_init.c

> +++ b/nptl/pthread_cond_init.c

> @@ -29,6 +29,10 @@ __pthread_cond_init (pthread_cond_t *cond, const pthread_condattr_t *cond_attr)

>    struct pthread_condattr *icond_attr = (struct pthread_condattr *) cond_attr;

>  

>    memset (cond, 0, sizeof (pthread_cond_t));

> +

> +  /* Update the pretty printers if the internal representation of icond_attr

> +     is changed.  */

> +

>    /* Iff not equal to ~0l, this is a PTHREAD_PROCESS_PRIVATE condvar.  */

>    if (icond_attr != NULL && (icond_attr->value & 1) != 0)

>      cond->__data.__wrefs |= __PTHREAD_COND_SHARED_MASK;

> diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c

> index 984f01f..2b43402 100644

> --- a/nptl/pthread_cond_wait.c

> +++ b/nptl/pthread_cond_wait.c

> @@ -293,6 +293,8 @@ __condvar_cleanup_waiting (void *arg)

>       * Bit 1 is the clock ID (0 == CLOCK_REALTIME, 1 == CLOCK_MONOTONIC).

>       * Bit 0 is true iff this is a process-shared condvar.

>       * Simple reference count used by both waiters and pthread_cond_destroy.

> +     (If the format of __wrefs is changed, update nptl_lock_constants.pysym

> +      and the pretty printers.)

>     For each of the two groups, we have:

>     __g_refs: Futex waiter reference count.

>       * LSB is true if waiters should run futex_wake when they remove the

> diff --git a/nptl/test-cond-printers.py b/nptl/test-cond-printers.py

> index af0e12e..9e807c9 100644

> --- a/nptl/test-cond-printers.py

> +++ b/nptl/test-cond-printers.py

> @@ -35,7 +35,7 @@ try:

>  

>      break_at(test_source, 'Test status (destroyed)')

>      continue_cmd() # Go to test_status_destroyed

> -    test_printer(var, to_string, {'Status': 'Destroyed'})

> +    test_printer(var, to_string, {'Threads known to still execute a wait function': '0'})

>  

>      continue_cmd() # Exit

>  

> diff --git a/scripts/test_printers_common.py b/scripts/test_printers_common.py

> index c79d7e3..91cf5ec 100644

> --- a/scripts/test_printers_common.py

> +++ b/scripts/test_printers_common.py

> @@ -157,7 +157,7 @@ def init_test(test_bin, printer_files, printer_names):

>  

>      # Load all the pretty printer files.  We're assuming these are safe.

>      for printer_file in printer_files:

> -        test('source {0}'.format(printer_file))

> +        test('source {0}'.format(printer_file), '')

>  

>      # Disable all the pretty printers.

>      test('disable pretty-printer', r'0 of [0-9]+ printers enabled')

> diff --git a/sysdeps/x86/bits/pthreadtypes.h b/sysdeps/x86/bits/pthreadtypes.h

> index 59b1d0d..38bc454 100644

> --- a/sysdeps/x86/bits/pthreadtypes.h

> +++ b/sysdeps/x86/bits/pthreadtypes.h

> @@ -161,8 +161,6 @@ typedef union

>      unsigned int __g1_orig_size;

>      unsigned int __wrefs;

>      unsigned int __g_signals[2];

> -#define __PTHREAD_COND_CLOCK_MONOTONIC_MASK 2

> -#define __PTHREAD_COND_SHARED_MASK 1

>    } __data;

>    char __size[__SIZEOF_PTHREAD_COND_T];

>    __extension__ long long int __align;



-- 
Cheers,
Carlos.

Patch hide | download patch | download mbox

commit 157cc3cc3e7c6656337f23079272d4247dd53814
Author: Torvald Riegel <triegel@redhat.com>
Date:   Fri Dec 23 14:52:08 2016 +0100

    Additional support for pretty printers.

diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py
index e402f23..c207e9c 100644
--- a/nptl/nptl-printers.py
+++ b/nptl/nptl-printers.py
@@ -293,16 +293,6 @@  class MutexAttributesPrinter(object):
         elif protocol == PTHREAD_PRIO_PROTECT:
             self.values.append(('Protocol', 'Priority protect'))
 
-CLOCK_IDS = {
-    CLOCK_REALTIME: 'CLOCK_REALTIME',
-    CLOCK_MONOTONIC: 'CLOCK_MONOTONIC',
-    CLOCK_PROCESS_CPUTIME_ID: 'CLOCK_PROCESS_CPUTIME_ID',
-    CLOCK_THREAD_CPUTIME_ID: 'CLOCK_THREAD_CPUTIME_ID',
-    CLOCK_MONOTONIC_RAW: 'CLOCK_MONOTONIC_RAW',
-    CLOCK_REALTIME_COARSE: 'CLOCK_REALTIME_COARSE',
-    CLOCK_MONOTONIC_COARSE: 'CLOCK_MONOTONIC_COARSE'
-}
-
 class ConditionVariablePrinter(object):
     """Pretty printer for pthread_cond_t."""
 
@@ -313,24 +303,8 @@  class ConditionVariablePrinter(object):
             cond: A gdb.value representing a pthread_cond_t.
         """
 
-        # Since PTHREAD_COND_SHARED is an integer, we need to cast it to void *
-        # to be able to compare it to the condvar's __data.__mutex member.
-        #
-        # While it looks like self.shared_value should be a class variable,
-        # that would result in it having an incorrect size if we're loading
-        # these printers through .gdbinit for a 64-bit objfile in AMD64.
-        # This is because gdb initially assumes the pointer size to be 4 bytes,
-        # and only sets it to 8 after loading the 64-bit objfiles.  Since
-        # .gdbinit runs before any objfiles are loaded, this would effectively
-        # make self.shared_value have a size of 4, thus breaking later
-        # comparisons with pointers whose types are looked up at runtime.
-        void_ptr_type = gdb.lookup_type('void').pointer()
-        self.shared_value = gdb.Value(PTHREAD_COND_SHARED).cast(void_ptr_type)
-
         data = cond['__data']
-        self.total_seq = data['__total_seq']
-        self.mutex = data['__mutex']
-        self.nwaiters = data['__nwaiters']
+        self.wrefs = data['__wrefs']
         self.values = []
 
         self.read_values()
@@ -360,7 +334,6 @@  class ConditionVariablePrinter(object):
 
         self.read_status()
         self.read_attributes()
-        self.read_mutex_info()
 
     def read_status(self):
         """Read the status of the condvar.
@@ -369,41 +342,22 @@  class ConditionVariablePrinter(object):
         are waiting for it.
         """
 
-        if self.total_seq == PTHREAD_COND_DESTROYED:
-            self.values.append(('Status', 'Destroyed'))
-
-        self.values.append(('Threads waiting for this condvar',
-                            self.nwaiters >> COND_NWAITERS_SHIFT))
+        self.values.append(('Threads known to still execute a wait function',
+                            self.wrefs >> PTHREAD_COND_WREFS_SHIFT))
 
     def read_attributes(self):
         """Read the condvar's attributes."""
 
-        clock_id = self.nwaiters & ((1 << COND_NWAITERS_SHIFT) - 1)
-
-        # clock_id must be casted to int because it's a gdb.Value
-        self.values.append(('Clock ID', CLOCK_IDS[int(clock_id)]))
+	if (self.wrefs & PTHREAD_COND_CLOCK_MONOTONIC_MASK) != 0:
+        	self.values.append(('Clock ID', 'CLOCK_MONOTONIC'))
+	else:
+        	self.values.append(('Clock ID', 'CLOCK_REALTIME'))
 
-        shared = (self.mutex == self.shared_value)
-
-        if shared:
+        if (self.wrefs & PTHREAD_COND_SHARED_MASK) != 0:
             self.values.append(('Shared', 'Yes'))
         else:
             self.values.append(('Shared', 'No'))
 
-    def read_mutex_info(self):
-        """Read the data of the mutex this condvar is bound to.
-
-        A pthread_cond_t's __data.__mutex member is a void * which
-        must be casted to pthread_mutex_t *.  For shared condvars, this
-        member isn't recorded and has a special value instead.
-        """
-
-        if self.mutex and self.mutex != self.shared_value:
-            mutex_type = gdb.lookup_type('pthread_mutex_t')
-            mutex = self.mutex.cast(mutex_type.pointer()).dereference()
-
-            self.values.append(('Mutex', mutex))
-
 class ConditionVariableAttributesPrinter(object):
     """Pretty printer for pthread_condattr_t.
 
@@ -453,10 +407,12 @@  class ConditionVariableAttributesPrinter(object):
         created in self.children.
         """
 
-        clock_id = self.condattr & ((1 << COND_NWAITERS_SHIFT) - 1)
+        clock_id = (self.condattr >> 1) & ((1 << COND_CLOCK_BITS) - 1)
 
-        # clock_id must be casted to int because it's a gdb.Value
-        self.values.append(('Clock ID', CLOCK_IDS[int(clock_id)]))
+	if clock_id != 0:
+        	self.values.append(('Clock ID', 'CLOCK_MONOTONIC'))
+	else:
+        	self.values.append(('Clock ID', 'CLOCK_REALTIME'))
 
         if self.condattr & 1:
             self.values.append(('Shared', 'Yes'))
diff --git a/nptl/nptl_lock_constants.pysym b/nptl/nptl_lock_constants.pysym
index 303ec61..2ab3179 100644
--- a/nptl/nptl_lock_constants.pysym
+++ b/nptl/nptl_lock_constants.pysym
@@ -44,26 +44,13 @@  PTHREAD_PRIO_NONE
 PTHREAD_PRIO_INHERIT
 PTHREAD_PRIO_PROTECT
 
--- These values are hardcoded as well:
--- Value of __mutex for shared condvars.
-PTHREAD_COND_SHARED             (void *)~0l
-
--- Value of __total_seq for destroyed condvars.
-PTHREAD_COND_DESTROYED          -1ull
-
--- __nwaiters encodes the number of threads waiting on a condvar
--- and the clock ID.
--- __nwaiters >> COND_NWAITERS_SHIFT gives us the number of waiters.
-COND_NWAITERS_SHIFT
-
--- Condvar clock IDs
-CLOCK_REALTIME
-CLOCK_MONOTONIC
-CLOCK_PROCESS_CPUTIME_ID
-CLOCK_THREAD_CPUTIME_ID
-CLOCK_MONOTONIC_RAW
-CLOCK_REALTIME_COARSE
-CLOCK_MONOTONIC_COARSE
+-- Condition variable
+-- FIXME Why do macros prefixed with __ cannot be used directly?
+PTHREAD_COND_SHARED_MASK          __PTHREAD_COND_SHARED_MASK
+PTHREAD_COND_CLOCK_MONOTONIC_MASK __PTHREAD_COND_CLOCK_MONOTONIC_MASK
+COND_CLOCK_BITS
+-- These values are hardcoded:
+PTHREAD_COND_WREFS_SHIFT          3
 
 -- Rwlock attributes
 PTHREAD_RWLOCK_PREFER_READER_NP
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 6e0dd09..92a9992 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -167,6 +167,13 @@  enum
 #define __PTHREAD_ONCE_FORK_GEN_INCR	4
 
 
+/* Condition variable definitions.  See __pthread_cond_wait_common.
+   Need to be defined here so there is one place from which
+   nptl_lock_constants can grab them.  */
+#define __PTHREAD_COND_CLOCK_MONOTONIC_MASK 2
+#define __PTHREAD_COND_SHARED_MASK 1
+
+
 /* Internal variables.  */
 
 
diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
index cdd9b7d..c1eac5f 100644
--- a/nptl/pthread_cond_init.c
+++ b/nptl/pthread_cond_init.c
@@ -29,6 +29,10 @@  __pthread_cond_init (pthread_cond_t *cond, const pthread_condattr_t *cond_attr)
   struct pthread_condattr *icond_attr = (struct pthread_condattr *) cond_attr;
 
   memset (cond, 0, sizeof (pthread_cond_t));
+
+  /* Update the pretty printers if the internal representation of icond_attr
+     is changed.  */
+
   /* Iff not equal to ~0l, this is a PTHREAD_PROCESS_PRIVATE condvar.  */
   if (icond_attr != NULL && (icond_attr->value & 1) != 0)
     cond->__data.__wrefs |= __PTHREAD_COND_SHARED_MASK;
diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
index 984f01f..2b43402 100644
--- a/nptl/pthread_cond_wait.c
+++ b/nptl/pthread_cond_wait.c
@@ -293,6 +293,8 @@  __condvar_cleanup_waiting (void *arg)
      * Bit 1 is the clock ID (0 == CLOCK_REALTIME, 1 == CLOCK_MONOTONIC).
      * Bit 0 is true iff this is a process-shared condvar.
      * Simple reference count used by both waiters and pthread_cond_destroy.
+     (If the format of __wrefs is changed, update nptl_lock_constants.pysym
+      and the pretty printers.)
    For each of the two groups, we have:
    __g_refs: Futex waiter reference count.
      * LSB is true if waiters should run futex_wake when they remove the
diff --git a/nptl/test-cond-printers.py b/nptl/test-cond-printers.py
index af0e12e..9e807c9 100644
--- a/nptl/test-cond-printers.py
+++ b/nptl/test-cond-printers.py
@@ -35,7 +35,7 @@  try:
 
     break_at(test_source, 'Test status (destroyed)')
     continue_cmd() # Go to test_status_destroyed
-    test_printer(var, to_string, {'Status': 'Destroyed'})
+    test_printer(var, to_string, {'Threads known to still execute a wait function': '0'})
 
     continue_cmd() # Exit
 
diff --git a/scripts/test_printers_common.py b/scripts/test_printers_common.py
index c79d7e3..91cf5ec 100644
--- a/scripts/test_printers_common.py
+++ b/scripts/test_printers_common.py
@@ -157,7 +157,7 @@  def init_test(test_bin, printer_files, printer_names):
 
     # Load all the pretty printer files.  We're assuming these are safe.
     for printer_file in printer_files:
-        test('source {0}'.format(printer_file))
+        test('source {0}'.format(printer_file), '')
 
     # Disable all the pretty printers.
     test('disable pretty-printer', r'0 of [0-9]+ printers enabled')
diff --git a/sysdeps/x86/bits/pthreadtypes.h b/sysdeps/x86/bits/pthreadtypes.h
index 59b1d0d..38bc454 100644
--- a/sysdeps/x86/bits/pthreadtypes.h
+++ b/sysdeps/x86/bits/pthreadtypes.h
@@ -161,8 +161,6 @@  typedef union
     unsigned int __g1_orig_size;
     unsigned int __wrefs;
     unsigned int __g_signals[2];
-#define __PTHREAD_COND_CLOCK_MONOTONIC_MASK 2
-#define __PTHREAD_COND_SHARED_MASK 1
   } __data;
   char __size[__SIZEOF_PTHREAD_COND_T];
   __extension__ long long int __align;