PR Python/12212

PR Python/12212

[Phil Muldoon]

This patches fixes a case where the API can request the currently
selected thread from GDB when no inferior is loaded.  This triggers an
error via assert.  Handle it in Python code instead.

Cheers,

Phil

--

2010-11-16  Phil Muldoon  <pmuldoon@redhat.com>

        PR python/12212 

	* python/py-infthread.c (gdbpy_selected_thread): Raise an error if
	find_thread_object returns NULL.
	* python/py-inferior.c (find_thread_object): Check if PIDGET
	returns 0.

2010-11-16  Phil Muldoon  <pmuldoon@redhat.com>

        PR python/12212 

	* gdb.python/python.exp: Check that selected_thread raises an
          error when no inferior is loaded.

--

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index b1ddb168..6382dab 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -130,6 +130,9 @@ find_thread_object (ptid_t ptid)
   PyObject *inf_obj;
 
   pid = PIDGET (ptid);
+  if (pid == 0)
+    return NULL;
+
   inf_obj = find_inferior_object (pid);
 
   if (inf_obj)
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 86aba50..188f8bd 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -175,13 +175,16 @@ gdbpy_selected_thread (PyObject *self, PyObject *args)
   PyObject *thread_obj;
 
   thread_obj = (PyObject *) find_thread_object (inferior_ptid);
-  if (thread_obj)
+
+  if (!thread_obj)
     {
-      Py_INCREF (thread_obj);
-      return thread_obj;
+      PyErr_SetString (PyExc_RuntimeError,
+		       _("Cannot return currently selected thread."));
+      return NULL;
     }
 
-  Py_RETURN_NONE;
+  Py_INCREF (thread_obj);
+  return thread_obj;
 }
 
 
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index dd9175a..a860dd1 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -146,6 +146,9 @@ gdb_test_no_output "python a = gdb.execute('help', to_string=True)" "collect hel
 
 gdb_test "python print a" ".*aliases -- Aliases of other commands.*" "verify help to uiout"
 
+# Test PR 12212, using InfThread.selected_thread() when no inferior is
+# loaded.
+gdb_test "python print gdb.selected_thread()" "RuntimeError: Cannot return currently selected thread.*" "selected_thread and no inferior"
 # Start with a fresh gdb.
 clean_restart ${testfile}
 

Re: PR Python/12212

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> This patches fixes a case where the API can request the currently
> Phil> selected thread from GDB when no inferior is loaded.  This triggers an
> Phil> error via assert.  Handle it in Python code instead.
>
> This changes gdb.selected_thread to throw an exception if there is no
> thread.  However, this is wrong -- the function is documented to return
> None in this case.


Oops I did not notice that the documentation.  In that case the only
change is to py-inferior.c, and I can delete the changes to
py-infthread.c.  Do you want me to generate a new patch, or should I
just go ahead check that change in?

Cheers,

Phil

Re: PR Python/12212

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> This patches fixes a case where the API can request the currently
Phil> selected thread from GDB when no inferior is loaded.  This triggers an
Phil> error via assert.  Handle it in Python code instead.

This changes gdb.selected_thread to throw an exception if there is no
thread.  However, this is wrong -- the function is documented to return
None in this case.

Tom

Re: PR Python/12212

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> Oops I did not notice that the documentation.  In that case the only
Phil> change is to py-inferior.c, and I can delete the changes to
Phil> py-infthread.c.  Do you want me to generate a new patch, or should I
Phil> just go ahead check that change in?

It would be nice to have a regression test for the crash.

Tom

Re: PR Python/12212

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> Oops I did not notice that the documentation.  In that case the only
> Phil> change is to py-inferior.c, and I can delete the changes to
> Phil> py-infthread.c.  Do you want me to generate a new patch, or should I
> Phil> just go ahead check that change in?
>
> It would be nice to have a regression test for the crash.

I adjusted the regression test in the previous patch I sent to account
for this.  

Patch included.

Cheers,

Phil

--

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index b1ddb168..6382dab 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -130,6 +130,9 @@ find_thread_object (ptid_t ptid)
   PyObject *inf_obj;
 
   pid = PIDGET (ptid);
+  if (pid == 0)
+    return NULL;
+
   inf_obj = find_inferior_object (pid);
 
   if (inf_obj)
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index dd9175a..e6080d1 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -146,6 +146,11 @@ gdb_test_no_output "python a = gdb.execute('help', to_string=True)" "collect hel
 
 gdb_test "python print a" ".*aliases -- Aliases of other commands.*" "verify help to uiout"
 
+# Test PR 12212, using InfThread.selected_thread() when no inferior is
+# loaded.
+gdb_py_test_silent_cmd "python nothread = gdb.selected_thread()" "Attempt to aquire thread with no inferior" 1
+gdb_test "python print nothread == None" "True" "Ensure that no threads are returned"
+
 # Start with a fresh gdb.
 clean_restart ${testfile}
 

Re: PR Python/12212

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> I adjusted the regression test in the previous patch I sent to account
Phil> for this.  

Thanks.  This is ok.

Tom

Re: PR Python/12212

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> I adjusted the regression test in the previous patch I sent to account
> Phil> for this.  
>
> Thanks.  This is ok.
>
> Tom

So committed, thanks.

Cheers,

Phil