diff mbox

[PING,2,v3] Add pretty printers for the NPTL lock types

Message ID 1447768994-5368-1-git-send-email-martin.galvan@tallertechnologies.com
State New
Headers show

Commit Message

Martin Galvan Nov. 17, 2015, 2:03 p.m. UTC
This patch adds the pretty-printers for the following NPTL types:

- pthread_mutex_t
- pthread_mutexattr_t
- pthread_cond_t
- pthread_condattr_t
- pthread_rwlock_t
- pthread_rwlockattr_t

To load the pretty-printers into your gdb session, do the following:

python
import sys
sys.path.insert(0, '/path/to/glibc/build/')
end
source /path/to/glibc/source/bin/nptl-printers.py

You can check which printers are registered and enabled by issuing the
'info pretty-printer' gdb command. Printers should trigger automatically when
trying to print a variable of one of the types mentioned above.

The printers are architecture-independent, and were tested on both the gdb CLI
and Eclipse CDT.

In order to work, the printers need to know the values of various flags that
are scattered throughout pthread.h and pthreadP.h as enums and #defines. Since
replicating these constants in the printers file itself would create a
maintenance burden, I followed Joseph Myers' advice and wrote a script
called gen-py-const.awk that Makerules uses to extract the constants.
This script is pretty much the same as gen-as-const.awk, except it doesn't cast
the constant values to 'long' and is thorougly documented. The constants need
only to be enumerated in a .pysym file, which is then referenced by a Make
variable called gen-py-const-headers.

As for the install directory, I discussed this with Mike Frysinger and we agreed
that this should be handled by the maintainers of each distro, since the paths
may vary from one distro to another. We also agreed that this issue shouldn't
block the merging of this patch either.

I have a company-wide copyright assignment for glibc. I don't have write access,
though, so if anyone could commit this for me it would be great.

ChangeLog:

2015-11-07  Martin Galvan  <martin.galvan@tallertechnologies.com>

	* Makerules ($(common-objpfx)%.py): New target.
	* nptl/Makefile (gen-py-const-headers): New define.
	* nptl/nptl-printers.py: New file.
	* nptl/nptl_lock_constants.pysym: New file.
	* scripts/gen-py-const.awk: New file.

---
 Makerules                      |  27 ++
 nptl/Makefile                  |   1 +
 nptl/nptl-printers.py          | 629 +++++++++++++++++++++++++++++++++++++++++
 nptl/nptl_lock_constants.pysym |  66 +++++
 scripts/gen-py-const.awk       | 116 ++++++++
 5 files changed, 839 insertions(+)
 create mode 100644 nptl/nptl-printers.py
 create mode 100644 nptl/nptl_lock_constants.pysym
 create mode 100644 scripts/gen-py-const.awk

--
2.6.3

Comments

Florian Weimer Nov. 17, 2015, 3:55 p.m. UTC | #1
On 11/17/2015 03:03 PM, Martin Galvan wrote:
> This patch adds the pretty-printers for the following NPTL types:
> 
> - pthread_mutex_t
> - pthread_mutexattr_t
> - pthread_cond_t
> - pthread_condattr_t
> - pthread_rwlock_t
> - pthread_rwlockattr_t

Is there a test for this somewhere?  Can it be put into glibc?

I'm worried that these helpers break the debugging experience at some
point, and we would like to catch that early.  A test in glibc proper
would help with that.

Thanks,
Florian
Martin Galvan Nov. 17, 2015, 5:26 p.m. UTC | #2
On Tue, Nov 17, 2015 at 12:55 PM, Florian Weimer <fweimer@redhat.com> wrote:
> Is there a test for this somewhere?  Can it be put into glibc?

If you mean automated unit tests, then no, there aren't any. I think
including them in glibc would be quite a hassle, since they'd need to
interact with gdb as well.

When developing these printers I based my work on the ones for
libstdc++. I think those don't come with unit tests either.

> I'm worried that these helpers break the debugging experience at some
> point, and we would like to catch that early.

Notice these printers won't necessarily have to be enabled by default.
If there was a problem with the printers, the user can simply keep the
default printing behavior. libstdc++ does this as well: I had to add a
bit of code to my .gdbinit so that those printers will always load
them at startup.

This is merely a tool to make the user's life easier. It's not by any
means a requirement for using glibc.
Tom Tromey Nov. 17, 2015, 5:34 p.m. UTC | #3
Martin> When developing these printers I based my work on the ones for
Martin> libstdc++. I think those don't come with unit tests either.

They do.  They work by invoking gdb and checking the expected output.

>> I'm worried that these helpers break the debugging experience at some
>> point, and we would like to catch that early.

FWIW it is a valid concern, this has happened with the libstdc++ printers.

Martin> libstdc++ does this as well: I had to add a
Martin> bit of code to my .gdbinit so that those printers will always load
Martin> them at startup.

With a correct setup (typically done by the distros), you shouldn't need
this.  That's what all the hook business is for.  (I didn't read your
patch, sorry; but you should do this if you haven't already.)

Tom
Martin Galvan Nov. 17, 2015, 7:14 p.m. UTC | #4
On Tue, Nov 17, 2015 at 2:34 PM, Tom Tromey <tom@tromey.com> wrote:
> With a correct setup (typically done by the distros), you shouldn't need
> this.

Curious, at least Ubuntu 14.04 doesn't seem to do this.

Anyway, I'd like this patch to be reviewed first so we can make sure
there are no (obvious) flaws. Mike Frysinger and Joseph Myers already
pointed out some things which I fixed for this version. Is there a
chance to merge just this code and later the unit tests, or must it
all go in as one big commit?
Carlos O'Donell Feb. 5, 2016, 4:19 p.m. UTC | #5
On 11/17/2015 10:55 AM, Florian Weimer wrote:
> On 11/17/2015 03:03 PM, Martin Galvan wrote:
>> This patch adds the pretty-printers for the following NPTL types:
>>
>> - pthread_mutex_t
>> - pthread_mutexattr_t
>> - pthread_cond_t
>> - pthread_condattr_t
>> - pthread_rwlock_t
>> - pthread_rwlockattr_t
> 
> Is there a test for this somewhere?  Can it be put into glibc?
> 
> I'm worried that these helpers break the debugging experience at some
> point, and we would like to catch that early.  A test in glibc proper
> would help with that.

Agreed. This needs testing.

You need to:

- Create a test script that is run as a test.
- The test script needs to check for gdb, this can't
  be done at host configure time because the test script
  might be run on a remote host for cross-testing via the
  test wrapper.
- If gdb is not present on the target then fail with
  UNSUPPORTED (not sure if this is possible, we usually
  mark unsupported testes by putting them into a special
  Make variable).
- If gdb is present then run it with a script (-x) and
  do some printing tests of various initialized mutexes
  and a test program, and then compare that to expected
  data (store the expected data in a file if you want)
  and return PASS or FAIL.

Cheers,
Carlso.
Joseph Myers Feb. 5, 2016, 4:28 p.m. UTC | #6
On Fri, 5 Feb 2016, Carlos O'Donell wrote:

> - Create a test script that is run as a test.

Note that right now we don't have any test scripts that run on the glibc 
host, they all run on the build system.  Thus, the makefile variables 
describing how to run programs for the glibc host, test-wrapper, 
test-wrapper-env and test-wrapper-env-only, all expect to be running a 
binary.

The effect is that a new variable or variables needs adding.  It might 
default to using existing variables, and that might be right when 
cross-testing via ssh - but people cross-testing using an emulator to run 
binaries (the same situation for which the default setting of 
test-wrapper-env may not work) might need to do something different.
Carlos O'Donell Feb. 5, 2016, 4:37 p.m. UTC | #7
On 02/05/2016 11:28 AM, Joseph Myers wrote:
> On Fri, 5 Feb 2016, Carlos O'Donell wrote:
> 
>> - Create a test script that is run as a test.
> 
> Note that right now we don't have any test scripts that run on the glibc 
> host, they all run on the build system.  Thus, the makefile variables 
> describing how to run programs for the glibc host, test-wrapper, 
> test-wrapper-env and test-wrapper-env-only, all expect to be running a 
> binary.

Good point.
 
> The effect is that a new variable or variables needs adding.  It might 
> default to using existing variables, and that might be right when 
> cross-testing via ssh - but people cross-testing using an emulator to run 
> binaries (the same situation for which the default setting of 
> test-wrapper-env may not work) might need to do something different.

When you say emulator I expect you mean something like a qemu userspace
emualtor that runs only the cross-platform binary without starting a full
system simulator?

I think we need to reject the notion that glibc testing is going to be
runnable on a userspace emulator. There is more testing that
I'd like to do which is going to require chroot or containers, and
therefore unusable without an ssh-like bridge to a complete target
system.

Thoughts?

Cheers,
Carlos.
Joseph Myers Feb. 5, 2016, 4:39 p.m. UTC | #8
On Fri, 5 Feb 2016, Carlos O'Donell wrote:

> When you say emulator I expect you mean something like a qemu userspace
> emualtor that runs only the cross-platform binary without starting a full
> system simulator?

Yes.  I think Roland has used something like that for NaCl testing, for 
example.  I'd expect the vast bulk of tests to work in that context, 
though maybe not threading tests or whole-system tests such as you 
mention.
Carlos O'Donell Feb. 5, 2016, 5:08 p.m. UTC | #9
On 02/05/2016 11:39 AM, Joseph Myers wrote:
> On Fri, 5 Feb 2016, Carlos O'Donell wrote:
> 
>> When you say emulator I expect you mean something like a qemu userspace
>> emualtor that runs only the cross-platform binary without starting a full
>> system simulator?
> 
> Yes.  I think Roland has used something like that for NaCl testing, for 
> example.  I'd expect the vast bulk of tests to work in that context, 
> though maybe not threading tests or whole-system tests such as you 
> mention.

I guess we could explicitly describe this notion and then allow the
testsuite to mark whole-system tests as UNSUPPORTED in configurations
that can't run them.

Cheers,
Carlos.
Siddhesh Poyarekar Feb. 9, 2016, 3:54 p.m. UTC | #10
Sorry about jumping on this late, I missed this patch on the list
somehow.

The most important requirement IMO is a README in the toplevel for
anyone looking to add pretty printers to other modules in glibc.  What
you have here is the beginnings of a feature set for glibc and we need
to make sure that others can build upon it easily.

> +class Printer(object):
> +    """Printer class which conforms to the gdb pretty printing interface."""
> +
> +    def __init__(self, name):
> +        self.name = name
> +        self.enabled = True
> +        self.subprinters = []
> +
> +    class Subprinter(object):
> +        """A regex-based printer.
> +
> +        Individual pretty-printers are registered as subprinters of a single
> +        Printer instance.
> +        """
> +
> +        def __init__(self, name, regex, callable_obj):
> +            """Initialize a pretty-printer.
> +
> +            Args:
> +                name: The name of the printer.
> +                regex: A regular expression.  When gdb tries to print a
> +                    variable whose type matches the regex, it'll trigger
> +                    this printer.
> +                callable_obj: A function or callable object that gdb will call
> +                    when trying to print some value.  It should return a
> +                    pretty-printer.
> +            """
> +            self.name = name
> +            self.regex = re.compile(regex)
> +            self.callable = callable_obj
> +            self.enabled = True
> +
> +    def add_subprinter(self, name, regex, callable_obj):
> +        """Register a regex-based subprinter."""
> +
> +        self.subprinters.append(self.Subprinter(name, regex, callable_obj))
> +
> +    def __call__(self, value):
> +        """gdb API function.
> +
> +        This is called when trying to print an inferior value from gdb.
> +        If a registered printer's regex matches the value's type, gdb will use
> +        the printer to print the value.
> +        """
> +
> +        type_name = value.type.name
> +
> +        if type_name:
> +            for subprinter in self.subprinters:
> +                if subprinter.enabled and subprinter.regex.match(type_name):
> +                    return subprinter.callable(value)
> +
> +        # Return None if we have no type name or if we can't find a subprinter
> +        # for the given type.
> +        return None
> +
> +def register(objfile):
> +    """Register the pretty printers within the given objfile."""
> +
> +    printer = Printer('Glibc pthread locks')
> +
> +    printer.add_subprinter('pthread_mutex_t', '^pthread_mutex_t$',
> +                           MutexPrinter)
> +    printer.add_subprinter('pthread_mutexattr_t', '^pthread_mutexattr_t$',
> +                           MutexAttributesPrinter)
> +    printer.add_subprinter('pthread_cond_t', '^pthread_cond_t$',
> +                           ConditionVariablePrinter)
> +    printer.add_subprinter('pthread_condattr_t', '^pthread_condattr_t$',
> +                           ConditionVariableAttributesPrinter)
> +    printer.add_subprinter('pthread_rwlock_t', '^pthread_rwlock_t$',
> +                           RWLockPrinter)
> +    printer.add_subprinter('pthread_rwlockattr_t', '^pthread_rwlockattr_t$',
> +                           RWLockAttributesPrinter)
> +
> +    gdb.printing.register_pretty_printer(objfile, printer)
> +
> +register(gdb.current_objfile())

These bits look generic, so I would suggest pulling them out into a
separate file and make it reusable for other modules' pretty
printers. In fact, I think the pretty printing code should go into a
separate directory (gdb-pp or similar, maybe put the README file in
this) so that all pretty printers stay in one place.  It would be
clumsy to have them in different module directories now and then
putting them all into the same directory during installation.  It
would be much easier to test them with all of them in the same
directory.

Siddhesh
Martin Galvan Feb. 9, 2016, 5:40 p.m. UTC | #11
On Fri, Feb 5, 2016 at 1:19 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> You need to:
>
> - Create a test script that is run as a test.
> - The test script needs to check for gdb, this can't
>   be done at host configure time because the test script
>   might be run on a remote host for cross-testing via the
>   test wrapper.
> - If gdb is not present on the target then fail with
>   UNSUPPORTED (not sure if this is possible, we usually
>   mark unsupported testes by putting them into a special
>   Make variable).
> - If gdb is present then run it with a script (-x) and
>   do some printing tests of various initialized mutexes
>   and a test program, and then compare that to expected
>   data (store the expected data in a file if you want)
>   and return PASS or FAIL.

I think it'd be way easier to just use PExpect in a single Python
script. I've been playing around with it and it seems to work fine,
and it uses a GPL-compatible license. I'll write a few test programs
with various kinds of lock types that exercise the pretty printers and
check for the output using PExpect.

On Fri, Feb 5, 2016 at 1:28 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> Note that right now we don't have any test scripts that run on the glibc
> host, they all run on the build system.  Thus, the makefile variables
> describing how to run programs for the glibc host, test-wrapper,
> test-wrapper-env and test-wrapper-env-only, all expect to be running a
> binary.
>
> The effect is that a new variable or variables needs adding.  It might
> default to using existing variables, and that might be right when
> cross-testing via ssh - but people cross-testing using an emulator to run
> binaries (the same situation for which the default setting of
> test-wrapper-env may not work) might need to do something different.

I'm sorry, but I don't understand what you mean by this, as I'm not
very familiar with the glibc Makefile system. If you mean that the
pretty printers should be somehow tested in the glibc host instead of
the build system (assuming they're different), I don't see why we
should do that. If we're cross-debugging, the printers wouldn't run on
the glibc host but on the machine we're running the main gdb instance
on.

In any case, this sounds like even more complexity added to this
particular contribution.

On Tue, Feb 9, 2016 at 12:54 PM, Siddhesh Poyarekar
<sid@reserved-bit.com> wrote:
> These bits look generic, so I would suggest pulling them out into a
> separate file and make it reusable for other modules' pretty
> printers.

Those bits will be gone away in the next patch. I had to define my own
Printer classes because of a bug in the gdb API that wouldn't trigger
pretty printers for typedef'd variables. That bug was fixed a while
ago in the gdb mainline, so I'll stick to using
gdb.RegexpCollectionPrettyPrinter.

> In fact, I think the pretty printing code should go into a
> separate directory (gdb-pp or similar, maybe put the README file in
> this) so that all pretty printers stay in one place.  It would be
> clumsy to have them in different module directories now and then
> putting them all into the same directory during installation.  It
> would be much easier to test them with all of them in the same
> directory.

Ok.
Joseph Myers Feb. 9, 2016, 5:48 p.m. UTC | #12
On Tue, 9 Feb 2016, Martin Galvan wrote:

> On Fri, Feb 5, 2016 at 1:28 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> > Note that right now we don't have any test scripts that run on the glibc
> > host, they all run on the build system.  Thus, the makefile variables
> > describing how to run programs for the glibc host, test-wrapper,
> > test-wrapper-env and test-wrapper-env-only, all expect to be running a
> > binary.
> >
> > The effect is that a new variable or variables needs adding.  It might
> > default to using existing variables, and that might be right when
> > cross-testing via ssh - but people cross-testing using an emulator to run
> > binaries (the same situation for which the default setting of
> > test-wrapper-env may not work) might need to do something different.
> 
> I'm sorry, but I don't understand what you mean by this, as I'm not

See Roland's original comments resulting in test-wrapper-env as a separate 
variable <https://sourceware.org/ml/libc-alpha/2012-10/msg00631.html>.

1. Work out where you want to run GDB when testing this feature - on the 
system on which glibc is built (possibly a cross GDB), or on the system on 
which glibc runs.

2. Work out what makefile variables are needed to describe how to run GDB 
when testing this feature.

> very familiar with the glibc Makefile system. If you mean that the
> pretty printers should be somehow tested in the glibc host instead of
> the build system (assuming they're different), I don't see why we
> should do that. If we're cross-debugging, the printers wouldn't run on
> the glibc host but on the machine we're running the main gdb instance
> on.

Yes, there is a good argument that the right way to test this feature when 
cross-building glibc is to use a cross-GDB (which may well still need new 
makefile variables to describe how to run the cross-GDB on the system 
running "make check" and the native gdbserver on the system running the 
newly built glibc).
Martin Galvan Feb. 10, 2016, 12:23 p.m. UTC | #13
On Tue, Feb 9, 2016 at 2:48 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> 1. Work out where you want to run GDB when testing this feature - on the
> system on which glibc is built (possibly a cross GDB), or on the system on
> which glibc runs.

What's the point on cross-testing the printers anyway? They're
platform-independent.
Joseph Myers Feb. 10, 2016, 1:01 p.m. UTC | #14
On Wed, 10 Feb 2016, Martin Galvan wrote:

> On Tue, Feb 9, 2016 at 2:48 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> > 1. Work out where you want to run GDB when testing this feature - on the
> > system on which glibc is built (possibly a cross GDB), or on the system on
> > which glibc runs.
> 
> What's the point on cross-testing the printers anyway? They're
> platform-independent.

The point is that native and cross builds should produce the same results 
and run the same tests.
Carlos O'Donell Feb. 11, 2016, 4:41 p.m. UTC | #15
On 02/10/2016 08:01 AM, Joseph Myers wrote:
> On Wed, 10 Feb 2016, Martin Galvan wrote:
> 
>> On Tue, Feb 9, 2016 at 2:48 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> 1. Work out where you want to run GDB when testing this feature - on the
>>> system on which glibc is built (possibly a cross GDB), or on the system on
>>> which glibc runs.
>>
>> What's the point on cross-testing the printers anyway? They're
>> platform-independent.
> 
> The point is that native and cross builds should produce the same results 
> and run the same tests.

Or you could run the test on both and compare results looking for
cross-debugging problems that you might see only in more advanced
remote debug configurations.

A native build could elide the target test.

Cheers,
Carlos.
Martin Galvan March 1, 2016, 6:24 p.m. UTC | #16
As you wish. I may have a couple more questions regarding the
cross-testing stuff as I dwell on.

However, before I continue: a couple e-mails ago I mentioned the test
scripts will be based on PExpect (which has a GPL-compatible license).
I want to confirm again this is acceptable before I keep working on
this. If necessary I could add some code to check if PExpect is
present before trying to run the tests.
Carlos O'Donell March 1, 2016, 6:30 p.m. UTC | #17
On 03/01/2016 01:24 PM, Martin Galvan wrote:
> As you wish. I may have a couple more questions regarding the
> cross-testing stuff as I dwell on.
> 
> However, before I continue: a couple e-mails ago I mentioned the test
> scripts will be based on PExpect (which has a GPL-compatible license).
> I want to confirm again this is acceptable before I keep working on
> this. If necessary I could add some code to check if PExpect is
> present before trying to run the tests.

I don't see any problem with using or requiring that PExpect be installed
on the development system to run these tests. It should be possible to
mark the tests as UNSUPPORTED if pexpect is not present (which can be
done dynamically by exiting with the reserved error code 77 IIRC).

Cheers,
Carlos.
Martin Galvan March 1, 2016, 6:33 p.m. UTC | #18
On Tue, Mar 1, 2016 at 3:30 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> I don't see any problem with using or requiring that PExpect be installed
> on the development system to run these tests. It should be possible to
> mark the tests as UNSUPPORTED if pexpect is not present (which can be
> done dynamically by exiting with the reserved error code 77 IIRC).

Will do, thanks!
Martin Galvan March 9, 2016, 8:01 p.m. UTC | #19
Hi again everyone,

As requested, I've written a series of test cases for the pretty
printers. Each lock type (mutex, condvar and rwlock) has two test
programs, one for itself and other for its related 'attributes'
object. Each test program in turn has a PExpect-based Python script
that drives gdb and compares its output to the expected printer's. I'm
attaching the test cases so you guys can start reviewing them; in the
meanwhile I'll dwell further into the build system stuff.

A few things to consider while reviewing this:

* I use the 'GCC optimize' pargma to disable compiler optimizations.
This is necessary because I rely on the C code structure being
preserved when stepping through the programs. Things like aggressive
instruction reordering or optimizing variables out will complicate the
test scripts beyond my reach.
* Each call to the pthread_* functions should be kept on a separate
line. This is done so I can easily advance through the programs using
the gdb 'next' command.
* If you want to run the unit tests, I've included a small Makefile
that will compile each program and run its corresponding script. You
should have my printers (and its constants file) set to be loaded when
gdb starts, e.g. by adding something like the following to .gdbinit:

python
import sys
sys.path.insert(0, '/home/martin')
end

source /home/martin/nptl-printers.py

I've also attached the newest version of the pretty printers and their
constants. The last version I sent defined a Printer class which works
exactly as gdb.RegexpCollectionPrettyPrinter except it could read
through typedefs. I needed this because of a bug in the Python API
which is fixed in the latest gdb. I also corrected a couple of
mistakes and added the corresponding constants for the condvar's clock
ID attribute.
Martin Galvan March 10, 2016, 8:39 p.m. UTC | #20
After dwelling a bit more into the test system, I think I'm beginning
to understand how the test-wrapper variables work. Correct me if I'm
wrong but, except for NaCl, most (if not all) the tests should work
with cross-test-ssh.sh. If that's the case, I think the simplest way
to do this is to use the gdb that's in the system where the
cross-glibc runs, and use cross-test-ssh.sh to run my test scripts. If
required I could check whether Python and gdb actually exist in the
target, failing with UNSUPPORTED if they don't.

Assuming I'm correct, in that case I shouldn't need to define any
additional variables. test-wrapper should be enough.

I'm about to run a cross make check using cross-test-ssh.sh on a
Beaglebone Black running Debian; hopefully I can later use it to
cross-tests my printers.
Carlos O'Donell March 11, 2016, 4:24 a.m. UTC | #21
On 03/10/2016 03:39 PM, Martin Galvan wrote:
> After dwelling a bit more into the test system, I think I'm beginning
> to understand how the test-wrapper variables work. Correct me if I'm
> wrong but, except for NaCl, most (if not all) the tests should work
> with cross-test-ssh.sh. If that's the case, I think the simplest way
> to do this is to use the gdb that's in the system where the
> cross-glibc runs, and use cross-test-ssh.sh to run my test scripts. If
> required I could check whether Python and gdb actually exist in the
> target, failing with UNSUPPORTED if they don't.

Correct.

You can absolutely do that, return exit code 77 to indicate the test
was UNSUPPORTED.

The alternative is to run the test with a host cross-gdb that talks
to a gdbserver (which you'd have to start as part of the test). It's
certainly easier to just run everything on the target, and that's fine.
 
> Assuming I'm correct, in that case I shouldn't need to define any
> additional variables. test-wrapper should be enough.

Right.

> I'm about to run a cross make check using cross-test-ssh.sh on a
> Beaglebone Black running Debian; hopefully I can later use it to
> cross-tests my printers.

Sounds good.
Carlos O'Donell March 11, 2016, 4:30 a.m. UTC | #22
On 03/09/2016 03:01 PM, Martin Galvan wrote:
> As requested, I've written a series of test cases for the pretty
> printers. Each lock type (mutex, condvar and rwlock) has two test
> programs, one for itself and other for its related 'attributes'
> object. Each test program in turn has a PExpect-based Python script
> that drives gdb and compares its output to the expected printer's. I'm
> attaching the test cases so you guys can start reviewing them; in the
> meanwhile I'll dwell further into the build system stuff.

This is really awesome.

From a high level I think the testing is coming along very nicely.

Things I really like:

* PExpect looks very cool. It looks like you can just drive the
  test easily by hand if you detect it's failing which is an
  important part of re-running the test with local changes
  while debugging.

* I like one *.c and one *.py file tied together for testing
  purposes. It makes it easy to see how they work together.
  We should keep this 1:1 relationship as much as we can.
  I really like how easy glibc's tests are to hack on.

One things I'd like to see:

* Test timeouts? What if the tests get stuck?
  - I don't see how you control the timeouts or time them to
    the existing TIMEOUTFACTOR or define default timeouts.


> A few things to consider while reviewing this:
> 
> * I use the 'GCC optimize' pargma to disable compiler optimizations.
> This is necessary because I rely on the C code structure being
> preserved when stepping through the programs. Things like aggressive
> instruction reordering or optimizing variables out will complicate the
> test scripts beyond my reach.

That's fine.

> * Each call to the pthread_* functions should be kept on a separate
> line. This is done so I can easily advance through the programs using
> the gdb 'next' command.

OK.

> * If you want to run the unit tests, I've included a small Makefile
> that will compile each program and run its corresponding script. You
> should have my printers (and its constants file) set to be loaded when
> gdb starts, e.g. by adding something like the following to .gdbinit:

OK.

> python
> import sys
> sys.path.insert(0, '/home/martin')
> end
> 
> source /home/martin/nptl-printers.py
> 
> I've also attached the newest version of the pretty printers and their
> constants. The last version I sent defined a Printer class which works
> exactly as gdb.RegexpCollectionPrettyPrinter except it could read
> through typedefs. I needed this because of a bug in the Python API
> which is fixed in the latest gdb. I also corrected a couple of
> mistakes and added the corresponding constants for the condvar's clock
> ID attribute.

Awesome. I haven't tried these yet, but I looked through the tests and
they all look pretty OK. I'm not going to nit pick the details right now,
but I think you're getting close to ready for a detailed review.

I think you need to answer the test timeout question though since it
becomes very important, particularly if your model in the PyExpect
gets out of sync with the test program.
Martin Galvan March 11, 2016, 2:30 p.m. UTC | #23
On Fri, Mar 11, 2016 at 1:30 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> This is really awesome.
>
> From a high level I think the testing is coming along very nicely.

Thanks a lot! I'm glad to hear that.

> One things I'd like to see:
>
> * Test timeouts? What if the tests get stuck?
>   - I don't see how you control the timeouts or time them to
>     the existing TIMEOUTFACTOR or define default timeouts.

pexpect.spawn takes a 'timeout' parameter, which tells it how long it
should wait for the target to output a matching response after each
call to expect(). If the expect() call times out, an exception is
raised and the test exits with code 1.

I arbitrarily set that timeout to 1 second, but I can change that. Is
TIMEOUTFACTOR an environment variable you can set when running make
check?
Carlos O'Donell March 14, 2016, 6:14 p.m. UTC | #24
On 03/11/2016 09:30 AM, Martin Galvan wrote:
> On Fri, Mar 11, 2016 at 1:30 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> This is really awesome.
>>
>> From a high level I think the testing is coming along very nicely.
> 
> Thanks a lot! I'm glad to hear that.
> 
>> One things I'd like to see:
>>
>> * Test timeouts? What if the tests get stuck?
>>   - I don't see how you control the timeouts or time them to
>>     the existing TIMEOUTFACTOR or define default timeouts.
> 
> pexpect.spawn takes a 'timeout' parameter, which tells it how long it
> should wait for the target to output a matching response after each
> call to expect(). If the expect() call times out, an exception is
> raised and the test exits with code 1.

Thanks. I didn't catch that. You need to multiply that by TIMEOUTFACTOR.
 
> I arbitrarily set that timeout to 1 second, but I can change that. Is
> TIMEOUTFACTOR an environment variable you can set when running make
> check?

Yes. It is the multiple by which all test timeouts are scaled.
Martin Galvan March 18, 2016, 3:53 p.m. UTC | #25
Hi again everyone,

Just wanted to report my status on this. I integrated the printer unit
tests to the NPTL Makefile, which means we can now run the tests by
doing 'make check'. I successfully tested the printers on a Beaglebone
Black using cross-test-ssh.sh as test-wrapper. If we're in a cross
build and don't specify a test-wrapper, it'll simply compile the test
programs but mark the tests themselves as UNRESOLVED.

I had to drop the 'GCC optimize' pragma because, at least on ARM, I
saw there were still a few optimizations being applied when compiling
with -O2 (at least the DWARF was different). I don't know if this is a
gcc bug; in any case I manage to work around this by setting the
CFLAGS-* variables for each test program in the Makefile.

I've also set the PExpect timeout to TIMEOUTFACTOR; if it's not
defined it'll default to 1 second.

I'm currently working on creating a separate subdirectory for the
pretty printers as Siddhesh said. I'll be placing both the printers
and their unit tests there, while the .pysym constants will remain in
the nptl subdir. If everything goes right I should be sending the
(hopefully final) patch in the following days.
Martin Galvan March 21, 2016, 3:04 p.m. UTC | #26
Hi again everyone,

I'm having some trouble when running 'make check' after placing the
pretty printers in their own directory. The Makefile looks like this:

subdir := pretty-printers

include ../Makeconfig

PYTHON := python

tests-pretty-printers := test-mutex-attributes test-mutex-printer \
             test-condvar-attributes test-condvar-printer \
             test-rwlock-attributes test-rwlock-printer

test-srcs := $(tests-pretty-printers)

CFLAGS-test-mutex-attributes.c := -O0 -ggdb3 -pthread
CFLAGS-test-mutex-printer.c := -O0 -ggdb3 -pthread
CFLAGS-test-condvar-attributes.c := -O0 -ggdb3 -pthread
CFLAGS-test-condvar-printer.c := -O0 -ggdb3 -pthread
CFLAGS-test-rwlock-attributes.c := -O0 -ggdb3 -pthread
CFLAGS-test-rwlock-printer.c := -O0 -ggdb3 -pthread

tests-pretty-printers-dest := $(addprefix $(objpfx), $(tests-pretty-printers))
tests-pretty-printers-pp := $(addsuffix -pp, $(tests-pretty-printers-dest))

ifeq ($(run-built-tests), yes)
tests-special += $(tests-pretty-printers-pp)
endif

include ../Rules

$(tests-pretty-printers-pp): $(objpfx)%-pp: $(objpfx)%  %.py test_common.py
    $(test-wrapper-env) $(PYTHON) $*.py $*.c $(objpfx)$*; \
    $(evaluate-test)

I added the 'pretty-printers' subdir to the 'all-subdirs' variable in
Makeconfig. Whenever I run 'make check', however, I get the following
error:

make[2]: Entering directory `/home/martin/glibc/source/pretty-printers'
gcc test-mutex-attributes.c -c -std=gnu11 -fgnu89-inline
-fno-stack-protector -O2 -Wall -Werror -Wundef -Wwrite-strings
-fmerge-all-constants -frounding-math -g -Wstrict-prototypes
-Wold-style-definition   -O0 -ggdb3 -pthread     -U_FORTIFY_SOURCE
-I../include -I/home/martin/glibc/build/pretty-printers
-I/home/martin/glibc/build  -I../sysdeps/unix/sysv/linux/x86_64/64
-I../sysdeps/unix/sysv/linux/x86_64  -I../sysdeps/unix/sysv/linux/x86
-I../sysdeps/unix/sysv/linux/wordsize-64  -I../sysdeps/x86_64/nptl
-I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux
-I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu
-I../sysdeps/unix/inet  -I../sysdeps/unix/sysv
-I../sysdeps/unix/x86_64  -I../sysdeps/unix  -I../sysdeps/posix
-I../sysdeps/x86_64/64  -I../sysdeps/x86_64/fpu/multiarch
-I../sysdeps/x86_64/fpu  -I../sysdeps/x86/fpu/include
-I../sysdeps/x86/fpu  -I../sysdeps/x86_64/multiarch
-I../sysdeps/x86_64  -I../sysdeps/x86  -I../sysdeps/ieee754/ldbl-96
-I../sysdeps/ieee754/dbl-64/wordsize-64  -I../sysdeps/ieee754/dbl-64
-I../sysdeps/ieee754/flt-32  -I../sysdeps/wordsize-64
-I../sysdeps/ieee754  -I../sysdeps/generic  -I.. -I../libio -I.
-D_LIBC_REENTRANT -include /home/martin/glibc/build/libc-modules.h
-DMODULE_NAME=nonlib -include ../include/libc-symbols.h       -o
/home/martin/glibc/build/pretty-printers/test-mutex-attributes.o -MD
-MP -MF /home/martin/glibc/build/pretty-printers/test-mutex-attributes.o.dt
-MT /home/martin/glibc/build/pretty-printers/test-mutex-attributes.o
In file included from ./../include/libc-symbols.h:58:0,
                 from <command-line>:0:
/home/martin/glibc/build/config.h:4:3: error: #error "glibc cannot be
compiled without optimization"
 # error "glibc cannot be compiled without optimization"
   ^

Removing the "-O0" from CFLAGS-* in turn gives me the following error
when linking:

gcc -nostdlib -nostartfiles -o
/home/martin/glibc/build/pretty-printers/test-mutex-attributes
-Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both
/home/martin/glibc/build/csu/crt1.o
/home/martin/glibc/build/csu/crti.o `gcc
--print-file-name=crtbegin.o`
/home/martin/glibc/build/pretty-printers/test-mutex-attributes.o
-Wl,-dynamic-linker=/home/martin/glibc/install/lib/ld-linux-x86-64.so.2
-Wl,-rpath-link=/home/martin/glibc/build:/home/martin/glibc/build/math:/home/martin/glibc/build/elf:/home/martin/glibc/build/dlfcn:/home/martin/glibc/build/nss:/home/martin/glibc/build/nis:/home/martin/glibc/build/rt:/home/martin/glibc/build/resolv:/home/martin/glibc/build/crypt:/home/martin/glibc/build/mathvec:/home/martin/glibc/build/nptl
/home/martin/glibc/build/libc.so.6
/home/martin/glibc/build/libc_nonshared.a -Wl,--as-needed
/home/martin/glibc/build/elf/ld.so -Wl,--no-as-needed -lgcc
-Wl,--as-needed -lgcc_s  -Wl,--no-as-needed `gcc
--print-file-name=crtend.o` /home/martin/glibc/build/csu/crtn.o
/home/martin/glibc/build/pretty-printers/test-mutex-attributes.o: In
function `main':
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:42:
undefined reference to `pthread_mutexattr_init'
/home/martin/glibc/build/pretty-printers/test-mutex-attributes.o: In
function `test_settype':
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:73:
undefined reference to `pthread_mutexattr_settype'
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:75:
undefined reference to `pthread_mutexattr_settype'
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:77:
undefined reference to `pthread_mutexattr_settype'
/home/martin/glibc/build/pretty-printers/test-mutex-attributes.o: In
function `test_setrobust':
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:90:
undefined reference to `pthread_mutexattr_setrobust'
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:92:
undefined reference to `pthread_mutexattr_setrobust'
/home/martin/glibc/build/pretty-printers/test-mutex-attributes.o: In
function `test_setpshared':
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:105:
undefined reference to `pthread_mutexattr_setpshared'
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:107:
undefined reference to `pthread_mutexattr_setpshared'
/home/martin/glibc/build/pretty-printers/test-mutex-attributes.o: In
function `test_setprotocol':
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:122:
undefined reference to `pthread_mutexattr_setprotocol'
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:124:
undefined reference to `pthread_mutexattr_setprotocol'
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:126:
undefined reference to `pthread_mutex_setprioceiling'
/home/martin/glibc/source/pretty-printers/test-mutex-attributes.c:127:
undefined reference to `pthread_mutexattr_setprotocol'
collect2: error: ld returned 1 exit status

I know these two issues are probably unrelated to each other. However,
the weird thing here is that I had that exact same Makefile lines in
the NPTL Makefile, and there it always worked. Can anyone tell me what
exactly should I write in the new Makefile to make this work?

The alternative is to keep the printers in the NPTL subdir (which is
the only place they'll be used in anyway, at least for now), and solve
these issues in a future patch.
Martin Galvan March 21, 2016, 3:58 p.m. UTC | #27
I went a bit deeper and I think the issue is that I'm adding the test
programs to 'test-srcs', which compiles them including libc-symbols.h.
I added them to test-srcs to make sure that even if we don't run the
test scripts, the test programs would be compiled.

The workaround would be adding a new target which compiled the test
programs by hand, like this:

$(tests-pretty-printers-out): $(objpfx)%.out: %.c
    $(CC) $(CFLAGS) -o $@ $*.c

That way, we could make tests-pretty-printers-out be a prerequisite of
the test target, which would run the python scripts only if requested
by run-built-tests.

I'm not sure if this is the cleanest way to do it, tough.
Additionally, I'd still need to know how to link against libpthread.
I'd appreciate it if anyone could give me a hand on how to write the
Makefile.
Andreas Schwab March 21, 2016, 4:02 p.m. UTC | #28
Martin Galvan <martin.galvan@tallertechnologies.com> writes:

> I know these two issues are probably unrelated to each other. However,
> the weird thing here is that I had that exact same Makefile lines in
> the NPTL Makefile, and there it always worked. Can anyone tell me what
> exactly should I write in the new Makefile to make this work?

nptl/Makefile automatically links all tests against libpthread.  You
have to copy that into the new makefile.

Andreas.
Martin Galvan March 21, 2016, 7:04 p.m. UTC | #29
On Mon, Mar 21, 2016 at 1:02 PM, Andreas Schwab <schwab@suse.de> wrote:
> nptl/Makefile automatically links all tests against libpthread.  You
> have to copy that into the new makefile.

I know it does, the question is *how* it does it.

From what I saw, all the nptl Makefile does is set the libpthread
binaries as prerequisites of its tests. Is there some magic somewhere
that uses this to link to libpthread? If so, how does it work, and
what exactly should I write?

Also, what about static linking? Does the 'build-shared' variable tell
how to link against libpthread in all the Makefiles?
Martin Galvan March 21, 2016, 10:28 p.m. UTC | #30
On Mon, Mar 21, 2016 at 4:04 PM, Martin Galvan
<martin.galvan@tallertechnologies.com> wrote:
> I know it does, the question is *how* it does it.
>
> From what I saw, all the nptl Makefile does is set the libpthread
> binaries as prerequisites of its tests. Is there some magic somewhere
> that uses this to link to libpthread? If so, how does it work, and
> what exactly should I write?
>
> Also, what about static linking? Does the 'build-shared' variable tell
> how to link against libpthread in all the Makefiles?

After investigating some more I think I know how this works: the
'Rules' Makefile has a rule for binaries-shared-tests that calls
$(+link-tests), which in turn invokes gcc passing it a filtered $^. If
we set the libpthread as prerequisites of the tests in our Makefile,
$^ will also have them due to Make's feature of multiple rules for the same
target.

The question still remains, though, of how should I handle static
linking against libpthread as well as the -O0 issue.
Andreas Schwab March 22, 2016, 8:30 a.m. UTC | #31
Martin Galvan <martin.galvan@tallertechnologies.com> writes:

> The question still remains, though, of how should I handle static
> linking against libpthread

git grep is your friend.

$ git grep static-thread-library
ChangeLog.8:    and static-thread-library if a thread library is available.  Don't
ChangeLog.8:    static-thread-library in dependencies.
conform/Makefile:                    $(common-objpfx)rt/librt.a $(static-thread-library)
intl/Makefile:$(addprefix $(objpfx),$(multithread-test-srcs)): $(static-thread-library)
nscd/Makefile:$(objpfx)nscd: $(static-thread-library) $(common-objpfx)nis/libnsl.a
rt/Makefile:$(addprefix $(objpfx),$(tests)): $(objpfx)librt.a $(static-thread-library)
sysdeps/nptl/Makeconfig:static-thread-library = $(common-objpfx)nptl/libpthread.a
sysdeps/pthread/Makefile:$(objpfx)tst-timer: $(objpfx)librt.a $(static-thread-library)

Andreas.
diff mbox

Patch

diff --git a/Makerules b/Makerules
index 1329f73..3f84d9f 100644
--- a/Makerules
+++ b/Makerules
@@ -190,6 +190,33 @@  sed-remove-dotdot := -e 's@  *\([^ 	\/$$][^ 	\]*\)@ $$(..)\1@g' \
 		     -e 's@^\([^ 	\/$$][^ 	\]*\)@$$(..)\1@g'
 endif

+ifdef gen-py-const-headers
+# This is a hack we use to generate .py files with constants for Python
+# pretty-printers. It works the same way as gen-as-const. See gen-py-const
+# for details on how the awk | gcc mechanism works.
+$(common-objpfx)%.py: $(..)scripts/gen-py-const.awk %.pysym $(common-before-compile)
+	$(AWK) -f $< $(filter %.pysym,$^) \
+		| $(CC) -S -o $(addsuffix .tmp,$@) $(CFLAGS) $(CPPFLAGS) -x c -
+	echo '# GENERATED FILE, DO NOT EDIT\n' > $@
+	echo '# Constant definitions for the NPTL pretty printers.' >> $@
+	echo '# See gen-py-const.awk for details.\n' >> $@
+
+# Replace "@name@SOME_NAME@value@SOME_VALUE@" strings from the output of
+# gen-py-const.awk with "SOME_NAME = SOME_VALUE" strings.
+# The '-n' option, combined with the '/p' command, makes sed output only the
+# modified lines instead of the whole input file. The output is redirected
+# to a .py file; we'll import it from printers.py to read the constants
+# generated by gen-py-const.awk.
+# The regex has two capturing groups, for SOME_NAME and SOME_VALUE
+# respectively. Notice SOME_VALUE will start with a '$'; Make requires that
+# we write '$' twice.
+	sed -n -r 's/^.*@name@([^@]+)@value@\$$([[:xdigit:]Xx-]+)@.*/\1 = \2/p' \
+		$(addsuffix .tmp,$@) >> $@
+	rm -f $(addsuffix .tmp,$@)
+
+before-compile += $(patsubst %.pysym,%.py,$(common-objpfx)$(gen-py-const-headers))
+generated += $(patsubst %.pysym,%.py,$(common-objpfx)$(gen-py-const-headers))
+endif  # gen-py-const-headers

 ifdef gen-as-const-headers
 # Generating headers for assembly constants.
diff --git a/nptl/Makefile b/nptl/Makefile
index 9ad8793..4871310 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -305,6 +305,7 @@  gen-as-const-headers = pthread-errnos.sym \
 		       lowlevelbarrier.sym unwindbuf.sym \
 		       lowlevelrobustlock.sym pthread-pi-defines.sym

+gen-py-const-headers = nptl_lock_constants.pysym

 LDFLAGS-pthread.so = -Wl,--enable-new-dtags,-z,nodelete,-z,initfirst

diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py
new file mode 100644
index 0000000..ce96fa9
--- /dev/null
+++ b/nptl/nptl-printers.py
@@ -0,0 +1,629 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+# Contributed by Martin Galvan <martin.galvan@tallertechnologies.com>
+#
+# 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/>.
+
+"""Pretty printers for the NTPL lock types.
+
+This file contains the gdb pretty printers for the following types:
+
+    * pthread_mutex_t
+    * pthread_mutexattr_t
+    * pthread_cond_t
+    * pthread_condattr_t
+    * pthread_rwlock_t
+    * pthread_rwlockattr_t
+
+You can check which printers are registered and enabled by issuing the
+'info pretty-printer' gdb command.  Printers should trigger automatically when
+trying to print a variable of one of the types mentioned above.
+"""
+
+from __future__ import print_function
+import itertools
+import re
+import sys
+
+import gdb
+from nptl_lock_constants import *
+
+MUTEX_TYPES = {
+    PTHREAD_MUTEX_NORMAL: ('Type', 'Normal'),
+    PTHREAD_MUTEX_RECURSIVE: ('Type', 'Recursive'),
+    PTHREAD_MUTEX_ERRORCHECK: ('Type', 'Error check'),
+    PTHREAD_MUTEX_ADAPTIVE_NP: ('Type', 'Adaptive')
+}
+
+class MutexPrinter(object):
+    """Pretty printer for pthread_mutex_t."""
+
+    def __init__(self, mutex):
+        """Initialize the printer's internal data structures.
+
+        Args:
+            mutex: A gdb.value representing a pthread_mutex_t.
+        """
+
+        data = mutex['__data']
+        self.lock = data['__lock']
+        self.count = data['__count']
+        self.owner = data['__owner']
+        self.kind = data['__kind']
+        self.values = []
+        self.read_values()
+
+    def to_string(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_mutex_t.
+        """
+
+        return 'pthread_mutex_t'
+
+    def children(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_mutex_t.
+        """
+
+        return itertools.chain(self.values)
+
+    def read_values(self):
+        """Read the mutex's info and store it in self.values.
+
+        The data contained in self.values will be returned by the Iterator
+        created in self.children.
+        """
+
+        self.read_type()
+        self.read_status()
+        self.read_attributes()
+        self.read_misc_info()
+
+    def read_type(self):
+        """Read the mutex's type."""
+
+        mutex_type = self.kind & PTHREAD_MUTEX_KIND_MASK
+
+        # mutex_type must be casted to int because it's a gdb.Value
+        self.values.append(MUTEX_TYPES[int(mutex_type)])
+
+    def read_status(self):
+        """Read the mutex's status.
+
+        For architectures which support lock elision, this method reads
+        whether the mutex is actually locked (i.e. it may show it as unlocked
+        even after calling pthread_mutex_lock).
+        """
+
+        if self.kind == PTHREAD_MUTEX_DESTROYED:
+            self.values.append(('Status', 'Destroyed'))
+        elif self.kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP:
+            self.read_status_robust()
+        else:
+            self.read_status_no_robust()
+
+    def read_status_robust(self):
+        """Read the status of a robust mutex.
+
+        In glibc robust mutexes are implemented in a very different way than
+        non-robust ones.  This method reads their locking status,
+        whether it may have waiters, their registered owner (if any),
+        whether the owner is alive or not, and the status of the state
+        they're protecting.
+        """
+
+        if self.lock == PTHREAD_MUTEX_UNLOCKED:
+            self.values.append(('Status', 'Unlocked'))
+        else:
+            if self.lock & FUTEX_WAITERS:
+                self.values.append(('Status', 'Locked, possibly with waiters'))
+            else:
+                self.values.append(('Status', 'Locked, no waiters'))
+
+            if self.lock & FUTEX_OWNER_DIED:
+                self.values.append(('Owner ID', '%d (dead)' % self.owner))
+            else:
+                self.values.append(('Owner ID', self.lock & FUTEX_TID_MASK))
+
+        if self.owner == PTHREAD_MUTEX_INCONSISTENT:
+            self.values.append(('State protected by this mutex',
+                                'Inconsistent'))
+        elif self.owner == PTHREAD_MUTEX_NOTRECOVERABLE:
+            self.values.append(('State protected by this mutex',
+                                'Not recoverable'))
+
+    def read_status_no_robust(self):
+        """Read the status of a non-robust mutex.
+
+        Read info on whether the mutex is locked, if it may have waiters
+        and its owner (if any).
+        """
+
+        if self.lock == PTHREAD_MUTEX_UNLOCKED:
+            self.values.append(('Status', 'Unlocked'))
+        else:
+            if self.kind & PTHREAD_MUTEX_PRIO_INHERIT_NP:
+                waiters = self.lock & FUTEX_WAITERS
+                owner = self.lock & FUTEX_TID_MASK
+            else:
+                # Mutex protocol is PP or none
+                waiters = (self.lock != PTHREAD_MUTEX_LOCKED_NO_WAITERS)
+                owner = self.owner
+
+            if waiters:
+                self.values.append(('Status', 'Locked, possibly with waiters'))
+            else:
+                self.values.append(('Status', 'Locked, no waiters'))
+
+            self.values.append(('Owner ID', owner))
+
+    def read_attributes(self):
+        """Read the mutex's attributes."""
+
+        if self.kind != PTHREAD_MUTEX_DESTROYED:
+            if self.kind & PTHREAD_MUTEX_ROBUST_NORMAL_NP:
+                self.values.append(('Robust', 'Yes'))
+            else:
+                self.values.append(('Robust', 'No'))
+
+            # In glibc, robust mutexes always have their pshared flag set to
+            # 'shared' regardless of what the pshared flag of their
+            # mutexattr was.  Therefore a robust mutex will act as shared
+            # even if it was initialized with a 'private' mutexattr.
+            if self.kind & PTHREAD_MUTEX_PSHARED_BIT:
+                self.values.append(('Shared', 'Yes'))
+            else:
+                self.values.append(('Shared', 'No'))
+
+            if self.kind & PTHREAD_MUTEX_PRIO_INHERIT_NP:
+                self.values.append(('Protocol', 'Priority inherit'))
+            elif self.kind & PTHREAD_MUTEX_PRIO_PROTECT_NP:
+                prio_ceiling = ((self.lock & PTHREAD_MUTEX_PRIO_CEILING_MASK)
+                                >> PTHREAD_MUTEX_PRIO_CEILING_SHIFT)
+
+                self.values.append(('Protocol', 'Priority protect'))
+                self.values.append(('Priority ceiling', prio_ceiling))
+            else:
+                # PTHREAD_PRIO_NONE
+                self.values.append(('Protocol', 'None'))
+
+    def read_misc_info(self):
+        """Read miscellaneous info on the mutex.
+
+        For now this reads the number of times a row a recursive mutex
+        was locked by the same thread.
+        """
+
+        mutex_type = self.kind & PTHREAD_MUTEX_KIND_MASK
+
+        if mutex_type == PTHREAD_MUTEX_RECURSIVE and self.count > 1:
+            self.values.append(('Times locked recursively', self.count))
+
+class MutexAttributesPrinter(object):
+    """Pretty printer for pthread_mutexattr_t.
+
+    In the NPTL this is a type that's always casted to struct pthread_mutexattr
+    which has a single 'mutexkind' field containing the actual attributes.
+    """
+
+    def __init__(self, mutexattr):
+        """Initialize the printer's internal data structures.
+
+        Args:
+            mutexattr: A gdb.value representing a pthread_mutexattr_t.
+        """
+
+        mutexattr_struct = gdb.lookup_type('struct pthread_mutexattr')
+        self.mutexattr = mutexattr.cast(mutexattr_struct)['mutexkind']
+        self.values = []
+        self.read_values()
+
+    def to_string(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_mutexattr_t.
+        """
+
+        return 'pthread_mutexattr_t'
+
+    def children(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_mutexattr_t.
+        """
+
+        return itertools.chain(self.values)
+
+    def read_values(self):
+        """Read the mutexattr's info and store it in self.values.
+
+        The data contained in self.values will be returned by the Iterator
+        created in self.children.
+        """
+
+        mutexattr_type = (self.mutexattr
+                          & ~PTHREAD_MUTEXATTR_FLAG_BITS
+                          & ~PTHREAD_MUTEX_NO_ELISION_NP)
+
+        # mutexattr_type must be casted to int because it's a gdb.Value
+        self.values.append(MUTEX_TYPES[int(mutexattr_type)])
+
+        if self.mutexattr & PTHREAD_MUTEXATTR_FLAG_ROBUST:
+            self.values.append(('Robust', 'Yes'))
+        else:
+            self.values.append(('Robust', 'No'))
+
+        if self.mutexattr & PTHREAD_MUTEXATTR_FLAG_PSHARED:
+            self.values.append(('Shared', 'Yes'))
+        else:
+            self.values.append(('Shared', 'No'))
+
+        protocol = ((self.mutexattr & PTHREAD_MUTEXATTR_PROTOCOL_MASK) >>
+                    PTHREAD_MUTEXATTR_PROTOCOL_SHIFT)
+
+        if protocol == PTHREAD_PRIO_NONE:
+            self.values.append(('Protocol', 'None'))
+        elif protocol == PTHREAD_PRIO_INHERIT:
+            self.values.append(('Protocol', 'Priority inherit'))
+        elif protocol == PTHREAD_PRIO_PROTECT:
+            self.values.append(('Protocol', 'Priority protect'))
+
+class ConditionVariablePrinter(object):
+    """Pretty printer for pthread_cond_t."""
+
+    def __init__(self, cond):
+        """Initialize the printer's internal data structures.
+
+        Args:
+            cond: A gdb.value representing a pthread_cond_t.
+        """
+
+        data = cond['__data']
+        self.total_seq = data['__total_seq']
+        self.mutex = data['__mutex']
+        self.nwaiters = data['__nwaiters']
+        self.values = []
+        self.read_values()
+
+    def to_string(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_cond_t.
+        """
+
+        return 'pthread_cond_t'
+
+    def children(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_cond_t.
+        """
+
+        return itertools.chain(self.values)
+
+    def read_values(self):
+        """Read the condvar's info and store it in self.values.
+
+        The data contained in self.values will be returned by the Iterator
+        created in self.children.
+        """
+
+        self.read_status()
+        self.read_attributes()
+        self.read_mutex_info()
+
+    def read_status(self):
+        """Read the status of the condvar.
+
+        This method reads whether the condvar is destroyed and how many threads
+        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))
+
+    def read_attributes(self):
+        """Read the condvar's attributes."""
+
+        clock_id = self.nwaiters & ((1 << COND_NWAITERS_SHIFT) - 1)
+        shared = (self.mutex == PTHREAD_COND_SHARED)
+
+        if shared:
+            self.values.append(('Shared', 'Yes'))
+        else:
+            self.values.append(('Shared', 'No'))
+
+        self.values.append(('Clock ID', clock_id))
+
+    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 value of ~0l instead.
+        """
+
+        if self.mutex and self.mutex != PTHREAD_COND_SHARED:
+            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.
+
+    In the NPTL this is a type that's
+    always casted to struct pthread_condattr, which has a single 'value' field
+    containing the actual attributes.
+    """
+
+    def __init__(self, condattr):
+        """Initialize the printer's internal data structures.
+
+        Args:
+            condattr: A gdb.value representing a pthread_condattr_t.
+        """
+
+        condattr_struct = gdb.lookup_type('struct pthread_condattr')
+        self.condattr = condattr.cast(condattr_struct)['value']
+        self.values = []
+        self.read_values()
+
+    def to_string(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_condattr_t.
+        """
+
+        return 'pthread_condattr_t'
+
+    def children(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_condattr_t.
+        """
+
+        return itertools.chain(self.values)
+
+    def read_values(self):
+        """Read the condattr's info and store it in self.values.
+
+        The data contained in self.values will be returned by the Iterator
+        created in self.children.
+        """
+
+        clock_id = self.condattr & ((1 << COND_NWAITERS_SHIFT) - 1)
+
+        if self.condattr & 1:
+            self.values.append(('Shared', 'Yes'))
+        else:
+            self.values.append(('Shared', 'No'))
+
+        self.values.append(('Clock ID', clock_id))
+
+class RWLockPrinter(object):
+    """Pretty printer for pthread_rwlock_t."""
+
+    def __init__(self, rwlock):
+        """Initialize the printer's internal data structures.
+
+        Args:
+            rwlock: A gdb.value representing a pthread_rwlock_t.
+        """
+
+        data = rwlock['__data']
+        self.readers = data['__nr_readers']
+        self.queued_readers = data['__nr_readers_queued']
+        self.queued_writers = data['__nr_writers_queued']
+        self.writer_id = data['__writer']
+        self.shared = data['__shared']
+        self.prefers_writers = data['__flags']
+        self.values = []
+        self.read_values()
+
+    def to_string(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_rwlock_t.
+        """
+
+        return 'pthread_rwlock_t'
+
+    def children(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_rwlock_t.
+        """
+
+        return itertools.chain(self.values)
+
+    def read_values(self):
+        """Read the rwlock's info and store it in self.values.
+
+        The data contained in self.values will be returned by the Iterator
+        created in self.children.
+        """
+
+        self.read_status()
+        self.read_attributes()
+
+    def read_status(self):
+        """Read the status of the rwlock."""
+
+        if self.writer_id:
+            self.values.append(('Status', 'Locked (Write)'))
+            self.values.append(('Writer ID', self.writer_id))
+        elif self.readers:
+            self.values.append(('Status', 'Locked (Read)'))
+            self.values.append(('Readers', self.readers))
+        else:
+            self.values.append(('Status', 'Unlocked'))
+
+        self.values.append(('Queued readers', self.queued_readers))
+        self.values.append(('Queued writers', self.queued_writers))
+
+    def read_attributes(self):
+        """Read the attributes of the rwlock."""
+
+        if self.shared:
+            self.values.append(('Shared', 'Yes'))
+        else:
+            self.values.append(('Shared', 'No'))
+
+        if self.prefers_writers:
+            self.values.append(('Prefers', 'Writers'))
+        else:
+            self.values.append(('Prefers', 'Readers'))
+
+class RWLockAttributesPrinter(object):
+    """Pretty printer for pthread_rwlockattr_t.
+
+    In the NPTL this is a type that's always casted to
+    struct pthread_rwlockattr, which has two fields ('lockkind' and 'pshared')
+    containing the actual attributes.
+    """
+
+    def __init__(self, rwlockattr):
+        """Initialize the printer's internal data structures.
+
+        Args:
+            rwlockattr: A gdb.value representing a pthread_rwlockattr_t.
+        """
+
+        rwlockattr_struct = gdb.lookup_type('struct pthread_rwlockattr')
+        self.rwlockattr = rwlockattr.cast(rwlockattr_struct)
+        self.values = []
+        self.read_values()
+
+    def to_string(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_rwlockattr_t.
+        """
+
+        return 'pthread_rwlockattr_t'
+
+    def children(self):
+        """gdb API function.
+
+        This is called from gdb when we try to print a pthread_rwlockattr_t.
+        """
+
+        return itertools.chain(self.values)
+
+    def read_values(self):
+        """Read the rwlockattr's info and store it in self.values.
+
+        The data contained in self.values will be returned by the Iterator
+        created in self.children.
+        """
+
+        rwlock_type = self.rwlockattr['lockkind']
+        shared = self.rwlockattr['pshared']
+
+        if shared == PTHREAD_PROCESS_SHARED:
+            self.values.append(('Shared', 'Yes'))
+        else:
+            # PTHREAD_PROCESS_PRIVATE
+            self.values.append(('Shared', 'No'))
+
+        if rwlock_type == PTHREAD_RWLOCK_PREFER_READER_NP:
+            self.values.append(('Prefers', 'Readers'))
+        elif (rwlock_type == PTHREAD_RWLOCK_PREFER_WRITER_NP or
+              rwlock_type == PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP):
+            self.values.append(('Prefers', 'Writers'))
+
+class Printer(object):
+    """Printer class which conforms to the gdb pretty printing interface."""
+
+    def __init__(self, name):
+        self.name = name
+        self.enabled = True
+        self.subprinters = []
+
+    class Subprinter(object):
+        """A regex-based printer.
+
+        Individual pretty-printers are registered as subprinters of a single
+        Printer instance.
+        """
+
+        def __init__(self, name, regex, callable_obj):
+            """Initialize a pretty-printer.
+
+            Args:
+                name: The name of the printer.
+                regex: A regular expression.  When gdb tries to print a
+                    variable whose type matches the regex, it'll trigger
+                    this printer.
+                callable_obj: A function or callable object that gdb will call
+                    when trying to print some value.  It should return a
+                    pretty-printer.
+            """
+            self.name = name
+            self.regex = re.compile(regex)
+            self.callable = callable_obj
+            self.enabled = True
+
+    def add_subprinter(self, name, regex, callable_obj):
+        """Register a regex-based subprinter."""
+
+        self.subprinters.append(self.Subprinter(name, regex, callable_obj))
+
+    def __call__(self, value):
+        """gdb API function.
+
+        This is called when trying to print an inferior value from gdb.
+        If a registered printer's regex matches the value's type, gdb will use
+        the printer to print the value.
+        """
+
+        type_name = value.type.name
+
+        if type_name:
+            for subprinter in self.subprinters:
+                if subprinter.enabled and subprinter.regex.match(type_name):
+                    return subprinter.callable(value)
+
+        # Return None if we have no type name or if we can't find a subprinter
+        # for the given type.
+        return None
+
+def register(objfile):
+    """Register the pretty printers within the given objfile."""
+
+    printer = Printer('Glibc pthread locks')
+
+    printer.add_subprinter('pthread_mutex_t', '^pthread_mutex_t$',
+                           MutexPrinter)
+    printer.add_subprinter('pthread_mutexattr_t', '^pthread_mutexattr_t$',
+                           MutexAttributesPrinter)
+    printer.add_subprinter('pthread_cond_t', '^pthread_cond_t$',
+                           ConditionVariablePrinter)
+    printer.add_subprinter('pthread_condattr_t', '^pthread_condattr_t$',
+                           ConditionVariableAttributesPrinter)
+    printer.add_subprinter('pthread_rwlock_t', '^pthread_rwlock_t$',
+                           RWLockPrinter)
+    printer.add_subprinter('pthread_rwlockattr_t', '^pthread_rwlockattr_t$',
+                           RWLockAttributesPrinter)
+
+    gdb.printing.register_pretty_printer(objfile, printer)
+
+register(gdb.current_objfile())
diff --git a/nptl/nptl_lock_constants.pysym b/nptl/nptl_lock_constants.pysym
new file mode 100644
index 0000000..c576822
--- /dev/null
+++ b/nptl/nptl_lock_constants.pysym
@@ -0,0 +1,66 @@ 
+#include <pthreadP.h>
+
+-- Mutex types
+PTHREAD_MUTEX_KIND_MASK          PTHREAD_MUTEX_KIND_MASK_NP
+PTHREAD_MUTEX_NORMAL
+PTHREAD_MUTEX_RECURSIVE          PTHREAD_MUTEX_RECURSIVE_NP
+PTHREAD_MUTEX_ERRORCHECK         PTHREAD_MUTEX_ERRORCHECK_NP
+PTHREAD_MUTEX_ADAPTIVE_NP
+
+-- Mutex status
+-- These are hardcoded all over the code; there are no enums/macros for them.
+PTHREAD_MUTEX_DESTROYED         -1
+PTHREAD_MUTEX_UNLOCKED           0
+PTHREAD_MUTEX_LOCKED_NO_WAITERS  1
+
+-- For robust mutexes
+PTHREAD_MUTEX_INCONSISTENT
+PTHREAD_MUTEX_NOTRECOVERABLE
+FUTEX_OWNER_DIED
+
+-- For robust and PI mutexes
+FUTEX_WAITERS
+FUTEX_TID_MASK
+
+-- Mutex attributes
+PTHREAD_MUTEX_ROBUST_NORMAL_NP
+PTHREAD_MUTEX_PRIO_INHERIT_NP
+PTHREAD_MUTEX_PRIO_PROTECT_NP
+PTHREAD_MUTEX_PSHARED_BIT
+PTHREAD_MUTEX_PRIO_CEILING_SHIFT
+PTHREAD_MUTEX_PRIO_CEILING_MASK
+
+-- Mutex attribute flags
+PTHREAD_MUTEXATTR_PROTOCOL_SHIFT
+PTHREAD_MUTEXATTR_PROTOCOL_MASK
+PTHREAD_MUTEXATTR_PRIO_CEILING_MASK
+PTHREAD_MUTEXATTR_FLAG_ROBUST
+PTHREAD_MUTEXATTR_FLAG_PSHARED
+PTHREAD_MUTEXATTR_FLAG_BITS
+PTHREAD_MUTEX_NO_ELISION_NP
+
+-- Priority protocols
+PTHREAD_PRIO_NONE
+PTHREAD_PRIO_INHERIT
+PTHREAD_PRIO_PROTECT
+
+-- These values are hardcoded as well:
+-- Value of __mutex for shared condvars.
+PTHREAD_COND_SHARED             ~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
+
+-- Rwlock attributes
+PTHREAD_RWLOCK_PREFER_READER_NP
+PTHREAD_RWLOCK_PREFER_WRITER_NP
+PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
+
+-- 'Shared' attribute values
+PTHREAD_PROCESS_PRIVATE
+PTHREAD_PROCESS_SHARED
diff --git a/scripts/gen-py-const.awk b/scripts/gen-py-const.awk
new file mode 100644
index 0000000..5fd2b8d
--- /dev/null
+++ b/scripts/gen-py-const.awk
@@ -0,0 +1,116 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+# Contributed by Martin Galvan <martin.galvan@tallertechnologies.com>
+#
+# 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/>.
+
+# This script is a smaller version of the clever gen-asm-const.awk hack used to
+# generate ASM constants from .sym files. We'll use this to generate constants
+# for Python pretty-printers.
+#
+# The input to this script are .pysym files that look like:
+# #C_Preprocessor_Directive...
+# NAME1
+# NAME2 expression...
+#
+# A line giving just a name implies an expression consisting of just that name.
+# Comments start with '--'.
+#
+# The output of this script is a 'dummy' function containing 'asm' declarations
+# for each non-preprocessor line in the .pysym file. The expression values will
+# appear as input operands to the 'asm' declaration. For example, if we have:
+#
+# /* header.h */
+# #define MACRO 42
+#
+# struct S {
+#     char c1;
+#     char c2;
+#     char c3;
+# };
+#
+# enum E {
+#     ZERO,
+#     ONE
+# };
+#
+# /* symbols.pysym */
+# #include <stddef.h>
+# #include "header.h"
+# -- This is a comment
+# MACRO
+# C3_OFFSET offsetof(struct S, c3)
+# E_ONE ONE
+#
+# the output will be:
+#
+# #include <stddef.h>
+# #include "header.h"
+# void dummy(void)
+# {
+#   asm ("@name@MACRO@value@%0@" : : "i" (MACRO));
+#   asm ("@name@C3_OFFSET@value@%0@" : : "i" (offsetof(struct S, c3)));
+#   asm ("@name@E_ONE@value@%0@" : : "i" (ONE));
+# }
+#
+# We'll later feed this output to gcc -S. Since '-S' tells gcc to compile but
+# not assemble, gcc will output something like:
+#
+# dummy:
+# 	...
+# 	@name@MACRO@value@$42@
+# 	@name@C3_OFFSET@value@$2@
+# 	@name@E_ONE@value@$1@
+#
+# Finally, we can process that output to extract the constant values.
+# Notice gcc will prepend a '$' to each value.
+
+# found_symbol indicates whether we found a non-comment, non-preprocessor line.
+BEGIN { found_symbol = 0 }
+
+# C preprocessor directives go straight through.
+/^#/ { print; next; }
+
+# Skip comments.
+/--/ { next; }
+
+# Trim leading whitespace.
+{ sub(/^[[:blank:]]*/, ""); }
+
+# If we found a non-comment, non-preprocessor line, print the 'dummy' function
+# header.
+NF > 0 && !found_symbol {
+    print "void dummy(void)\n{";
+    found_symbol = 1;
+}
+
+# If the line contains just a name, duplicate it so we can use that name
+# as the value of the expression.
+NF == 1 { sub(/^.*$/, "& &"); }
+
+# If a line contains a name and an expression...
+NF > 1 {
+    name = $1;
+
+    # Remove any characters before the second field.
+    sub(/^[^[:blank:]]+[[:blank:]]+/, "");
+
+    # '$0' ends up being everything that appeared after the first field
+    # separator.
+    printf "  asm (\"@name@%s@value@%0@\" : : \"i\" (%s));\n", name, $0;
+}
+
+# Close the 'dummy' function.
+END { if (found_symbol) print "}"; }