[PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[markus.t.metzger]

From: Markus Metzger <markus.t.metzger@intel.com>

When querying an inferior's threads in Python in a remote debugging
configuration, only the already known threads are returned.

Update the thread list in infpy_threads () before creating the Python objects.

2012-07-19 Markus Metzger <markus.t.metzger@intel.com>

gdb/python/
	* py-inferior.c (infpy_threads): Call update_thread_list ().

gdb/testsuite/gdb.python/
	* py-threads.c: New file.
	* py-threads.exp: New file.


---
 gdb/python/py-inferior.c                |    5 +++
 gdb/testsuite/gdb.python/py-threads.c   |   51 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-threads.exp |   30 ++++++++++++++++++
 3 files changed, 86 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-threads.c
 create mode 100644 gdb/testsuite/gdb.python/py-threads.exp

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 2b229be..22adf8c 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -300,6 +300,11 @@ infpy_threads (PyObject *self, PyObject *args)
   struct threadlist_entry *entry;
   inferior_object *inf_obj = (inferior_object *) self;
   PyObject *tuple;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    update_thread_list ();
+  GDB_PY_HANDLE_EXCEPTION (except);
 
   INFPY_REQUIRE_VALID (inf_obj);
 
diff --git a/gdb/testsuite/gdb.python/py-threads.c b/gdb/testsuite/gdb.python/py-threads.c
new file mode 100644
index 0000000..bb08979
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-threads.c
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program 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 General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see  <http://www.gnu.org/licenses/>.
+*/
+
+#include <pthread.h>
+
+#define NUMTH 8
+
+static void *
+thread (void *param)
+{
+  for (;;)
+    ;
+  return param;
+}
+
+static void
+check (void)
+{
+}
+
+extern int
+main (void)
+{
+  pthread_t threads[NUMTH];
+  int i;
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_create (&threads[i], NULL, thread, NULL);
+
+  check ();
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_join (threads[i], NULL);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-threads.exp b/gdb/testsuite/gdb.python/py-threads.exp
new file mode 100644
index 0000000..ab2bccb
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-threads.exp
@@ -0,0 +1,30 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+clean_restart $testfile
+
+if { [skip_python_tests] } { continue }
+
+if ![runto check] {
+	untested "py-threads"
+	return -1
+}
+
+gdb_test "python print len(gdb.selected_inferior().threads())" "9" "py-threads"
-- 
1.7.1

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Jan Kratochvil]

On Thu, 19 Jul 2012 17:29:17 +0200, markus.t.metzger@intel.com wrote:
> gdb/python/
> 	* py-inferior.c (infpy_threads): Call update_thread_list ().
> 
> gdb/testsuite/gdb.python/
> 	* py-threads.c: New file.
> 	* py-threads.exp: New file.

OK with the issue below but please check it in only after several days, there
is questionable the caching of threads list.


> +static void *
> +thread (void *param)
> +{
> +  for (;;)
> +    ;

(1) Never create needlessly busy-looping testcases.
    Therefore use sleep().

(2) Never create testcases which run forever, always use some reasonable
    timeout (such as 60 seconds).


> +  return param;
> +}
[...]
> +gdb_test "python print len(gdb.selected_inferior().threads())" "9" "py-threads"

Nitpick but better "\r\n9" as otherwise any number x%10==9 would match.



Thanks,
Jan

RE: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Metzger, Markus T]

[-- Attachment #1.1: Type: text/plain, Size: 1368 bytes --]

> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Subject: Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()
> > Date: July 19, 2012 8:40:50 PM GMT+02:00

Thanks for your review!

> > On Thu, 19 Jul 2012 17:29:17 +0200, markus.t.metzger@intel.com wrote:
> >> gdb/python/
> >> 	* py-inferior.c (infpy_threads): Call update_thread_list ().
> >>
> >> gdb/testsuite/gdb.python/
> >> 	* py-threads.c: New file.
> >> 	* py-threads.exp: New file.
> >
> > OK with the issue below but please check it in only after several days, there
> > is questionable the caching of threads list.

I don't have write access. Regarding the caching question, this is being discussed here:
http://sourceware.org/ml/gdb-patches/2012-07/msg00382.html.


> >> +static void *
> >> +thread (void *param)
> >> +{
> >> +  for (;;)
> >> +    ;
> >
> > (1) Never create needlessly busy-looping testcases.
> >    Therefore use sleep().
> >
> > (2) Never create testcases which run forever, always use some reasonable
> >    timeout (such as 60 seconds).

I now use a barrier to synchronize threads. This should satisfy both requirements.


> >> +  return param;
> >> +}
> > [...]
> >> +gdb_test "python print len(gdb.selected_inferior().threads())" "9" "py-threads"
> >
> > Nitpick but better "\r\n9" as otherwise any number x%10==9 would match.

Done. Thanks.

Regards,
Markus.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 350 bytes --]

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer, Christian Lamprechter
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Tom Tromey]

>>>>> "Markus" == Metzger, Markus T <markus.t.metzger@intel.com> writes:

Markus> I don't have write access.

Do you have a copyright assignment on file?  I think the test suite
addition is large enough to require one.  If you don't have one (and
aren't covered by a company assignment -- I can't check this so I rely
on you to know :), let me know off-list and I will get you started.  If
you are covered, just let us know.

thanks,
Tom

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> OK with the issue below but please check it in only after several
Jan> days, there is questionable the caching of threads list.

I think making thread notification smarter, as you mention upthread,
would be good, but isn't needed for this patch.  This patch is an
improvement on its own.

Tom

RE: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Metzger, Markus T]

[-- Attachment #1.1: Type: text/plain, Size: 626 bytes --]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, July 25, 2012 6:13 PM


> Markus> I don't have write access.
> 
> Do you have a copyright assignment on file?  I think the test suite
> addition is large enough to require one.  If you don't have one (and
> aren't covered by a company assignment -- I can't check this so I rely
> on you to know :), let me know off-list and I will get you started.  If
> you are covered, just let us know.

I'm not sure whether it's a company assignment or individual assignments for a handful of Intel employees, but I'm covered.

Regards,
Markus.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 350 bytes --]

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer, Christian Lamprechter
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

[PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[markus.t.metzger]

From: Markus Metzger <markus.t.metzger@intel.com>

When querying an inferior's threads in Python in a remote debugging
configuration, only the already known threads are returned.

Update the thread list in infpy_threads () before creating the Python objects.

2012-07-26 Markus Metzger <markus.t.metzger@intel.com>

	* python/py-inferior.c (infpy_threads): Call update_thread_list ().

testsuite/
	* gdb.python/py-inferior.c (thread): New function.
	(check_threads): New function.
	(test_threads): New function.
	* gdb.python/py-inferior.exp: Added test.
	Replaced runto with continue to breakpoint.


---
 gdb/python/py-inferior.c                 |    5 +++
 gdb/testsuite/gdb.python/py-inferior.c   |   41 ++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-inferior.exp |   15 ++++++++--
 3 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 2b229be..907b73e 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -300,9 +300,14 @@ infpy_threads (PyObject *self, PyObject *args)
   struct threadlist_entry *entry;
   inferior_object *inf_obj = (inferior_object *) self;
   PyObject *tuple;
+  volatile struct gdb_exception except;
 
   INFPY_REQUIRE_VALID (inf_obj);
 
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    update_thread_list ();
+  GDB_PY_HANDLE_EXCEPTION (except);
+
   tuple = PyTuple_New (inf_obj->nthreads);
   if (!tuple)
     return NULL;
diff --git a/gdb/testsuite/gdb.python/py-inferior.c b/gdb/testsuite/gdb.python/py-inferior.c
index 04ec476..3ee9a46 100644
--- a/gdb/testsuite/gdb.python/py-inferior.c
+++ b/gdb/testsuite/gdb.python/py-inferior.c
@@ -2,9 +2,11 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#include <pthread.h>
 
 #define CHUNK_SIZE 16000 /* same as findcmd.c's */
 #define BUF_SIZE (2 * CHUNK_SIZE) /* at least two chunks */
+#define NUMTH 8
 
 int8_t int8_search_buf[100];
 int16_t int16_search_buf[100];
@@ -43,8 +45,47 @@ init_bufs ()
   memset (search_buf, 'x', search_buf_size);
 }
 
+static void *
+thread (void *param)
+{
+  pthread_barrier_t *barrier = (pthread_barrier_t *) param;
+
+  pthread_barrier_wait (barrier);
+
+  return param;
+}
+
+static void
+check_threads (pthread_barrier_t *barrier)
+{
+  pthread_barrier_wait (barrier);
+}
+
+extern int
+test_threads (void)
+{
+  pthread_t threads[NUMTH];
+  pthread_barrier_t barrier;
+  int i;
+
+  pthread_barrier_init (&barrier, NULL, NUMTH + 1);
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_create (&threads[i], NULL, thread, &barrier);
+
+  check_threads (&barrier);
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_join (threads[i], NULL);
+
+  pthread_barrier_destroy (&barrier);
+
+  return 0;
+}
+
 int main (int argc, char *argv[])
 {
+  test_threads ();
   init_bufs ();
 
   return f1 (1, 2);
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index b40a514..15e684a 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -20,7 +20,7 @@ load_lib gdb-python.exp
 
 standard_testfile
 
-if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+if { [gdb_compile_pthreads ${srcdir}/${subdir}/${srcfile} ${binfile} executable {debug}] != "" } {
     return -1
 }
 
@@ -48,8 +48,6 @@ if ![runto_main] then {
     return 0
 }
 
-runto [gdb_get_line_number "Break here."]
-
 # Test basic gdb.Inferior attributes and methods.
 
 gdb_py_test_silent_cmd "python inferiors = gdb.inferiors ()" "get inferiors list" 1
@@ -62,6 +60,17 @@ gdb_test "python print 'result =', i0.pid" " = \[0-9\]+" "test Inferior.pid"
 gdb_test "python print 'result =', i0.was_attached" " = False" "test Inferior.was_attached"
 gdb_test "python print i0.threads ()" "\\(<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>,\\)" "test Inferior.threads"
 
+# Test the number of inferior threads.
+
+gdb_breakpoint check_threads
+gdb_continue_to_breakpoint "cont to check_threads" ".*pthread_barrier_wait.*"
+gdb_test "python print len (i0.threads ())" "\r\n9" "test Inferior.threads 2"
+
+# Proceed to the next test.
+
+gdb_breakpoint [gdb_get_line_number "Break here."]
+gdb_continue_to_breakpoint "cont to Break here." ".*Break here\..*"
+
 # Test memory read and write operations.
 
 gdb_py_test_silent_cmd "python addr = gdb.selected_frame ().read_var ('str')" \
-- 
1.7.1

RE: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Metzger, Markus T]

[-- Attachment #1.1: Type: text/plain, Size: 833 bytes --]

> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Thursday, July 26, 2012 10:42 AM
> To: Metzger, Markus T


> > Thanks. Who would be the approver of the change?
> 
> I believe Tom approved it this way.

Ah. Good.


> BTW instead of those two runto calls it would be cheaper to:
> 
> gdb_breakpoint "check_threads"
> gdb_continue_to_breakpoint "check_threads" ".* pthread_barrier_wait .*"
> 
> and
> 
> gdb_breakpoint [gdb_get_line_number "Break here."]
> gdb_continue_to_breakpoint "Break-here" {.* Break here\. .*}
> 
> (The second parameter to gdb_continue_to_breakpoint is not mandatory.)
> 
> Otherwise you do the whole spawn of gdbserver 2 more times.

Thanks. I'll send an updated patch.

Would be nice to have a library function similar to runto that does this.

Regards,
Markus.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 350 bytes --]

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer, Christian Lamprechter
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Jan Kratochvil]

On Thu, 26 Jul 2012 09:49:44 +0200, Metzger, Markus T wrote:
> > Otherwise looks good to me.
> 
> Thanks. Who would be the approver of the change?

I believe Tom approved it this way.

BTW instead of those two runto calls it would be cheaper to:

gdb_breakpoint "check_threads"
gdb_continue_to_breakpoint "check_threads" ".* pthread_barrier_wait .*"

and

gdb_breakpoint [gdb_get_line_number "Break here."]
gdb_continue_to_breakpoint "Break-here" {.* Break here\. .*}

(The second parameter to gdb_continue_to_breakpoint is not mandatory.)

Otherwise you do the whole spawn of gdbserver 2 more times.


Thanks,
Jan

RE: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Metzger, Markus T]

[-- Attachment #1.1: Type: text/plain, Size: 630 bytes --]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Tom Tromey
> Sent: Wednesday, July 25, 2012 6:15 PM

[...]

> Markus> -if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
> Markus> +if { [gdb_compile_pthreads ${srcdir}/${subdir}/${srcfile} ${binfile} executable {debug}] != "" } {
> 
> prepare_for_testing also calls clean_restart.
> I think your change should add this.

The test already does clean_restart after prepare_for_testing.


> Otherwise looks good to me.

Thanks. Who would be the approver of the change?

Regards,
Markus.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 350 bytes --]

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer, Christian Lamprechter
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Tom Tromey]

>>>>> "Markus" == markus t metzger <markus.t.metzger@intel.com> writes:

Markus> -if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
Markus> +if { [gdb_compile_pthreads ${srcdir}/${subdir}/${srcfile} ${binfile} executable {debug}] != "" } {

prepare_for_testing also calls clean_restart.
I think your change should add this.

Otherwise looks good to me.

Tom

[PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[markus.t.metzger]

From: Markus Metzger <markus.t.metzger@intel.com>

When querying an inferior's threads in Python in a remote debugging
configuration, only the already known threads are returned.

Update the thread list in infpy_threads () before creating the Python objects.

2012-07-25 Markus Metzger <markus.t.metzger@intel.com>

	* python/py-inferior.c (infpy_threads): Call update_thread_list ().

testsuite/
	* gdb.python/py-inferior.c (thread): New function.
	(check_threads): New function.
	(test_threads): New function.
	* gdb.python/py-inferior.exp: Added test.


---
 gdb/python/py-inferior.c                 |    5 +++
 gdb/testsuite/gdb.python/py-inferior.c   |   41 ++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-inferior.exp |   13 +++++++--
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 2b229be..907b73e 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -300,9 +300,14 @@ infpy_threads (PyObject *self, PyObject *args)
   struct threadlist_entry *entry;
   inferior_object *inf_obj = (inferior_object *) self;
   PyObject *tuple;
+  volatile struct gdb_exception except;
 
   INFPY_REQUIRE_VALID (inf_obj);
 
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    update_thread_list ();
+  GDB_PY_HANDLE_EXCEPTION (except);
+
   tuple = PyTuple_New (inf_obj->nthreads);
   if (!tuple)
     return NULL;
diff --git a/gdb/testsuite/gdb.python/py-inferior.c b/gdb/testsuite/gdb.python/py-inferior.c
index 04ec476..3ee9a46 100644
--- a/gdb/testsuite/gdb.python/py-inferior.c
+++ b/gdb/testsuite/gdb.python/py-inferior.c
@@ -2,9 +2,11 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#include <pthread.h>
 
 #define CHUNK_SIZE 16000 /* same as findcmd.c's */
 #define BUF_SIZE (2 * CHUNK_SIZE) /* at least two chunks */
+#define NUMTH 8
 
 int8_t int8_search_buf[100];
 int16_t int16_search_buf[100];
@@ -43,8 +45,47 @@ init_bufs ()
   memset (search_buf, 'x', search_buf_size);
 }
 
+static void *
+thread (void *param)
+{
+  pthread_barrier_t *barrier = (pthread_barrier_t *) param;
+
+  pthread_barrier_wait (barrier);
+
+  return param;
+}
+
+static void
+check_threads (pthread_barrier_t *barrier)
+{
+  pthread_barrier_wait (barrier);
+}
+
+extern int
+test_threads (void)
+{
+  pthread_t threads[NUMTH];
+  pthread_barrier_t barrier;
+  int i;
+
+  pthread_barrier_init (&barrier, NULL, NUMTH + 1);
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_create (&threads[i], NULL, thread, &barrier);
+
+  check_threads (&barrier);
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_join (threads[i], NULL);
+
+  pthread_barrier_destroy (&barrier);
+
+  return 0;
+}
+
 int main (int argc, char *argv[])
 {
+  test_threads ();
   init_bufs ();
 
   return f1 (1, 2);
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index b40a514..b351f3e 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -20,7 +20,7 @@ load_lib gdb-python.exp
 
 standard_testfile
 
-if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+if { [gdb_compile_pthreads ${srcdir}/${subdir}/${srcfile} ${binfile} executable {debug}] != "" } {
     return -1
 }
 
@@ -48,8 +48,6 @@ if ![runto_main] then {
     return 0
 }
 
-runto [gdb_get_line_number "Break here."]
-
 # Test basic gdb.Inferior attributes and methods.
 
 gdb_py_test_silent_cmd "python inferiors = gdb.inferiors ()" "get inferiors list" 1
@@ -62,6 +60,15 @@ gdb_test "python print 'result =', i0.pid" " = \[0-9\]+" "test Inferior.pid"
 gdb_test "python print 'result =', i0.was_attached" " = False" "test Inferior.was_attached"
 gdb_test "python print i0.threads ()" "\\(<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>,\\)" "test Inferior.threads"
 
+# Test the number of inferior threads.
+
+runto check_threads
+gdb_test "python print len (i0.threads ())" "\r\n9" "test Inferior.threads 2"
+
+# Proceed to the next test.
+
+runto [gdb_get_line_number "Break here."]
+
 # Test memory read and write operations.
 
 gdb_py_test_silent_cmd "python addr = gdb.selected_frame ().read_var ('str')" \
-- 
1.7.1

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Phil Muldoon]

On 07/24/2012 09:11 AM, markus.t.metzger@intel.com wrote:
> From: Markus Metzger <markus.t.metzger@intel.com>
> 
> When querying an inferior's threads in Python in a remote debugging
> configuration, only the already known threads are returned.

Thanks.

> Update the thread list in infpy_threads () before creating the Python objects.
> 
> 2012-07-24 Markus Metzger <markus.t.metzger@intel.com>
> 
> gdb/python/
> 	* py-inferior.c (infpy_threads): Call update_thread_list ().>

ChangeLog paths are relative, so in these entries you do not need the
"gdb" section.  Also the path and entry should be appended to be on
the same line. E.g.:

2012-07-24 Markus Metzger <markus.t.metzger@intel.com>
 
    * python/py-inferior.c (infpy_threads): Call update_thread_list ().


> gdb/testsuite/gdb.python/
> 	* py-threads.c: New file.
> 	* py-threads.exp: New file.
> 

The testsuite has its own ChangeLog located in testsuite/, so these
entries need to go there.  Also, the paths are relative too, so no
"gdb/testsuite" in those entries.

> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index 2b229be..22adf8c 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -300,6 +300,11 @@ infpy_threads (PyObject *self, PyObject *args)
>    struct threadlist_entry *entry;
>    inferior_object *inf_obj = (inferior_object *) self;
>    PyObject *tuple;
> +  volatile struct gdb_exception except;
> +
> +  TRY_CATCH (except, RETURN_MASK_ALL)
> +    update_thread_list ();
> +  GDB_PY_HANDLE_EXCEPTION (except);
>  
>    INFPY_REQUIRE_VALID (inf_obj);

If the inferior is no longer around, then updating the thread list is
invalid.  update_thread_list and the associated thread exception code
should be hoisted below the inferior validity check performed in
INFPY_REQUIRE_VALID.

> diff --git a/gdb/testsuite/gdb.python/py-threads.c b/gdb/testsuite/gdb.python/py-threads.c
> new file mode 100644

> diff --git a/gdb/testsuite/gdb.python/py-threads.exp b/gdb/testsuite/gdb.python/py-threads.exp
> new file mode 100644

py-inferior has its own series of test cases in py-inferior.exp, with the test
inferior being py-inferior.c.  I would prefer it if we could keep
these tests as one unit.  Could you try merging the two testcases?

Thanks

Phil

[PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[markus.t.metzger]

From: Markus Metzger <markus.t.metzger@intel.com>

When querying an inferior's threads in Python in a remote debugging
configuration, only the already known threads are returned.

Update the thread list in infpy_threads () before creating the Python objects.

2012-07-24 Markus Metzger <markus.t.metzger@intel.com>

gdb/python/
	* py-inferior.c (infpy_threads): Call update_thread_list ().

gdb/testsuite/gdb.python/
	* py-threads.c: New file.
	* py-threads.exp: New file.


---
 gdb/python/py-inferior.c                |    5 +++
 gdb/testsuite/gdb.python/py-threads.c   |   59 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-threads.exp |   30 ++++++++++++++++
 3 files changed, 94 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-threads.c
 create mode 100644 gdb/testsuite/gdb.python/py-threads.exp

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 2b229be..22adf8c 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -300,6 +300,11 @@ infpy_threads (PyObject *self, PyObject *args)
   struct threadlist_entry *entry;
   inferior_object *inf_obj = (inferior_object *) self;
   PyObject *tuple;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    update_thread_list ();
+  GDB_PY_HANDLE_EXCEPTION (except);
 
   INFPY_REQUIRE_VALID (inf_obj);
 
diff --git a/gdb/testsuite/gdb.python/py-threads.c b/gdb/testsuite/gdb.python/py-threads.c
new file mode 100644
index 0000000..23eadb5
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-threads.c
@@ -0,0 +1,59 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program 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 General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see  <http://www.gnu.org/licenses/>.
+*/
+
+#include <pthread.h>
+
+#define NUMTH 8
+
+static void *
+thread (void *param)
+{
+  pthread_barrier_t *barrier = (pthread_barrier_t *) param;
+
+  pthread_barrier_wait (barrier);
+
+  return param;
+}
+
+static void
+check (pthread_barrier_t *barrier)
+{
+  pthread_barrier_wait (barrier);
+}
+
+extern int
+main (void)
+{
+  pthread_t threads[NUMTH];
+  pthread_barrier_t barrier;
+  int i;
+
+  pthread_barrier_init (&barrier, NULL, NUMTH + 1);
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_create (&threads[i], NULL, thread, &barrier);
+
+  check (&barrier);
+
+  for (i = 0; i < NUMTH; ++i)
+    pthread_join (threads[i], NULL);
+
+  pthread_barrier_destroy (&barrier);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-threads.exp b/gdb/testsuite/gdb.python/py-threads.exp
new file mode 100644
index 0000000..9459c00
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-threads.exp
@@ -0,0 +1,30 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    return -1
+}
+clean_restart $testfile
+
+if { [skip_python_tests] } { continue }
+
+if ![runto check] {
+	untested "py-threads"
+	return -1
+}
+
+gdb_test "python print len(gdb.selected_inferior().threads())" "\r\n9" "py-threads"
-- 
1.7.1

RE: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Metzger, Markus T]

[-- Attachment #1.1: Type: text/plain, Size: 2577 bytes --]

> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Thursday, July 19, 2012 8:45 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org; markus.t.metzger@gmail.com; Pedro Alves (palves@redhat.com)
> Subject: Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()
> 
> On Thu, 19 Jul 2012 17:26:17 +0200, Metzger, Markus T wrote:
> > > So far update_thread_list is called only before (in) the commands which really
> > > need the thread list.  The problem is that infpy_threads can be called from
> > > a script repeatedly which may be pretty inefficient to call update_thread_list
> > > each time.
> >
> > So it is the right place - just not a good one.
> >
> > Those scripts will now do "info threads" to achieve the same effect.
> 
> Not sure if we understand each other.
> 
> If I do GDB script:
> while 1
>   info threads
> end
> 
> Then it is needlessly inefficient, stopped inferior cannot change threads list
> while this loop will re-scan the threads each time.  But I do not think that
> is a problem, 'info threads' is usually executed only after inferior has run
> for a bit.
> 
> Contrary to it I can imagine Python script doing
> gdb.selected_inferior().threads() a lot of times without running the inferior.
> 
> Maybe this GDB scripting vs. Python scripting difference is not significant.

I expected python scripts to store the thread list and only query it when the script doesn't know whether the thread list changed.
Of course, if you repeatedly issue a command instead of reusing the previously computed result, you pay for it.

Currently, python scripts would do gdb.execute ("info threads", False, True) before inferior.threads () to work around the problem.
This would be even worse.


> > > The right would be to:
> > > (1) Support update_thread_list per-inferior, not just for all inferiors at
> > >     once.  For example infpy_threads is interested only in specific inferior.
> > > (2) Cache the current thread list in struct inferior, clearing it from
> > >     begin of target_resume according to PTID (which depends for example on
> > >     'set schedule-multiple', see user_visible_resume_ptid), therefore either
> > >     the specific inferior only or all inferiors.
> >
> > Shouldn't this rather be an event?
> 
> I do not understand here.  Even is Python specific, caching of threads list
> would be good to have even in non-Python GDB.

I am wondering whether gdbserver should notify gdb about new threads via an event. This would obsolete update_thread_list ().

Regards,
Markus.


[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 350 bytes --]

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer, Christian Lamprechter
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Jan Kratochvil]

On Thu, 19 Jul 2012 17:26:17 +0200, Metzger, Markus T wrote:
> > So far update_thread_list is called only before (in) the commands which really
> > need the thread list.  The problem is that infpy_threads can be called from
> > a script repeatedly which may be pretty inefficient to call update_thread_list
> > each time.
> 
> So it is the right place - just not a good one.
> 
> Those scripts will now do "info threads" to achieve the same effect.

Not sure if we understand each other.

If I do GDB script:
while 1
  info threads
end

Then it is needlessly inefficient, stopped inferior cannot change threads list
while this loop will re-scan the threads each time.  But I do not think that
is a problem, 'info threads' is usually executed only after inferior has run
for a bit.

Contrary to it I can imagine Python script doing
gdb.selected_inferior().threads() a lot of times without running the inferior.

Maybe this GDB scripting vs. Python scripting difference is not significant.


> > The right would be to:
> > (1) Support update_thread_list per-inferior, not just for all inferiors at
> >     once.  For example infpy_threads is interested only in specific inferior.
> > (2) Cache the current thread list in struct inferior, clearing it from
> >     begin of target_resume according to PTID (which depends for example on
> >     'set schedule-multiple', see user_visible_resume_ptid), therefore either
> >     the specific inferior only or all inferiors.
> 
> Shouldn't this rather be an event?

I do not understand here.  Even is Python specific, caching of threads list
would be good to have even in non-Python GDB.


Thanks,
Jan

RE: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Metzger, Markus T]

[-- Attachment #1.1: Type: text/plain, Size: 1534 bytes --]

> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Wednesday, July 18, 2012 3:54 PM

Thanks for your review!

> > Update the thread list in infpy_threads () before creating the Python objects.
> 
> It would be good to have a testcase for this issue.

I added a test.


> > Not sure this is the right place. We should probably update the thread list as
> > soon as we learn that the target stopped.
> 
> So far update_thread_list is called only before (in) the commands which really
> need the thread list.  The problem is that infpy_threads can be called from
> a script repeatedly which may be pretty inefficient to call update_thread_list
> each time.

So it is the right place - just not a good one.

Those scripts will now do "info threads" to achieve the same effect.


> The right would be to:
> (1) Support update_thread_list per-inferior, not just for all inferiors at
>     once.  For example infpy_threads is interested only in specific inferior.
> (2) Cache the current thread list in struct inferior, clearing it from
>     begin of target_resume according to PTID (which depends for example on
>     'set schedule-multiple', see user_visible_resume_ptid), therefore either
>     the specific inferior only or all inferiors.

Shouldn't this rather be an event?


> Still it is trying to push at you optimization out of the scope of this patch.
> 
> With a preferred testcase I find your patch OK.  (Pedro knows better all this
> infrun stuff.)

CC'ing Pedro.


Regards,
Markus.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 350 bytes --]

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer, Christian Lamprechter
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

RE: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Metzger, Markus T]

[-- Attachment #1.1: Type: text/plain, Size: 534 bytes --]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Friday, July 06, 2012 7:54 PM

Thanks for your review!

> Markus> Not sure this is the right place. We should probably update the
> Markus> thread list as soon as we learn that the target stopped.
> 
> I'm not sure about this either.

This seems to be common practice. I found 6 other calls.


> Markus> +  update_thread_list ();
> 
> Generally, calls from the python layer into gdb have to be wrapped in a
> TRY_CATCH.

I added it.

Regards,
Markus.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 350 bytes --]

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer, Christian Lamprechter
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Jan Kratochvil]

On Fri, 06 Jul 2012 11:49:31 +0200, markus.t.metzger@intel.com wrote:
> Update the thread list in infpy_threads () before creating the Python objects.

It would be good to have a testcase for this issue.


> Not sure this is the right place. We should probably update the thread list as
> soon as we learn that the target stopped.

So far update_thread_list is called only before (in) the commands which really
need the thread list.  The problem is that infpy_threads can be called from
a script repeatedly which may be pretty inefficient to call update_thread_list
each time.

The right would be to:
(1) Support update_thread_list per-inferior, not just for all inferiors at
    once.  For example infpy_threads is interested only in specific inferior.
(2) Cache the current thread list in struct inferior, clearing it from
    begin of target_resume according to PTID (which depends for example on
    'set schedule-multiple', see user_visible_resume_ptid), therefore either
    the specific inferior only or all inferiors.

Still it is trying to push at you optimization out of the scope of this patch.

With a preferred testcase I find your patch OK.  (Pedro knows better all this
infrun stuff.)


Thanks,
Jan

Re: [PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[Tom Tromey]

>>>>> "Markus" == markus t metzger <markus.t.metzger@intel.com> writes:

Markus> Not sure this is the right place. We should probably update the
Markus> thread list as soon as we learn that the target stopped.

I'm not sure about this either.

Markus> +  update_thread_list ();

Generally, calls from the python layer into gdb have to be wrapped in a
TRY_CATCH.

Tom

[PATCH 1/1] gdb, python: update threads in Inferior.threads ()

[markus.t.metzger]

From: Markus Metzger <markus.t.metzger@intel.com>

When querying an inferior's threads in Python in a remote debugging
configuration, only the already known threads are returned.

Update the thread list in infpy_threads () before creating the Python objects.

Not sure this is the right place. We should probably update the thread list as
soon as we learn that the target stopped.

2012-07-06 Markus Metzger <markus.t.metzger@intel.com>

gdb/python/
	* py-inferior.c (infpy_threads): Call update_thread_list ().


---
 gdb/python/py-inferior.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 2b229be..ef28c33 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -303,6 +303,8 @@ infpy_threads (PyObject *self, PyObject *args)
 
   INFPY_REQUIRE_VALID (inf_obj);
 
+  update_thread_list ();
+
   tuple = PyTuple_New (inf_obj->nthreads);
   if (!tuple)
     return NULL;
-- 
1.7.1