[PATCH] fix bug PR 21213 and add test case

[PATCH] fix bug PR 21213 and add test case

[Maksim Dzabraev]

---
 gdb/ChangeLog                             | 6 ++++++
 gdb/python/py-infthread.c                 | 1 +
 gdb/testsuite/ChangeLog                   | 6 ++++++
 gdb/testsuite/gdb.python/py-infthread.exp | 9 +++++++++
 4 files changed, 22 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c4ad2bf1eb..759be23f77 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2017-08-09 Maksim Dzabraev <dzabraew@gmail.com>
+
+	PR python/21213
+	* python/py-infthread.c (thpy_get_inferior): Increment reference
+	of inferior before returning it.
+
 2017-08-07  Maciej W. Rozycki  <macro@imgtec.com>
 
 	PR breakpoints/21886
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 626c15cedb..31d576f7c7 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -162,6 +162,7 @@ thpy_get_inferior (PyObject *self, void *ignore)
   thread_object *thread_obj = (thread_object *) self;
 
   THPY_REQUIRE_VALID (thread_obj);
+  Py_INCREF (thread_obj->inf_obj);
 
   return thread_obj->inf_obj;
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 31f419faf5..4eb7c25af2 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-08-09 Maksim Dzabraev <dzabraew@gmail.com>
+
+	PR python/21213
+	* gdb.python/py-infthread.exp: Test calling
+	InferiorThread.inferior () multiple times in a row.
+
 2017-08-05  Tom Tromey  <tom@tromey.com>
 
 	* gdb.rust/simple.exp: Allow String to appear in a different
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index a5fed8d893..4120ff1ce8 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -39,6 +39,15 @@ if ![runto_main] then {
 
 # Test basic gdb.Inferior attributes and methods.
 
+# Make sure that InferiorThread.inferior returns a new reference (see PR 21213).
+
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 1" 1
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 2" 1
+gdb_test_no_output "python import gc; gc.collect()" "call Python garbage collection"
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 3" 1
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 4" 1
+
+
 gdb_py_test_silent_cmd "python t0 = gdb.selected_thread ()" "test gdb.selected_thread" 1
 gdb_test "python print (t0)" "\\<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>" "verify InferiorThread object"
 gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num"
-- 
2.13.0

[PATCH] fix bug PR 21213 and add test case

[Maksim Dzabraev]

---
 gdb/ChangeLog                             | 5 +++++
 gdb/python/py-infthread.c                 | 1 +
 gdb/testsuite/ChangeLog                   | 5 +++++
 gdb/testsuite/gdb.python/py-infthread.exp | 9 +++++++++
 4 files changed, 20 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c4ad2bf1eb..7c04d4a50e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-08-09 Maksim Dzabraev <dzabraew@gmail.com>
+
+	* python/py-infthread.c (thpy_get_inferior): Increment reference
+	of inferior before returning it.
+
 2017-08-07  Maciej W. Rozycki  <macro@imgtec.com>
 
 	PR breakpoints/21886
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 626c15cedb..31d576f7c7 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -162,6 +162,7 @@ thpy_get_inferior (PyObject *self, void *ignore)
   thread_object *thread_obj = (thread_object *) self;
 
   THPY_REQUIRE_VALID (thread_obj);
+  Py_INCREF (thread_obj->inf_obj);
 
   return thread_obj->inf_obj;
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 31f419faf5..e8107c39cf 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-08-09 Maksim Dzabraev <dzabraew@gmail.com>
+
+	* gdb.python/py-infthread.exp: Test calling
+	InferiorThread.inferior () multiple times in a row.
+
 2017-08-05  Tom Tromey  <tom@tromey.com>
 
 	* gdb.rust/simple.exp: Allow String to appear in a different
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index a5fed8d893..4120ff1ce8 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -39,6 +39,15 @@ if ![runto_main] then {
 
 # Test basic gdb.Inferior attributes and methods.
 
+# Make sure that InferiorThread.inferior returns a new reference (see PR 21213).
+
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 1" 1
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 2" 1
+gdb_test_no_output "python import gc; gc.collect()" "call Python garbage collection"
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 3" 1
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 4" 1
+
+
 gdb_py_test_silent_cmd "python t0 = gdb.selected_thread ()" "test gdb.selected_thread" 1
 gdb_test "python print (t0)" "\\<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>" "verify InferiorThread object"
 gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num"
-- 
2.13.0

Re: [PATCH] fix bug PR 21213 and add test case

[Tom Tromey]

>>>>> "Maksim" == Maksim Dzabraev <dzabraew@gmail.com> writes:

Maksim> +2017-08-09 Maksim Dzabraev <dzabraew@gmail.com>
Maksim> +
Maksim> +	* python/py-infthread.c (thpy_get_inferior): Increment reference
Maksim> +	of inferior before returning it.

If you mention "PR python/21213" in the ChangeLog entry, the commit will
show up in the bug in bugzilla as well.  See the ChangeLog for plenty of
examples of this.

Tom

Re: Garbarge collector eats gdb.selected_thread().inferior

[Simon Marchi]

Hi Maksim,

The fix looks right, and Tom also said so in Bugzilla.

However, I'm unable to apply your patch to look at the test case.  The 
long lines have been wrapped, presumably by your email client.  Can you 
send it again using the "git send-email" command?  I'll still make the 
comments I can about the current version.

On 2017-08-05 07:57, Максим Дзабраев wrote:
> If multiple time call gdb.selected_thread ().inferior, gdb throw 
> exception:
> 
> (gdb) pi gdb.selected_thread().inferior
> <gdb.Inferior object at 0x7f1952bea698>
> (gdb) pi gdb.selected_thread().inferior
> <gdb.Inferior object at 0x7f1952bea698>
> (gdb) pi gdb.selected_thread().inferior
> Python Exception <type 'exceptions.AttributeError'> 'NoneType' object
> has no attribute 'inferior':
> Error while executing Python code.
> (gdb) info threads
>   Id   Target Id         Frame
> * 1    Thread 0x7f54f0474740 (LWP 584) "mc" 0x00007f54ef055c33 in
> select () from /lib/x86_64-linux-gnu/libc.so.6
> 
> I report about this bug here:
> https://sourceware.org/bugzilla/show_bug.cgi?id=21213
> 
> Following patch fixes this bug and add test of this bug. In attachment
> I add gdb.log and gdb.sum before and after apply patch to
> thpy_get_inferior.
> 
> 
> ChangeLog
> 2017.08.05 Maksim Dzabraev <dzabraew@gmail.com>
> 
>         * gdb/python/py-infthread.c (hpy_get_inferior)
>         * gdb/testsuite/gdb.python/py-infthread.exp: Fix bug 21213 and 
> add test

Please consult this page for details about the ChangeLog entries.

https://sourceware.org/gdb/wiki/ContributionChecklist

In short, each change should be documented in the ChangeLog closest to 
it (then for each file you write the relative path to it), and should 
describe what changed a bit more precisely.  For example:

gdb/ChangeLog:

	* python/py-infthread.c (thpy_get_inferior): Increment reference
	of inferior before returning it.

gdb/testsuite/ChangeLog:

	* gdb.python/py-infthread.exp: Test calling
	InferiorThread.inferior () multiple times in a row.

> ---
> 
> diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
> index 626c15c..31d576f 100644
> --- a/gdb/python/py-infthread.c
> +++ b/gdb/python/py-infthread.c
> @@ -162,6 +162,7 @@ thpy_get_inferior (PyObject *self, void *ignore)
>    thread_object *thread_obj = (thread_object *) self;
> 
>    THPY_REQUIRE_VALID (thread_obj);
> +  Py_INCREF (thread_obj->inf_obj);
> 
>    return thread_obj->inf_obj;
>  }
> diff --git a/gdb/testsuite/gdb.python/py-infthread.exp
> b/gdb/testsuite/gdb.python/py-infthread.exp
> index a5fed8d..dcb7e26 100644
> --- a/gdb/testsuite/gdb.python/py-infthread.exp
> +++ b/gdb/testsuite/gdb.python/py-infthread.exp
> @@ -39,6 +39,14 @@ if ![runto_main] then {
> 
>  # Test basic gdb.Inferior attributes and methods.
> 
> +#testing https://sourceware.org/bugzilla/show_bug.cgi?id=21213

It's nice to give a bit of context so that readers don't have to visit 
the PR to have an idea of what it's about.  I suggest:

# Make sure that InferiorThread.inferior returns a new reference (see PR 
21213).

> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> gdb.selected_thread" 1
> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> gdb.selected_thread" 1
> +gdb_test_no_output "python import gc; gc.collect()" "test
> InferiorThread.inferior" "test InferiorThread.inferior"

You have one too many parameter to gdb_test_no_output.  Also, the test 
name should probably be something like "call Python garbage collection".

> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> gdb.selected_thread" 1
> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> gdb.selected_thread" 1

All tests (here one line == one test) should have a unique name, so you 
could name them "test InferiorThread.inferior #" where # is 1, 2, 3, 4.

> +
> +
>  gdb_py_test_silent_cmd "python t0 = gdb.selected_thread ()" "test
> gdb.selected_thread" 1
>  gdb_test "python print (t0)" "\\<gdb.InferiorThread object at
> 0x\[\[:xdigit:\]\]+>" "verify InferiorThread object"
>  gdb_test "python print ('result = %s' % t0.num)" " = 1" "test

Thanks!

Simon

[PATCH] fix bug PR 21213 and add test case

[Maksim Dzabraev]

---
 gdb/ChangeLog                             | 7 +++++++
 gdb/python/py-infthread.c                 | 1 +
 gdb/testsuite/ChangeLog                   | 6 ++++++
 gdb/testsuite/gdb.python/py-infthread.exp | 9 +++++++++
 4 files changed, 23 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9fe632985e..160c521c96 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-10-28 Maksim Dzabraev <dzabraew@gmail.com>
+
+	PR python/21213
+	* python/py-infthread.c (thpy_get_inferior): Increment reference
+	of inferior before returning it.
+
+
 2017-10-27  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* unittests/common-utils-selftests.c (format): Add
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 626c15cedb..31d576f7c7 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -162,6 +162,7 @@ thpy_get_inferior (PyObject *self, void *ignore)
   thread_object *thread_obj = (thread_object *) self;
 
   THPY_REQUIRE_VALID (thread_obj);
+  Py_INCREF (thread_obj->inf_obj);
 
   return thread_obj->inf_obj;
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0254a28550..72334fd3e2 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-10-28 Maksim Dzabraev <dzabraew@gmail.com>
+
+	PR python/21213
+	* gdb.python/py-infthread.exp: Test calling
+	InferiorThread.inferior () multiple times in a row.
+
 2017-10-27  Keith Seitz  <keiths@redhat.com>
 
 	* gdb.opt/inline-break.exp (break_info_1): New procedure.
diff --git a/gdb/testsuite/gdb.python/py-infthread.exp b/gdb/testsuite/gdb.python/py-infthread.exp
index 0711d6994e..69fc9709b2 100644
--- a/gdb/testsuite/gdb.python/py-infthread.exp
+++ b/gdb/testsuite/gdb.python/py-infthread.exp
@@ -51,6 +51,15 @@ gdb_test "python print(seen_a_thread)" "True"
 
 # Test basic gdb.Inferior attributes and methods.
 
+# Make sure that InferiorThread.inferior returns a new reference (see PR 21213).
+
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 1" 1
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 2" 1
+gdb_test_no_output "python import gc; gc.collect()" "call Python garbage collection"
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 3" 1
+gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test InferiorThread.inferior 4" 1
+
+
 gdb_py_test_silent_cmd "python t0 = gdb.selected_thread ()" "test gdb.selected_thread" 1
 gdb_test "python print (t0)" "\\<gdb.InferiorThread object at 0x\[\[:xdigit:\]\]+>" "verify InferiorThread object"
 gdb_test "python print ('result = %s' % t0.num)" " = 1" "test InferiorThread.num"
-- 
2.13.0

Re: [PATCH] fix bug PR 21213 and add test case

[Simon Marchi]

Hi Maksim,

Thanks for the update.  Your patch would need a more descriptive name 
than just the PR number.  I suggest "Make gdb.selected_thread().inferior 
return a new reference".

It would also need a summary in the commit log.  What you had in your 
original email was good, and you can also mention that the 
thpy_get_inferior function should return a new reference to the existing 
inferior object, and therefore should increment its refcount.


On 2017-10-28 13:18, Maksim Dzabraev wrote:
> ---
>  gdb/ChangeLog                             | 7 +++++++
>  gdb/python/py-infthread.c                 | 1 +
>  gdb/testsuite/ChangeLog                   | 6 ++++++
>  gdb/testsuite/gdb.python/py-infthread.exp | 9 +++++++++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 9fe632985e..160c521c96 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2017-10-28 Maksim Dzabraev <dzabraew@gmail.com>

Please use two spaces between each field (between date and name, and 
between name and email).

> +
> +	PR python/21213
> +	* python/py-infthread.c (thpy_get_inferior): Increment reference
> +	of inferior before returning it.
> +
> +
>  2017-10-27  Simon Marchi  <simon.marchi@ericsson.com>
> 
>  	* unittests/common-utils-selftests.c (format): Add
> diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
> index 626c15cedb..31d576f7c7 100644
> --- a/gdb/python/py-infthread.c
> +++ b/gdb/python/py-infthread.c
> @@ -162,6 +162,7 @@ thpy_get_inferior (PyObject *self, void *ignore)
>    thread_object *thread_obj = (thread_object *) self;
> 
>    THPY_REQUIRE_VALID (thread_obj);
> +  Py_INCREF (thread_obj->inf_obj);
> 
>    return thread_obj->inf_obj;
>  }
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 0254a28550..72334fd3e2 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-10-28 Maksim Dzabraev <dzabraew@gmail.com>

Same here.

> +
> +	PR python/21213
> +	* gdb.python/py-infthread.exp: Test calling
> +	InferiorThread.inferior () multiple times in a row.
> +
>  2017-10-27  Keith Seitz  <keiths@redhat.com>
> 
>  	* gdb.opt/inline-break.exp (break_info_1): New procedure.
> diff --git a/gdb/testsuite/gdb.python/py-infthread.exp
> b/gdb/testsuite/gdb.python/py-infthread.exp
> index 0711d6994e..69fc9709b2 100644
> --- a/gdb/testsuite/gdb.python/py-infthread.exp
> +++ b/gdb/testsuite/gdb.python/py-infthread.exp
> @@ -51,6 +51,15 @@ gdb_test "python print(seen_a_thread)" "True"
> 
>  # Test basic gdb.Inferior attributes and methods.
> 
> +# Make sure that InferiorThread.inferior returns a new reference (see
> PR 21213).
> +
> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> InferiorThread.inferior 1" 1
> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> InferiorThread.inferior 2" 1
> +gdb_test_no_output "python import gc; gc.collect()" "call Python
> garbage collection"
> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> InferiorThread.inferior 3" 1
> +gdb_py_test_silent_cmd "python gdb.selected_thread ().inferior" "test
> InferiorThread.inferior 4" 1
> +
> +
>  gdb_py_test_silent_cmd "python t0 = gdb.selected_thread ()" "test
> gdb.selected_thread" 1
>  gdb_test "python print (t0)" "\\<gdb.InferiorThread object at
> 0x\[\[:xdigit:\]\]+>" "verify InferiorThread object"
>  gdb_test "python print ('result = %s' % t0.num)" " = 1" "test
> InferiorThread.num"


Thanks,

Simon