[v11] Add pretty printers for the NPTL lock types

  Ok, I smoke-tested this natively on Ubuntu (AMD64) and it works fine. Could you please test it for cross-builds as well?

I'm attaching the patch here; please let me know if a separate thread is required. Thanks!

ChangeLog:

2016-12-12  Martin Galvan  <omgalvan.86@gmail.com>

	* Rules (python-flags, python-invoke): New.
	($(test-printers-out)): Use $(python-flags).
---
 Rules | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--
2.7.4
  

On Mon, 12 Dec 2016, Martin Galvan wrote:

> Ok, I smoke-tested this natively on Ubuntu (AMD64) and it works fine. 
> Could you please test it for cross-builds as well?

I don't have any cross-builds that also support running the 
pretty-printers tests.
  

2016-12-12 20:15 GMT-03:00 Joseph Myers <joseph@codesourcery.com>:
> I don't have any cross-builds that also support running the
> pretty-printers tests.

Well, since it's a trivial change could this go in without
cross-testing? I don't have access to the Beaglebone anymore.
  

On Mon, 12 Dec 2016, Martin Galvan wrote:

> 2016-12-12  Martin Galvan  <omgalvan.86@gmail.com>
> 
> 	* Rules (python-flags, python-invoke): New.
> 	($(test-printers-out)): Use $(python-flags).

OK.
  

2016-12-14 15:20 GMT-03:00 Joseph Myers <joseph@codesourcery.com>:
> OK.

Thanks. Can you commit this, since I don't have write access?

Again, thanks for the patience.
  

2016-12-14 16:46 GMT-03:00 Martin Galvan <omgalvan.86@gmail.com>:
> 2016-12-14 15:20 GMT-03:00 Joseph Myers <joseph@codesourcery.com>:
>> OK.
>
> Thanks. Can you commit this, since I don't have write access?

Forgot to mention: in the ChangeLog it should say
martingalvan@sourceware.org instead of omgalvan.86. I'll be using the
sourceware address from now on for GNU changelogs.

Thanks!
  

On Saturday 17 December 2016 11:39 PM, Martin Galvan wrote:
>> Thanks. Can you commit this, since I don't have write access?
> 
> Forgot to mention: in the ChangeLog it should say
> martingalvan@sourceware.org instead of omgalvan.86. I'll be using the
> sourceware address from now on for GNU changelogs.

I have pushed this for you.

Siddhesh
  

2016-12-17 16:07 GMT-03:00 Siddhesh Poyarekar <siddhesh@sourceware.org>:
> I have pushed this for you.

Thanks a lot! :)
  

On Sat, 2016-12-17 at 16:08 -0300, Martin Galvan wrote:
> 2016-12-17 16:07 GMT-03:00 Siddhesh Poyarekar <siddhesh@sourceware.org>:
> > I have pushed this for you.
> 
> Thanks a lot! :)

Is there any way to specify a different gdb binary during configure time
or such?  I've only seen the hard-coded value in
scripts/test_printers_common.py.

Testing the pretty printers if the build host gdb isn't ready out of the
box is quite annoying.  It would be good if README.pretty-printers could
be improved to cover this.
  

2016-12-22 13:34 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> Is there any way to specify a different gdb binary during configure time
> or such?  I've only seen the hard-coded value in
> scripts/test_printers_common.py.

I didn't touch any configure scripts for this patch. Sid was working
on a patch for detecting the python program at configure time, perhaps
he can do something similar for gdb?

I'm personally focused on the install patch, though I don't think I'll
have it ready for 2.25 as I'm doing all this in what little free time
I have.

> Testing the pretty printers if the build host gdb isn't ready out of the
> box is quite annoying.

How so? IIRC the tests should return UNSUPPORTED when gdb is absent.
Are you seeing something different?

> It would be good if README.pretty-printers could
> be improved to cover this.

The README says:

"The tests run on the glibc host, which is assumed to have both gdb
and PExpect; if any of those is absent the tests will fail with code
77 (UNSUPPORTED)."

What do you suggest? Should it mention that the gdb binary should
actually be called "gdb" and be somewhere within PATH?
  

On Thu, 2016-12-22 at 13:43 -0300, Martin Galvan wrote:
> 2016-12-22 13:34 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> > Testing the pretty printers if the build host gdb isn't ready out of the
> > box is quite annoying.
> 
> How so? IIRC the tests should return UNSUPPORTED when gdb is absent.
> Are you seeing something different?

Yeah, they print UNSUPPORTED, but it's not straightforward to get to a
point where your test setup is sufficient to do more than UNSUPPORTED.

> > It would be good if README.pretty-printers could
> > be improved to cover this.
> 
> The README says:
> 
> "The tests run on the glibc host, which is assumed to have both gdb
> and PExpect; if any of those is absent the tests will fail with code
> 77 (UNSUPPORTED)."
> 
> What do you suggest? Should it mention that the gdb binary should
> actually be called "gdb" and be somewhere within PATH?

I first thought it would pick the system gdb and not the one from $PATH.
This was because I had built a new gdb with --with-python, and I can do
a 'python print "hello"' successfully from the new prompt of the new gdb
I've built.  It seems that the new gdb is used, but the test script
still complaints that the gdb doesn't support python ('gdb must have
python support to test the pretty printers.').  pexpect should be
installed for the python that's used.  What am I doing wrong?
  

2016-12-22 19:50 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> I first thought it would pick the system gdb and not the one from $PATH.

What would be the difference between the "system gdb" and the one from
$PATH? Unless you altered your $PATH to make it point to the newly
built gdb, I don't understand what you mean.

>  It seems that the new gdb is used, but the test script
> still complaints that the gdb doesn't support python ('gdb must have
> python support to test the pretty printers.').  pexpect should be
> installed for the python that's used.  What am I doing wrong?

How did you come to the conclusion that the new gdb is being used?

In any case, I think it would be helpful to see the value of
gdb_python_error before the test script exits.
  

On Thu, 2016-12-22 at 20:39 -0300, Martin Galvan wrote:
> 2016-12-22 19:50 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> > I first thought it would pick the system gdb and not the one from $PATH.
> 
> What would be the difference between the "system gdb" and the one from
> $PATH? Unless you altered your $PATH to make it point to the newly
> built gdb, I don't understand what you mean.

I thought it would not pick up the gdb from PATH.  But it did, and the
problem was elsewhere (see below).

> >  It seems that the new gdb is used, but the test script
> > still complaints that the gdb doesn't support python ('gdb must have
> > python support to test the pretty printers.').  pexpect should be
> > installed for the python that's used.  What am I doing wrong?
> 
> How did you come to the conclusion that the new gdb is being used?

system gdb is too old and fails earlier tests.

> In any case, I think it would be helpful to see the value of
> gdb_python_error before the test script exits.

After *lots* of poking, it seems that something in the pexpect/gdb
combination in my case was adding terminal escape sequences to the gdb
output.  That made the error string look empty when printed, but it was
not a zero-sized string and thus does not evaluate to false.
I do not know what caused this, and whether the pretty printers script
should have filtered that out or pexpect/gdb shouldn't have produced the
escape sequence in the first place.  The combination I used was:
pexpect from pip on system python 2.7.5, gdb 7.8.5 built wiht
--with-python, and I had ncurses-devel-5.9-13.20130511.el7.x86_64
installed (picked up in the gdb build).
Switching to gdb 7.12 does not require ncurses or a termcap library
anymore, and works with the existing pretty printers script.

I suppose raising the minimally required gdb version would be helpful.

It would also be helpful if scripts/test_printers_common.py would not
discard the error when it cannot load the pretty printer modules for
some reason.  I've changed
    for printer_file in printer_files:
        test('source {0}'.format(printer_file))
to the following, but don't know whether that's the proper way to do it:
    for printer_file in printer_files:
        test('source {0}'.format(printer_file), '')

And nptl/test-mutex-printers fails in my setup, BTW.
  

2016-12-23 14:05 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> After *lots* of poking, it seems that something in the pexpect/gdb
> combination in my case was adding terminal escape sequences to the gdb
> output.  That made the error string look empty when printed, but it was
> not a zero-sized string and thus does not evaluate to false.

That's really weird. In any case, if I understood correctly: you were
using the newly built --with-python gdb, and still got an error saying
that gdb doesn't have Python support? If not, what was the actual
error?

> Switching to gdb 7.12 does not require ncurses or a termcap library
> anymore, and works with the existing pretty printers script.
>
> I suppose raising the minimally required gdb version would be helpful.

Alright, I can do that.

> It would also be helpful if scripts/test_printers_common.py would not
> discard the error when it cannot load the pretty printer modules for
> some reason.

Makes sense, will add it in a future patch.

> And nptl/test-mutex-printers fails in my setup, BTW.

How? What does the .out say? Does it fail for 7.12 too?
  

2016-12-23 14:40 GMT-03:00 Martin Galvan <omgalvan.86@gmail.com>:
> That's really weird. In any case, if I understood correctly: you were
> using the newly built --with-python gdb, and still got an error saying
> that gdb doesn't have Python support? If not, what was the actual
> error?

Nevermind, I just understood what you meant. I wonder what was causing
those escape sequences to appear, and if there's a way to disable them
in older gdbs..
  

On Fri, 2016-12-23 at 14:40 -0300, Martin Galvan wrote:
> 2016-12-23 14:05 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> > And nptl/test-mutex-printers fails in my setup, BTW.
> 
> How? What does the .out say? Does it fail for 7.12 too?

Error: Response does not match the expected pattern.
Command: print *mutex
Expected pattern: Owner ID = 30089
Response:  $7 = pthread_mutex_t = {Type = Normal, 
  Status = Locked, possibly with no waiters, Owner ID = 0, Robust = No, 
  Shared = No, Protocol = None}
  

2016-12-23 15:47 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
>   Status = Locked, possibly with no waiters, Owner ID = 0, Robust = No,

That's odd, IIRC locked mutexes always record their owner's ID. I
assume manually printing mutex->__data.__owner at that point shows 0
too, right?
  

2016-12-23 16:37 GMT-03:00 Martin Galvan <omgalvan.86@gmail.com>:
> 2016-12-23 15:47 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
>>   Status = Locked, possibly with no waiters, Owner ID = 0, Robust = No,
>
> That's odd, IIRC locked mutexes always record their owner's ID. I
> assume manually printing mutex->__data.__owner at that point shows 0
> too, right?

I've done some more testing and still can't reproduce the error. I've
tried using gdb 7.12 (sources downloaded from the GNU FTP site) and
Python 2.7.5 on Ubuntu 16.04, x86_64. gdb -configure shows:

This GDB was configured as follows:
   configure --host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu
             --with-auto-load-dir=$debugdir:$datadir/auto-load
             --with-auto-load-safe-path=$debugdir:$datadir/auto-load
             --with-expat
             --with-gdb-datadir=/home/martin/Documents/gdb-7.12/install/share/gdb
(relocatable)
             --with-jit-reader-dir=/home/martin/Documents/gdb-7.12/install/lib/gdb
(relocatable)
             --without-libunwind-ia64
             --without-lzma
             --with-python=/home/martin/Documents/python-2.7.5/install
             --without-guile
             --with-separate-debug-dir=/home/martin/Documents/gdb-7.12/install/lib/debug
(relocatable)
             --without-babeltrace

I set LD_LIBRARY_PATH to the Python 2.7.5 lib install path so that gdb
would pick the correct libpython2.7.so. I can post the output of gdb's
python's sysconfig.get_config_vars(), though it's quite verbose.

In any case, manually running gdb and stepping through the target
until line 80 (after locking the non-robust mutex, which is where I'm
assuming the test fails) still shows mutex->__data.__owner as being
the owner's TID.

Perhaps it would be useful to see what the mutex's member values are
in your case. At one point I thought your mutex might've been showing
all zeroes, but at least __data.__lock must be non-zero for it to show
as locked. Any other info that could help me reproduce the error is
welcome.
  

Guys, are there any news on this? Did the test fail for anyone else?

If not, I'd say we can follow Torvald's suggestion and bump up the
minimum required gdb version to 7.12, and see if that fixes the escape
codes issue. I don't have write access so somebody else should do it,
though.
  

On Mon, 2 Jan 2017, Martin Galvan wrote:

> Guys, are there any news on this? Did the test fail for anyone else?

I'm seeing the following failures with current sources on x86_64.  This is 
with system GDB on Ubuntu 16.04 (but a locally built compiler, configured 
as a cross compiler, for building glibc).  System GDB is:

GNU gdb (Ubuntu 7.11.1-0ubuntu1~16.04) 7.11.1

FAIL: nptl/test-cond-printers
FAIL: nptl/test-condattr-printers
FAIL: nptl/test-mutex-printers
FAIL: nptl/test-mutexattr-printers
FAIL: nptl/test-rwlock-printers
FAIL: nptl/test-rwlockattr-printers

test-cond-printers.out says:

Error: Response does not match the expected pattern.
Command: enable pretty-printer global glibc-pthread-locks
Expected pattern: [1-9][0-9]* of [1-9]+ printers enabled
Response:  0 printers enabled
0 of 1 printers enabled
(gdb) 

and all the others are similar.  GDB is linked with Python 3.5, which is 
also the version found by configure.
  

On Fri, 2016-12-23 at 16:37 -0300, Martin Galvan wrote:
> 2016-12-23 15:47 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> >   Status = Locked, possibly with no waiters, Owner ID = 0, Robust = No,
> 
> That's odd, IIRC locked mutexes always record their owner's ID. I
> assume manually printing mutex->__data.__owner at that point shows 0
> too, right?

This may be related to lock elision.  In a build with lock elision
disabled, I didn't see the failure.  The next build with lock elision
enabled again showed the failure.

Did you test with or without lock elision?
  

2017-01-06 18:58 GMT-03:00 Torvald Riegel <triegel@redhat.com>:
> Did you test with or without lock elision?

Good point! I never explicitly enabled lock elision when running
configure. However, I just ran the tests having set
--enable-lock-elision=yes and it's still working for me (though I
don't know why-- how deterministic is lock elision? I just tested this
on a Haswell machine). However, maybe that's what's causing the owner
ID to be 0 in your case. I'll see about fixing the mutex test to
support lock elision and send a patch for you guys to test.

Thanks!