* [PATCH] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
@ 2018-08-24 17:46 Pedro Alves
2018-08-24 18:53 ` Simon Marchi
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2018-08-24 17:46 UTC (permalink / raw)
To: gdb-patches
This commit fixes a 8.1->8.2 regression exposed by
gdb.python/py-evthreads.exp when testing with
--target_board=native-gdbserver.
gdb.log shows:
src/gdb/thread.c:93: internal-error: thread_info* inferior_thread(): Assertion `tp' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) FAIL: gdb.python/py-evthreads.exp: run to breakpoint 1 (GDB internal error)
A backtrace shows (frames #2 and #10 highlighted) that the assertion
fails when GDB is setting up the connection to the remote target, in
non-stop mode:
#0 0x0000000000622ff0 in internal_error(char const*, int, char const*, ...) (file=0xc1ad98 "src/gdb/thread.c", line=93, fmt=0xc1ad20 "%s: Assertion `%s' failed.") at src/gdb/common/errors.c:54
#1 0x000000000089567e in inferior_thread() () at src/gdb/thread.c:93
= #2 0x00000000004da91d in get_event_thread() () at src/gdb/python/py-threadevent.c:38
#3 0x00000000004da9b7 in create_thread_event_object(_typeobject*, _object*) (py_type=0x11574c0 <continue_event_object_type>, thread=0x0)
at src/gdb/python/py-threadevent.c:60
#4 0x00000000004bf6fe in create_continue_event_object() () at src/gdb/python/py-continueevent.c:27
#5 0x00000000004bf738 in emit_continue_event(ptid_t) (ptid=...) at src/gdb/python/py-continueevent.c:40
#6 0x00000000004c7d47 in python_on_resume(ptid_t) (ptid=...) at src/gdb/python/py-inferior.c:108
#7 0x0000000000485bfb in std::_Function_handler<void (ptid_t), void (*)(ptid_t)>::_M_invoke(std::_Any_data const&, ptid_t&&) (__functor=..., __args#0=...) at /usr/include/c++/7/bits/std_function.h:316
#8 0x000000000089b416 in std::function<void (ptid_t)>::operator()(ptid_t) const (this=0x12aa600, __args#0=...)
at /usr/include/c++/7/bits/std_function.h:706
#9 0x000000000089aa0e in gdb::observers::observable<ptid_t>::notify(ptid_t) const (this=0x118a7a0 <gdb::observers::target_resumed>, args#0=...)
at src/gdb/common/observable.h:106
= #10 0x0000000000896fbe in set_running(ptid_t, int) (ptid=..., running=1) at src/gdb/thread.c:880
#11 0x00000000007f750f in remote_target::remote_add_thread(ptid_t, bool, bool) (this=0x12c5440, ptid=..., running=true, executing=true) at src/gdb/remote.c:2434
#12 0x00000000007f779d in remote_target::remote_notice_new_inferior(ptid_t, int) (this=0x12c5440, currthread=..., executing=1)
at src/gdb/remote.c:2515
#13 0x00000000007f9c44 in remote_target::update_thread_list() (this=0x12c5440) at src/gdb/remote.c:3831
#14 0x00000000007fb922 in remote_target::start_remote(int, int) (this=0x12c5440, from_tty=0, extended_p=0)
at src/gdb/remote.c:4655
#15 0x00000000007fd102 in remote_target::open_1(char const*, int, int) (name=0x1a4f45e "localhost:2346", from_tty=0, extended_p=0)
at src/gdb/remote.c:5638
#16 0x00000000007fbec1 in remote_target::open(char const*, int) (name=0x1a4f45e "localhost:2346", from_tty=0)
at src/gdb/remote.c:4862
So on frame #10, we're marking a newly-discovered thread as running,
and that causes the Python API to emit a gdb.ContinueEvent.
gdb.ContinueEvent is a gdb.ThreadEvent, and as such includes the event
thread as the "inferior_thread" attribute. The problem is that when
we get to frame #3/#4, we lost all references to the thread that is
being marked as running. create_continue_event_object assumes that it
is the current thread, which is not true in this case.
Fix this by passing down the right thread in
create_continue_event_object. Also remove
create_thread_event_object's default argument and have the only other
caller left pass down the right thread explicitly too.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
PR gdb/23379
* python/py-continueevent.c: Include "gdbthread.h".
(create_continue_event_object): Add intro comment. Add 'ptid'
parameter. Use it to find thread to pass to
create_thread_event_object.
(emit_continue_event): Pass PTID down to
create_continue_event_object.
* python/py-event.h (py_get_event_thread): Declare.
(create_thread_event_object): Remove default from 'thread'
parameter.
* python/py-stopevent.c (create_stop_event_object): Use
py_get_event_thread.
* python/py-threadevent.c (get_event_thread): Rename to ...
(py_get_event_thread): ... this, and make extern.
(create_thread_event_object): Assert that THREAD isn't null.
Don't find the event thread here.
---
gdb/python/py-continueevent.c | 31 ++++++++++++++++++++++++++++---
gdb/python/py-event.h | 12 +++++++++++-
gdb/python/py-stopevent.c | 2 +-
gdb/python/py-threadevent.c | 22 +++++-----------------
4 files changed, 45 insertions(+), 22 deletions(-)
diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
index e5e922f0155..08d7da3cf2f 100644
--- a/gdb/python/py-continueevent.c
+++ b/gdb/python/py-continueevent.c
@@ -20,11 +20,36 @@
#include "defs.h"
#include "py-event.h"
#include "py-ref.h"
+#include "gdbthread.h"
+
+/* Create a gdb.ContinueEvent event. gdb.ContinueEvent is-a
+ gdb.ThreadEvent, and thread events can either be thread specific or
+ process wide. If gdb is running in non-stop mode then the event is
+ thread specific (in which case the PTID thread is included in the
+ event), otherwise it is process wide (in which case PTID is
+ ignored). In either case a borrowed reference is returned. */
static gdbpy_ref<>
-create_continue_event_object (void)
+create_continue_event_object (ptid_t ptid)
{
- return create_thread_event_object (&continue_event_object_type);
+ PyObject *py_thr = nullptr;
+
+ if (non_stop)
+ {
+ thread_info *thr = find_thread_ptid (ptid);
+ if (thr != nullptr)
+ py_thr = (PyObject *) thread_to_thread_object (thr);
+ }
+ else
+ py_thr = Py_None;
+
+ if (py_thr == nullptr)
+ {
+ PyErr_SetString (PyExc_RuntimeError, _("Could not find event thread"));
+ return nullptr;
+ }
+
+ return create_thread_event_object (&continue_event_object_type, py_thr);
}
/* Callback function which notifies observers when a continue event occurs.
@@ -37,7 +62,7 @@ emit_continue_event (ptid_t ptid)
if (evregpy_no_listeners_p (gdb_py_events.cont))
return 0;
- gdbpy_ref<> event (create_continue_event_object ());
+ gdbpy_ref<> event (create_continue_event_object (ptid));
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.cont);
return -1;
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index 22bab108ed1..7a9a51ddb1c 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -63,8 +63,18 @@ extern int evpy_emit_event (PyObject *event,
eventregistry_object *registry);
extern gdbpy_ref<> create_event_object (PyTypeObject *py_type);
+
+/* thread events can either be thread specific or process wide. If gdb is
+ running in non-stop mode then the event is thread specific, otherwise
+ it is process wide.
+ This function returns the currently stopped thread in non-stop mode and
+ Py_None otherwise. In each case it returns a borrowed reference. */
+extern PyObject *py_get_event_thread ()
+ CPYCHECKER_RETURNS_BORROWED_REF;
+
extern gdbpy_ref<> create_thread_event_object (PyTypeObject *py_type,
- PyObject *thread = nullptr);
+ PyObject *thread);
+
extern int emit_new_objfile_event (struct objfile *objfile);
extern int emit_clear_objfiles_event (void);
diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
index 884e6e3ef8f..33b65f99b80 100644
--- a/gdb/python/py-stopevent.c
+++ b/gdb/python/py-stopevent.c
@@ -23,7 +23,7 @@
gdbpy_ref<>
create_stop_event_object (PyTypeObject *py_type)
{
- return create_thread_event_object (py_type);
+ return create_thread_event_object (py_type, py_get_event_thread ());
}
/* Callback observers when a stop event occurs. This function will create a
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index d6aa9467b9a..1a115bad801 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -20,17 +20,10 @@
#include "infrun.h"
#include "gdbthread.h"
-/* thread events can either be thread specific or process wide. If gdb is
- running in non-stop mode then the event is thread specific, otherwise
- it is process wide.
- This function returns the currently stopped thread in non-stop mode and
- Py_None otherwise. In each case it returns a borrowed reference. */
+/* See py-event.h. */
-static PyObject *get_event_thread (void)
- CPYCHECKER_RETURNS_BORROWED_REF;
-
-static PyObject *
-get_event_thread (void)
+PyObject *
+py_get_event_thread ()
{
PyObject *thread;
@@ -51,17 +44,12 @@ get_event_thread (void)
gdbpy_ref<>
create_thread_event_object (PyTypeObject *py_type, PyObject *thread)
{
+ gdb_assert (thread != NULL);
+
gdbpy_ref<> thread_event_obj (create_event_object (py_type));
if (thread_event_obj == NULL)
return NULL;
- if (thread == NULL)
- {
- thread = get_event_thread ();
- if (!thread)
- return NULL;
- }
-
if (evpy_add_attribute (thread_event_obj.get (),
"inferior_thread",
thread) < 0)
--
2.14.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
2018-08-24 17:46 [PATCH] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379) Pedro Alves
@ 2018-08-24 18:53 ` Simon Marchi
2018-08-24 19:44 ` [PATCH v2] " Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2018-08-24 18:53 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2018-08-24 01:46 PM, Pedro Alves wrote:
> This commit fixes a 8.1->8.2 regression exposed by
> gdb.python/py-evthreads.exp when testing with
> --target_board=native-gdbserver.
>
> gdb.log shows:
>
> src/gdb/thread.c:93: internal-error: thread_info* inferior_thread(): Assertion `tp' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) FAIL: gdb.python/py-evthreads.exp: run to breakpoint 1 (GDB internal error)
>
> A backtrace shows (frames #2 and #10 highlighted) that the assertion
> fails when GDB is setting up the connection to the remote target, in
> non-stop mode:
>
> #0 0x0000000000622ff0 in internal_error(char const*, int, char const*, ...) (file=0xc1ad98 "src/gdb/thread.c", line=93, fmt=0xc1ad20 "%s: Assertion `%s' failed.") at src/gdb/common/errors.c:54
> #1 0x000000000089567e in inferior_thread() () at src/gdb/thread.c:93
> = #2 0x00000000004da91d in get_event_thread() () at src/gdb/python/py-threadevent.c:38
> #3 0x00000000004da9b7 in create_thread_event_object(_typeobject*, _object*) (py_type=0x11574c0 <continue_event_object_type>, thread=0x0)
> at src/gdb/python/py-threadevent.c:60
> #4 0x00000000004bf6fe in create_continue_event_object() () at src/gdb/python/py-continueevent.c:27
> #5 0x00000000004bf738 in emit_continue_event(ptid_t) (ptid=...) at src/gdb/python/py-continueevent.c:40
> #6 0x00000000004c7d47 in python_on_resume(ptid_t) (ptid=...) at src/gdb/python/py-inferior.c:108
> #7 0x0000000000485bfb in std::_Function_handler<void (ptid_t), void (*)(ptid_t)>::_M_invoke(std::_Any_data const&, ptid_t&&) (__functor=..., __args#0=...) at /usr/include/c++/7/bits/std_function.h:316
> #8 0x000000000089b416 in std::function<void (ptid_t)>::operator()(ptid_t) const (this=0x12aa600, __args#0=...)
> at /usr/include/c++/7/bits/std_function.h:706
> #9 0x000000000089aa0e in gdb::observers::observable<ptid_t>::notify(ptid_t) const (this=0x118a7a0 <gdb::observers::target_resumed>, args#0=...)
> at src/gdb/common/observable.h:106
> = #10 0x0000000000896fbe in set_running(ptid_t, int) (ptid=..., running=1) at src/gdb/thread.c:880
> #11 0x00000000007f750f in remote_target::remote_add_thread(ptid_t, bool, bool) (this=0x12c5440, ptid=..., running=true, executing=true) at src/gdb/remote.c:2434
> #12 0x00000000007f779d in remote_target::remote_notice_new_inferior(ptid_t, int) (this=0x12c5440, currthread=..., executing=1)
> at src/gdb/remote.c:2515
> #13 0x00000000007f9c44 in remote_target::update_thread_list() (this=0x12c5440) at src/gdb/remote.c:3831
> #14 0x00000000007fb922 in remote_target::start_remote(int, int) (this=0x12c5440, from_tty=0, extended_p=0)
> at src/gdb/remote.c:4655
> #15 0x00000000007fd102 in remote_target::open_1(char const*, int, int) (name=0x1a4f45e "localhost:2346", from_tty=0, extended_p=0)
> at src/gdb/remote.c:5638
> #16 0x00000000007fbec1 in remote_target::open(char const*, int) (name=0x1a4f45e "localhost:2346", from_tty=0)
> at src/gdb/remote.c:4862
>
> So on frame #10, we're marking a newly-discovered thread as running,
> and that causes the Python API to emit a gdb.ContinueEvent.
> gdb.ContinueEvent is a gdb.ThreadEvent, and as such includes the event
> thread as the "inferior_thread" attribute. The problem is that when
> we get to frame #3/#4, we lost all references to the thread that is
> being marked as running. create_continue_event_object assumes that it
> is the current thread, which is not true in this case.
>
> Fix this by passing down the right thread in
> create_continue_event_object. Also remove
> create_thread_event_object's default argument and have the only other
> caller left pass down the right thread explicitly too.
>
> gdb/ChangeLog:
> yyyy-mm-dd Pedro Alves <palves@redhat.com>
>
> PR gdb/23379
> * python/py-continueevent.c: Include "gdbthread.h".
> (create_continue_event_object): Add intro comment. Add 'ptid'
> parameter. Use it to find thread to pass to
> create_thread_event_object.
> (emit_continue_event): Pass PTID down to
> create_continue_event_object.
> * python/py-event.h (py_get_event_thread): Declare.
> (create_thread_event_object): Remove default from 'thread'
> parameter.
> * python/py-stopevent.c (create_stop_event_object): Use
> py_get_event_thread.
> * python/py-threadevent.c (get_event_thread): Rename to ...
> (py_get_event_thread): ... this, and make extern.
> (create_thread_event_object): Assert that THREAD isn't null.
> Don't find the event thread here.
> ---
> gdb/python/py-continueevent.c | 31 ++++++++++++++++++++++++++++---
> gdb/python/py-event.h | 12 +++++++++++-
> gdb/python/py-stopevent.c | 2 +-
> gdb/python/py-threadevent.c | 22 +++++-----------------
> 4 files changed, 45 insertions(+), 22 deletions(-)
>
> diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
> index e5e922f0155..08d7da3cf2f 100644
> --- a/gdb/python/py-continueevent.c
> +++ b/gdb/python/py-continueevent.c
> @@ -20,11 +20,36 @@
> #include "defs.h"
> #include "py-event.h"
> #include "py-ref.h"
> +#include "gdbthread.h"
> +
> +/* Create a gdb.ContinueEvent event. gdb.ContinueEvent is-a
> + gdb.ThreadEvent, and thread events can either be thread specific or
> + process wide. If gdb is running in non-stop mode then the event is
> + thread specific (in which case the PTID thread is included in the
> + event), otherwise it is process wide (in which case PTID is
> + ignored). In either case a borrowed reference is returned. */
I think the last sentence is wrong. If we are returning a gdbpy_ref, doesn't
mean we are returning an owning reference? From what I understand, a borrowed
reference would be a naked pointer that you don't need to decref.
py_get_event_thread, from which this seems to have been copied from, does indeed
return a borrowed reference.
So it should say "a new reference is returned".
Ref: https://docs.python.org/3.7/c-api/intro.html, ctrl-f "borrow".
> static gdbpy_ref<>
> -create_continue_event_object (void)
> +create_continue_event_object (ptid_t ptid)
> {
> - return create_thread_event_object (&continue_event_object_type);
> + PyObject *py_thr = nullptr;
> +
> + if (non_stop)
> + {
> + thread_info *thr = find_thread_ptid (ptid);
> + if (thr != nullptr)
> + py_thr = (PyObject *) thread_to_thread_object (thr);
> + }
> + else
> + py_thr = Py_None;
> +
> + if (py_thr == nullptr)
> + {
> + PyErr_SetString (PyExc_RuntimeError, _("Could not find event thread"));
> + return nullptr;
> + }
> +
> + return create_thread_event_object (&continue_event_object_type, py_thr);
This and py_get_thread_event look very similar, could we use py_get_thread_event
from here? See the patch at the end of the message.
> }
>
> /* Callback function which notifies observers when a continue event occurs.
> @@ -37,7 +62,7 @@ emit_continue_event (ptid_t ptid)
> if (evregpy_no_listeners_p (gdb_py_events.cont))
> return 0;
>
> - gdbpy_ref<> event (create_continue_event_object ());
> + gdbpy_ref<> event (create_continue_event_object (ptid));
> if (event != NULL)
> return evpy_emit_event (event.get (), gdb_py_events.cont);
> return -1;
> diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
> index 22bab108ed1..7a9a51ddb1c 100644
> --- a/gdb/python/py-event.h
> +++ b/gdb/python/py-event.h
> @@ -63,8 +63,18 @@ extern int evpy_emit_event (PyObject *event,
> eventregistry_object *registry);
>
> extern gdbpy_ref<> create_event_object (PyTypeObject *py_type);
> +
> +/* thread events can either be thread specific or process wide. If gdb is
> + running in non-stop mode then the event is thread specific, otherwise
> + it is process wide.
> + This function returns the currently stopped thread in non-stop mode and
> + Py_None otherwise. In each case it returns a borrowed reference. */
> +extern PyObject *py_get_event_thread ()
> + CPYCHECKER_RETURNS_BORROWED_REF;
> +
> extern gdbpy_ref<> create_thread_event_object (PyTypeObject *py_type,
> - PyObject *thread = nullptr);
> + PyObject *thread);
> +
> extern int emit_new_objfile_event (struct objfile *objfile);
> extern int emit_clear_objfiles_event (void);
>
> diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
> index 884e6e3ef8f..33b65f99b80 100644
> --- a/gdb/python/py-stopevent.c
> +++ b/gdb/python/py-stopevent.c
> @@ -23,7 +23,7 @@
> gdbpy_ref<>
> create_stop_event_object (PyTypeObject *py_type)
> {
> - return create_thread_event_object (py_type);
> + return create_thread_event_object (py_type, py_get_event_thread ());
> }
>
> /* Callback observers when a stop event occurs. This function will create a
> diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
> index d6aa9467b9a..1a115bad801 100644
> --- a/gdb/python/py-threadevent.c
> +++ b/gdb/python/py-threadevent.c
> @@ -20,17 +20,10 @@
> #include "infrun.h"
> #include "gdbthread.h"
>
> -/* thread events can either be thread specific or process wide. If gdb is
> - running in non-stop mode then the event is thread specific, otherwise
> - it is process wide.
> - This function returns the currently stopped thread in non-stop mode and
> - Py_None otherwise. In each case it returns a borrowed reference. */
> +/* See py-event.h. */
>
> -static PyObject *get_event_thread (void)
> - CPYCHECKER_RETURNS_BORROWED_REF;
> -
> -static PyObject *
> -get_event_thread (void)
> +PyObject *
> +py_get_event_thread ()
> {
> PyObject *thread;
>
> @@ -51,17 +44,12 @@ get_event_thread (void)
> gdbpy_ref<>
> create_thread_event_object (PyTypeObject *py_type, PyObject *thread)
> {
> + gdb_assert (thread != NULL);
> +
> gdbpy_ref<> thread_event_obj (create_event_object (py_type));
You mentioned at some point that you preferred
gdbpy_ref<> thread_event_obj = create_event_object (py_type);
over
gdbpy_ref<> thread_event_obj (create_event_object (py_type));
(which I agree with).
> if (thread_event_obj == NULL)
> return NULL;
>
> - if (thread == NULL)
> - {
> - thread = get_event_thread ();
> - if (!thread)
> - return NULL;
> - }
> -
> if (evpy_add_attribute (thread_event_obj.get (),
> "inferior_thread",
> thread) < 0)
>
Proposed changes:
From 6b789cb151b5160fca3e3c90d47fb921e3d7697e Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Fri, 24 Aug 2018 14:26:23 -0400
Subject: [PATCH] Use py_get_event_thread in create_continue_event_object
---
gdb/python/py-continueevent.c | 18 +++---------------
gdb/python/py-event.h | 2 +-
gdb/python/py-stopevent.c | 3 ++-
gdb/python/py-threadevent.c | 16 ++++++++++------
4 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
index 08d7da3..c011b71 100644
--- a/gdb/python/py-continueevent.c
+++ b/gdb/python/py-continueevent.c
@@ -32,22 +32,10 @@
static gdbpy_ref<>
create_continue_event_object (ptid_t ptid)
{
- PyObject *py_thr = nullptr;
-
- if (non_stop)
- {
- thread_info *thr = find_thread_ptid (ptid);
- if (thr != nullptr)
- py_thr = (PyObject *) thread_to_thread_object (thr);
- }
- else
- py_thr = Py_None;
+ PyObject *py_thr = py_get_event_thread (ptid);
if (py_thr == nullptr)
- {
- PyErr_SetString (PyExc_RuntimeError, _("Could not find event thread"));
- return nullptr;
- }
+ return nullptr;
return create_thread_event_object (&continue_event_object_type, py_thr);
}
@@ -62,7 +50,7 @@ emit_continue_event (ptid_t ptid)
if (evregpy_no_listeners_p (gdb_py_events.cont))
return 0;
- gdbpy_ref<> event (create_continue_event_object (ptid));
+ gdbpy_ref<> event = create_continue_event_object (ptid);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.cont);
return -1;
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index 7a9a51d..56003e8 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -69,7 +69,7 @@ extern gdbpy_ref<> create_event_object (PyTypeObject *py_type);
it is process wide.
This function returns the currently stopped thread in non-stop mode and
Py_None otherwise. In each case it returns a borrowed reference. */
-extern PyObject *py_get_event_thread ()
+extern PyObject *py_get_event_thread (ptid_t ptid)
CPYCHECKER_RETURNS_BORROWED_REF;
extern gdbpy_ref<> create_thread_event_object (PyTypeObject *py_type,
diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
index 33b65f9..f6bd207 100644
--- a/gdb/python/py-stopevent.c
+++ b/gdb/python/py-stopevent.c
@@ -23,7 +23,8 @@
gdbpy_ref<>
create_stop_event_object (PyTypeObject *py_type)
{
- return create_thread_event_object (py_type, py_get_event_thread ());
+ return create_thread_event_object (py_type,
+ py_get_event_thread (inferior_ptid));
}
/* Callback observers when a stop event occurs. This function will create a
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index 1a115ba..1bc2fb0 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -23,22 +23,26 @@
/* See py-event.h. */
PyObject *
-py_get_event_thread ()
+py_get_event_thread (ptid_t ptid)
{
- PyObject *thread;
+ PyObject *pythread;
if (non_stop)
- thread = (PyObject *) thread_to_thread_object (inferior_thread ());
+ {
+ thread_info *thread = find_thread_ptid (ptid);
+ if (thread != nullptr)
+ pythread = (PyObject *) thread_to_thread_object (thread);
+ }
else
- thread = Py_None;
+ pythread = Py_None;
- if (!thread)
+ if (!pythread)
{
PyErr_SetString (PyExc_RuntimeError, "Could not find event thread");
return NULL;
}
- return thread;
+ return pythread;
}
gdbpy_ref<>
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
2018-08-24 18:53 ` Simon Marchi
@ 2018-08-24 19:44 ` Pedro Alves
2018-08-24 19:47 ` Pedro Alves
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Pedro Alves @ 2018-08-24 19:44 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 08/24/2018 07:52 PM, Simon Marchi wrote:
> On 2018-08-24 01:46 PM, Pedro Alves wrote:
> I think the last sentence is wrong. If we are returning a gdbpy_ref, doesn't
> mean we are returning an owning reference? From what I understand, a borrowed
> reference would be a naked pointer that you don't need to decref.
>
> py_get_event_thread, from which this seems to have been copied from, does indeed
> return a borrowed reference.
>
> So it should say "a new reference is returned".
>
> Ref: https://docs.python.org/3.7/c-api/intro.html, ctrl-f "borrow".
Whoops, yes, blind copy&paste. Fixed.
> This and py_get_thread_event look very similar, could we use py_get_thread_event
> from here? See the patch at the end of the message.
I like it!
>
>> }
>>
>> /* Callback function which notifies observers when a continue event occurs.
>> @@ -37,7 +62,7 @@ emit_continue_event (ptid_t ptid)
>> if (evregpy_no_listeners_p (gdb_py_events.cont))
>> return 0;
>>
>> - gdbpy_ref<> event (create_continue_event_object ());
>> + gdbpy_ref<> event (create_continue_event_object (ptid));
*see below*
>> @@ -51,17 +44,12 @@ get_event_thread (void)
>> gdbpy_ref<>
>> create_thread_event_object (PyTypeObject *py_type, PyObject *thread)
>> {
>> + gdb_assert (thread != NULL);
>> +
>> gdbpy_ref<> thread_event_obj (create_event_object (py_type));
>
> You mentioned at some point that you preferred
>
> gdbpy_ref<> thread_event_obj = create_event_object (py_type);
>
> over
>
> gdbpy_ref<> thread_event_obj (create_event_object (py_type));
>
> (which I agree with).
I guess you may have wanted to quote the other hunk above,
since I didn't touch that line.
Yeah, I definitely prefer copy-init when possible. In this
case I'd prefer doing that as a separate patch though, because
that's not something that the regression fix is adding, and
also grepping around I see there's a lot of similar instances
that can all get the same treatment. I'll post a patch
for that as follow up.
> Proposed changes:
>
Here's the two patches combined. Should I merge it?
From aa0125e4c1fe4021aa4087cec5324ecc829c7fc3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 24 Aug 2018 20:03:57 +0100
Subject: [PATCH] Fix 8.2 regression in gdb.python/py-evthreads.exp w/
gdbserver (PR gdb/23379)
This commit fixes a 8.1->8.2 regression exposed by
gdb.python/py-evthreads.exp when testing with
--target_board=native-gdbserver.
gdb.log shows:
src/gdb/thread.c:93: internal-error: thread_info* inferior_thread(): Assertion `tp' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) FAIL: gdb.python/py-evthreads.exp: run to breakpoint 1 (GDB internal error)
A backtrace shows (frames #2 and #10 highlighted) that the assertion
fails when GDB is setting up the connection to the remote target, in
non-stop mode:
#0 0x0000000000622ff0 in internal_error(char const*, int, char const*, ...) (file=0xc1ad98 "src/gdb/thread.c", line=93, fmt=0xc1ad20 "%s: Assertion `%s' failed.") at src/gdb/common/errors.c:54
#1 0x000000000089567e in inferior_thread() () at src/gdb/thread.c:93
= #2 0x00000000004da91d in get_event_thread() () at src/gdb/python/py-threadevent.c:38
#3 0x00000000004da9b7 in create_thread_event_object(_typeobject*, _object*) (py_type=0x11574c0 <continue_event_object_type>, thread=0x0)
at src/gdb/python/py-threadevent.c:60
#4 0x00000000004bf6fe in create_continue_event_object() () at src/gdb/python/py-continueevent.c:27
#5 0x00000000004bf738 in emit_continue_event(ptid_t) (ptid=...) at src/gdb/python/py-continueevent.c:40
#6 0x00000000004c7d47 in python_on_resume(ptid_t) (ptid=...) at src/gdb/python/py-inferior.c:108
#7 0x0000000000485bfb in std::_Function_handler<void (ptid_t), void (*)(ptid_t)>::_M_invoke(std::_Any_data const&, ptid_t&&) (__functor=..., __args#0=...) at /usr/include/c++/7/bits/std_function.h:316
#8 0x000000000089b416 in std::function<void (ptid_t)>::operator()(ptid_t) const (this=0x12aa600, __args#0=...)
at /usr/include/c++/7/bits/std_function.h:706
#9 0x000000000089aa0e in gdb::observers::observable<ptid_t>::notify(ptid_t) const (this=0x118a7a0 <gdb::observers::target_resumed>, args#0=...)
at src/gdb/common/observable.h:106
= #10 0x0000000000896fbe in set_running(ptid_t, int) (ptid=..., running=1) at src/gdb/thread.c:880
#11 0x00000000007f750f in remote_target::remote_add_thread(ptid_t, bool, bool) (this=0x12c5440, ptid=..., running=true, executing=true) at src/gdb/remote.c:2434
#12 0x00000000007f779d in remote_target::remote_notice_new_inferior(ptid_t, int) (this=0x12c5440, currthread=..., executing=1)
at src/gdb/remote.c:2515
#13 0x00000000007f9c44 in remote_target::update_thread_list() (this=0x12c5440) at src/gdb/remote.c:3831
#14 0x00000000007fb922 in remote_target::start_remote(int, int) (this=0x12c5440, from_tty=0, extended_p=0)
at src/gdb/remote.c:4655
#15 0x00000000007fd102 in remote_target::open_1(char const*, int, int) (name=0x1a4f45e "localhost:2346", from_tty=0, extended_p=0)
at src/gdb/remote.c:5638
#16 0x00000000007fbec1 in remote_target::open(char const*, int) (name=0x1a4f45e "localhost:2346", from_tty=0)
at src/gdb/remote.c:4862
So on frame #10, we're marking a newly-discovered thread as running,
and that causes the Python API to emit a gdb.ContinueEvent.
gdb.ContinueEvent is a gdb.ThreadEvent, and as such includes the event
thread as the "inferior_thread" attribute. The problem is that when
we get to frame #3/#4, we lost all references to the thread that is
being marked as running. create_continue_event_object assumes that it
is the current thread, which is not true in this case.
Fix this by passing down the right thread in
create_continue_event_object. Also remove
create_thread_event_object's default argument and have the only other
caller left pass down the right thread explicitly too.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
Simon Marchi <simon.marchi@ericsson.com>
PR gdb/23379
* python/py-continueevent.c: Include "gdbthread.h".
(create_continue_event_object): Add intro comment. Add 'ptid'
parameter. Use it to find thread to pass to
create_thread_event_object.
(emit_continue_event): Pass PTID down to
create_continue_event_object.
* python/py-event.h (py_get_event_thread): Declare.
(create_thread_event_object): Remove default from 'thread'
parameter.
* python/py-stopevent.c (create_stop_event_object): Use
py_get_event_thread.
* python/py-threadevent.c (get_event_thread): Rename to ...
(py_get_event_thread): ... this, make extern, add 'ptid' parameter
and use it to find the thread.
(create_thread_event_object): Assert that THREAD isn't null.
Don't find the event thread here.
---
gdb/python/py-continueevent.c | 19 ++++++++++++++++---
gdb/python/py-event.h | 12 +++++++++++-
gdb/python/py-stopevent.c | 3 ++-
gdb/python/py-threadevent.c | 36 ++++++++++++++----------------------
4 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
index e5e922f0155..b047a83522b 100644
--- a/gdb/python/py-continueevent.c
+++ b/gdb/python/py-continueevent.c
@@ -20,11 +20,24 @@
#include "defs.h"
#include "py-event.h"
#include "py-ref.h"
+#include "gdbthread.h"
+
+/* Create a gdb.ContinueEvent event. gdb.ContinueEvent is-a
+ gdb.ThreadEvent, and thread events can either be thread specific or
+ process wide. If gdb is running in non-stop mode then the event is
+ thread specific (in which case the PTID thread is included in the
+ event), otherwise it is process wide (in which case PTID is
+ ignored). In either case a new reference is returned. */
static gdbpy_ref<>
-create_continue_event_object (void)
+create_continue_event_object (ptid_t ptid)
{
- return create_thread_event_object (&continue_event_object_type);
+ PyObject *py_thr = py_get_event_thread (ptid);
+
+ if (py_thr == nullptr)
+ return nullptr;
+
+ return create_thread_event_object (&continue_event_object_type, py_thr);
}
/* Callback function which notifies observers when a continue event occurs.
@@ -37,7 +50,7 @@ emit_continue_event (ptid_t ptid)
if (evregpy_no_listeners_p (gdb_py_events.cont))
return 0;
- gdbpy_ref<> event (create_continue_event_object ());
+ gdbpy_ref<> event (create_continue_event_object (ptid));
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.cont);
return -1;
diff --git a/gdb/python/py-event.h b/gdb/python/py-event.h
index 22bab108ed1..56003e8785f 100644
--- a/gdb/python/py-event.h
+++ b/gdb/python/py-event.h
@@ -63,8 +63,18 @@ extern int evpy_emit_event (PyObject *event,
eventregistry_object *registry);
extern gdbpy_ref<> create_event_object (PyTypeObject *py_type);
+
+/* thread events can either be thread specific or process wide. If gdb is
+ running in non-stop mode then the event is thread specific, otherwise
+ it is process wide.
+ This function returns the currently stopped thread in non-stop mode and
+ Py_None otherwise. In each case it returns a borrowed reference. */
+extern PyObject *py_get_event_thread (ptid_t ptid)
+ CPYCHECKER_RETURNS_BORROWED_REF;
+
extern gdbpy_ref<> create_thread_event_object (PyTypeObject *py_type,
- PyObject *thread = nullptr);
+ PyObject *thread);
+
extern int emit_new_objfile_event (struct objfile *objfile);
extern int emit_clear_objfiles_event (void);
diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
index 884e6e3ef8f..f6bd207f3b8 100644
--- a/gdb/python/py-stopevent.c
+++ b/gdb/python/py-stopevent.c
@@ -23,7 +23,8 @@
gdbpy_ref<>
create_stop_event_object (PyTypeObject *py_type)
{
- return create_thread_event_object (py_type);
+ return create_thread_event_object (py_type,
+ py_get_event_thread (inferior_ptid));
}
/* Callback observers when a stop event occurs. This function will create a
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index d6aa9467b9a..1bc2fb06d54 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -20,48 +20,40 @@
#include "infrun.h"
#include "gdbthread.h"
-/* thread events can either be thread specific or process wide. If gdb is
- running in non-stop mode then the event is thread specific, otherwise
- it is process wide.
- This function returns the currently stopped thread in non-stop mode and
- Py_None otherwise. In each case it returns a borrowed reference. */
+/* See py-event.h. */
-static PyObject *get_event_thread (void)
- CPYCHECKER_RETURNS_BORROWED_REF;
-
-static PyObject *
-get_event_thread (void)
+PyObject *
+py_get_event_thread (ptid_t ptid)
{
- PyObject *thread;
+ PyObject *pythread;
if (non_stop)
- thread = (PyObject *) thread_to_thread_object (inferior_thread ());
+ {
+ thread_info *thread = find_thread_ptid (ptid);
+ if (thread != nullptr)
+ pythread = (PyObject *) thread_to_thread_object (thread);
+ }
else
- thread = Py_None;
+ pythread = Py_None;
- if (!thread)
+ if (!pythread)
{
PyErr_SetString (PyExc_RuntimeError, "Could not find event thread");
return NULL;
}
- return thread;
+ return pythread;
}
gdbpy_ref<>
create_thread_event_object (PyTypeObject *py_type, PyObject *thread)
{
+ gdb_assert (thread != NULL);
+
gdbpy_ref<> thread_event_obj (create_event_object (py_type));
if (thread_event_obj == NULL)
return NULL;
- if (thread == NULL)
- {
- thread = get_event_thread ();
- if (!thread)
- return NULL;
- }
-
if (evpy_add_attribute (thread_event_obj.get (),
"inferior_thread",
thread) < 0)
--
2.14.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
2018-08-24 19:44 ` [PATCH v2] " Pedro Alves
@ 2018-08-24 19:47 ` Pedro Alves
2018-08-24 20:06 ` Simon Marchi
2018-08-24 20:02 ` [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379) Simon Marchi
2018-08-25 14:52 ` Tom Tromey
2 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2018-08-24 19:47 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 08/24/2018 08:44 PM, Pedro Alves wrote:
>
> Yeah, I definitely prefer copy-init when possible. In this
> case I'd prefer doing that as a separate patch though, because
> that's not something that the regression fix is adding, and
> also grepping around I see there's a lot of similar instances
> that can all get the same treatment. I'll post a patch
> for that as follow up.
Here's what I mean.
From bc3b8da0180c6ef782d3e38109b6bcb49b3664e1 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 24 Aug 2018 20:09:19 +0100
Subject: [PATCH] use copy-init more
---
gdb/python/py-bpevent.c | 2 +-
gdb/python/py-continueevent.c | 2 +-
gdb/python/py-exitedevent.c | 8 ++++----
gdb/python/py-inferior.c | 8 ++++----
gdb/python/py-infevents.c | 22 +++++++++++-----------
gdb/python/py-newobjfileevent.c | 24 ++++++++++--------------
gdb/python/py-signalevent.c | 5 ++---
gdb/python/py-threadevent.c | 2 +-
8 files changed, 34 insertions(+), 39 deletions(-)
diff --git a/gdb/python/py-bpevent.c b/gdb/python/py-bpevent.c
index 03863e4c9a2..f6f3c4c2378 100644
--- a/gdb/python/py-bpevent.c
+++ b/gdb/python/py-bpevent.c
@@ -28,7 +28,7 @@ gdbpy_ref<>
create_breakpoint_event_object (PyObject *breakpoint_list, PyObject *first_bp)
{
gdbpy_ref<> breakpoint_event_obj
- (create_stop_event_object (&breakpoint_event_object_type));
+ = create_stop_event_object (&breakpoint_event_object_type);
if (breakpoint_event_obj == NULL)
return NULL;
diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
index b047a83522b..759b4831366 100644
--- a/gdb/python/py-continueevent.c
+++ b/gdb/python/py-continueevent.c
@@ -50,7 +50,7 @@ emit_continue_event (ptid_t ptid)
if (evregpy_no_listeners_p (gdb_py_events.cont))
return 0;
- gdbpy_ref<> event (create_continue_event_object (ptid));
+ gdbpy_ref<> event = create_continue_event_object (ptid);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.cont);
return -1;
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 6a7c9f51fa1..22c095c7c79 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -20,10 +20,10 @@
#include "defs.h"
#include "py-event.h"
-static PyObject *
+static gdbpy_ref<>
create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
{
- gdbpy_ref<> exited_event (create_event_object (&exited_event_object_type));
+ gdbpy_ref<> exited_event = create_event_object (&exited_event_object_type);
if (exited_event == NULL)
return NULL;
@@ -45,7 +45,7 @@ create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
(PyObject *) inf_obj.get ()) < 0)
return NULL;
- return exited_event.release ();
+ return exited_event;
}
/* Callback that is used when an exit event occurs. This function
@@ -57,7 +57,7 @@ emit_exited_event (const LONGEST *exit_code, struct inferior *inf)
if (evregpy_no_listeners_p (gdb_py_events.exited))
return 0;
- gdbpy_ref<> event (create_exited_event_object (exit_code, inf));
+ gdbpy_ref<> event = create_exited_event_object (exit_code, inf);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.exited);
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 9f09ae93756..1cf37296973 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -256,7 +256,7 @@ python_new_inferior (struct inferior *inf)
return;
}
- gdbpy_ref<> event (create_event_object (&new_inferior_event_object_type));
+ gdbpy_ref<> event = create_event_object (&new_inferior_event_object_type);
if (event == NULL
|| evpy_add_attribute (event.get (), "inferior",
(PyObject *) inf_obj.get ()) < 0
@@ -284,7 +284,7 @@ python_inferior_deleted (struct inferior *inf)
return;
}
- gdbpy_ref<> event (create_event_object (&inferior_deleted_event_object_type));
+ gdbpy_ref<> event = create_event_object (&inferior_deleted_event_object_type);
if (event == NULL
|| evpy_add_attribute (event.get (), "inferior",
(PyObject *) inf_obj.get ()) < 0
@@ -353,8 +353,8 @@ add_thread_object (struct thread_info *tp)
if (evregpy_no_listeners_p (gdb_py_events.new_thread))
return;
- gdbpy_ref<> event (create_thread_event_object (&new_thread_event_object_type,
- (PyObject *) thread_obj));
+ gdbpy_ref<> event = create_thread_event_object (&new_thread_event_object_type,
+ (PyObject *) thread_obj);
if (event == NULL
|| evpy_emit_event (event.get (), gdb_py_events.new_thread) < 0)
gdbpy_print_stack ();
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index 2bd0bc7f2c4..0df7d96df30 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -24,7 +24,7 @@
/* Construct either a gdb.InferiorCallPreEvent or a
gdb.InferiorCallPostEvent. */
-static PyObject *
+static gdbpy_ref<>
create_inferior_call_event_object (inferior_call_kind flag, ptid_t ptid,
CORE_ADDR addr)
{
@@ -56,17 +56,17 @@ create_inferior_call_event_object (inferior_call_kind flag, ptid_t ptid,
if (evpy_add_attribute (event.get (), "address", addr_obj.get ()) < 0)
return NULL;
- return event.release ();
+ return event;
}
/* Construct a gdb.RegisterChangedEvent containing the affected
register number. */
-static PyObject *
+static gdbpy_ref<>
create_register_changed_event_object (struct frame_info *frame,
int regnum)
{
- gdbpy_ref<> event (create_event_object (®ister_changed_event_object_type));
+ gdbpy_ref<> event = create_event_object (®ister_changed_event_object_type);
if (event == NULL)
return NULL;
@@ -84,16 +84,16 @@ create_register_changed_event_object (struct frame_info *frame,
if (evpy_add_attribute (event.get (), "regnum", regnum_obj.get ()) < 0)
return NULL;
- return event.release ();
+ return event;
}
/* Construct a gdb.MemoryChangedEvent describing the extent of the
affected memory. */
-static PyObject *
+static gdbpy_ref<>
create_memory_changed_event_object (CORE_ADDR addr, ssize_t len)
{
- gdbpy_ref<> event (create_event_object (&memory_changed_event_object_type));
+ gdbpy_ref<> event = create_event_object (&memory_changed_event_object_type);
if (event == NULL)
return NULL;
@@ -112,7 +112,7 @@ create_memory_changed_event_object (CORE_ADDR addr, ssize_t len)
if (evpy_add_attribute (event.get (), "length", len_obj.get ()) < 0)
return NULL;
- return event.release ();
+ return event;
}
/* Callback function which notifies observers when an event occurs which
@@ -127,7 +127,7 @@ emit_inferior_call_event (inferior_call_kind flag, ptid_t thread,
if (evregpy_no_listeners_p (gdb_py_events.inferior_call))
return 0;
- gdbpy_ref<> event (create_inferior_call_event_object (flag, thread, addr));
+ gdbpy_ref<> event = create_inferior_call_event_object (flag, thread, addr);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.inferior_call);
return -1;
@@ -142,7 +142,7 @@ emit_memory_changed_event (CORE_ADDR addr, ssize_t len)
if (evregpy_no_listeners_p (gdb_py_events.memory_changed))
return 0;
- gdbpy_ref<> event (create_memory_changed_event_object (addr, len));
+ gdbpy_ref<> event = create_memory_changed_event_object (addr, len);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.memory_changed);
return -1;
@@ -157,7 +157,7 @@ emit_register_changed_event (struct frame_info* frame, int regnum)
if (evregpy_no_listeners_p (gdb_py_events.register_changed))
return 0;
- gdbpy_ref<> event (create_register_changed_event_object (frame, regnum));
+ gdbpy_ref<> event = create_register_changed_event_object (frame, regnum);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.register_changed);
return -1;
diff --git a/gdb/python/py-newobjfileevent.c b/gdb/python/py-newobjfileevent.c
index 86de9ac10ad..a9341a3be18 100644
--- a/gdb/python/py-newobjfileevent.c
+++ b/gdb/python/py-newobjfileevent.c
@@ -20,25 +20,23 @@
#include "defs.h"
#include "py-event.h"
-static PyObject *
+static gdbpy_ref<>
create_new_objfile_event_object (struct objfile *objfile)
{
- PyObject *py_objfile;
-
gdbpy_ref<> objfile_event
- (create_event_object (&new_objfile_event_object_type));
+ = create_event_object (&new_objfile_event_object_type);
if (objfile_event == NULL)
return NULL;
/* Note that objfile_to_objfile_object returns a borrowed reference,
so we don't need a decref here. */
- py_objfile = objfile_to_objfile_object (objfile);
+ PyObject *py_objfile = objfile_to_objfile_object (objfile);
if (!py_objfile || evpy_add_attribute (objfile_event.get (),
"new_objfile",
py_objfile) < 0)
return NULL;
- return objfile_event.release ();
+ return objfile_event;
}
/* Callback function which notifies observers when a new objfile event occurs.
@@ -51,7 +49,7 @@ emit_new_objfile_event (struct objfile *objfile)
if (evregpy_no_listeners_p (gdb_py_events.new_objfile))
return 0;
- gdbpy_ref<> event (create_new_objfile_event_object (objfile));
+ gdbpy_ref<> event = create_new_objfile_event_object (objfile);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.new_objfile);
return -1;
@@ -60,25 +58,23 @@ emit_new_objfile_event (struct objfile *objfile)
\f
/* Subroutine of emit_clear_objfiles_event to simplify it. */
-static PyObject *
+static gdbpy_ref<>
create_clear_objfiles_event_object (void)
{
- PyObject *py_progspace;
-
gdbpy_ref<> objfile_event
- (create_event_object (&clear_objfiles_event_object_type));
+ = create_event_object (&clear_objfiles_event_object_type);
if (objfile_event == NULL)
return NULL;
/* Note that pspace_to_pspace_object returns a borrowed reference,
so we don't need a decref here. */
- py_progspace = pspace_to_pspace_object (current_program_space);
+ PyObject *py_progspace = pspace_to_pspace_object (current_program_space);
if (!py_progspace || evpy_add_attribute (objfile_event.get (),
"progspace",
py_progspace) < 0)
return NULL;
- return objfile_event.release ();
+ return objfile_event;
}
/* Callback function which notifies observers when the "clear objfiles"
@@ -92,7 +88,7 @@ emit_clear_objfiles_event (void)
if (evregpy_no_listeners_p (gdb_py_events.clear_objfiles))
return 0;
- gdbpy_ref<> event (create_clear_objfiles_event_object ());
+ gdbpy_ref<> event = create_clear_objfiles_event_object ();
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.clear_objfiles);
return -1;
diff --git a/gdb/python/py-signalevent.c b/gdb/python/py-signalevent.c
index d88aaeaedea..a2e7519460c 100644
--- a/gdb/python/py-signalevent.c
+++ b/gdb/python/py-signalevent.c
@@ -23,14 +23,13 @@
gdbpy_ref<>
create_signal_event_object (enum gdb_signal stop_signal)
{
- const char *signal_name;
gdbpy_ref<> signal_event_obj
- (create_stop_event_object (&signal_event_object_type));
+ = create_stop_event_object (&signal_event_object_type);
if (signal_event_obj == NULL)
return NULL;
- signal_name = gdb_signal_to_name (stop_signal);
+ const char *signal_name = gdb_signal_to_name (stop_signal);
gdbpy_ref<> signal_name_obj (PyString_FromString (signal_name));
if (signal_name_obj == NULL)
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index 1bc2fb06d54..a78f0a38310 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -50,7 +50,7 @@ create_thread_event_object (PyTypeObject *py_type, PyObject *thread)
{
gdb_assert (thread != NULL);
- gdbpy_ref<> thread_event_obj (create_event_object (py_type));
+ gdbpy_ref<> thread_event_obj = create_event_object (py_type);
if (thread_event_obj == NULL)
return NULL;
--
2.14.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
2018-08-24 19:44 ` [PATCH v2] " Pedro Alves
2018-08-24 19:47 ` Pedro Alves
@ 2018-08-24 20:02 ` Simon Marchi
2018-08-25 14:52 ` Tom Tromey
2 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2018-08-24 20:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: Simon Marchi, gdb-patches
On 2018-08-24 15:44, Pedro Alves wrote:
> Here's the two patches combined. Should I merge it?
LGTM, thanks!
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
2018-08-24 19:47 ` Pedro Alves
@ 2018-08-24 20:06 ` Simon Marchi
2018-08-24 22:13 ` [pushed] gdb/python: Use copy-initialization more when possible Pedro Alves
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2018-08-24 20:06 UTC (permalink / raw)
To: Pedro Alves; +Cc: Simon Marchi, gdb-patches
On 2018-08-24 15:47, Pedro Alves wrote:
> On 08/24/2018 08:44 PM, Pedro Alves wrote:
>
>>
>> Yeah, I definitely prefer copy-init when possible. In this
>> case I'd prefer doing that as a separate patch though, because
>> that's not something that the regression fix is adding, and
>> also grepping around I see there's a lot of similar instances
>> that can all get the same treatment. I'll post a patch
>> for that as follow up.
> Here's what I mean.
Indeed, in the previous patch I just meant you could change the line you
were touching (and indeed quoted the wrong hunk), but this more
far-reaching cleanup is great too. I wouldn't put this one in the 8.2
branch though, since it doesn't actually fix anything.
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pushed] gdb/python: Use copy-initialization more when possible
2018-08-24 20:06 ` Simon Marchi
@ 2018-08-24 22:13 ` Pedro Alves
0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2018-08-24 22:13 UTC (permalink / raw)
To: Simon Marchi; +Cc: Simon Marchi, gdb-patches
On 08/24/2018 09:06 PM, Simon Marchi wrote:
> On 2018-08-24 15:47, Pedro Alves wrote:
>> On 08/24/2018 08:44 PM, Pedro Alves wrote:
>>
>>>
>>> Yeah, I definitely prefer copy-init when possible. In this
>>> case I'd prefer doing that as a separate patch though, because
>>> that's not something that the regression fix is adding, and
>>> also grepping around I see there's a lot of similar instances
>>> that can all get the same treatment. I'll post a patch
>>> for that as follow up.
>> Here's what I mean.
>
> Indeed, in the previous patch I just meant you could change the line you were touching (and indeed quoted the wrong hunk), but this more far-reaching cleanup is great too. I wouldn't put this one in the 8.2 branch though, since it doesn't actually fix anything.
Agreed. I've merged this to master now.
From d98fc15be2b8dd5699c1852db7d9d979231123dc Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 24 Aug 2018 20:09:19 +0100
Subject: [PATCH] gdb/python: Use copy-initialization more when possible
gdb/ChangeLog:
2018-08-24 Pedro Alves <palves@redhat.com>
* python/py-bpevent.c (create_breakpoint_event_object): Use
copy-initialization.
* python/py-continueevent.c (emit_continue_event): Use
copy-initialization.
* python/py-exitedevent.c (create_exited_event_object): Return a
gdbpy_ref<>.
(emit_exited_event): Use copy-initialization.
* python/py-inferior.c (python_new_inferior)
(python_inferior_deleted, add_thread_object): Use
copy-initialization.
* python/py-infevents.c (create_inferior_call_event_object)
(create_register_changed_event_object)
(create_memory_changed_event_object): Return a gdbpy_ref<>.
(emit_inferior_call_event, emit_memory_changed_event)
(emit_register_changed_event): Use copy-initialization.
* python/py-newobjfileevent.c (create_new_objfile_event_object):
Return a gdbpy_ref<>.
(emit_new_objfile_event): Use copy-initialization.
(create_clear_objfiles_event_object): Return a gdbpy_ref<>.
(emit_clear_objfiles_event): Use copy-initialization.
* python/py-signalevent.c (create_signal_event_object): Use
copy-initialization.
* python/py-threadevent.c (create_thread_event_object): Use
copy-initialization.
---
gdb/ChangeLog | 27 +++++++++++++++++++++++++++
gdb/python/py-bpevent.c | 2 +-
gdb/python/py-continueevent.c | 2 +-
gdb/python/py-exitedevent.c | 8 ++++----
gdb/python/py-inferior.c | 8 ++++----
gdb/python/py-infevents.c | 22 +++++++++++-----------
gdb/python/py-newobjfileevent.c | 24 ++++++++++--------------
gdb/python/py-signalevent.c | 5 ++---
gdb/python/py-threadevent.c | 2 +-
9 files changed, 61 insertions(+), 39 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e1bdd392b1b..12dda6c541d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,30 @@
+2018-08-24 Pedro Alves <palves@redhat.com>
+
+ * python/py-bpevent.c (create_breakpoint_event_object): Use
+ copy-initialization.
+ * python/py-continueevent.c (emit_continue_event): Use
+ copy-initialization.
+ * python/py-exitedevent.c (create_exited_event_object): Return a
+ gdbpy_ref<>.
+ (emit_exited_event): Use copy-initialization.
+ * python/py-inferior.c (python_new_inferior)
+ (python_inferior_deleted, add_thread_object): Use
+ copy-initialization.
+ * python/py-infevents.c (create_inferior_call_event_object)
+ (create_register_changed_event_object)
+ (create_memory_changed_event_object): Return a gdbpy_ref<>.
+ (emit_inferior_call_event, emit_memory_changed_event)
+ (emit_register_changed_event): Use copy-initialization.
+ * python/py-newobjfileevent.c (create_new_objfile_event_object):
+ Return a gdbpy_ref<>.
+ (emit_new_objfile_event): Use copy-initialization.
+ (create_clear_objfiles_event_object): Return a gdbpy_ref<>.
+ (emit_clear_objfiles_event): Use copy-initialization.
+ * python/py-signalevent.c (create_signal_event_object): Use
+ copy-initialization.
+ * python/py-threadevent.c (create_thread_event_object): Use
+ copy-initialization.
+
2018-08-24 Pedro Alves <palves@redhat.com>
Simon Marchi <simon.marchi@ericsson.com>
diff --git a/gdb/python/py-bpevent.c b/gdb/python/py-bpevent.c
index 03863e4c9a2..f6f3c4c2378 100644
--- a/gdb/python/py-bpevent.c
+++ b/gdb/python/py-bpevent.c
@@ -28,7 +28,7 @@ gdbpy_ref<>
create_breakpoint_event_object (PyObject *breakpoint_list, PyObject *first_bp)
{
gdbpy_ref<> breakpoint_event_obj
- (create_stop_event_object (&breakpoint_event_object_type));
+ = create_stop_event_object (&breakpoint_event_object_type);
if (breakpoint_event_obj == NULL)
return NULL;
diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
index b047a83522b..759b4831366 100644
--- a/gdb/python/py-continueevent.c
+++ b/gdb/python/py-continueevent.c
@@ -50,7 +50,7 @@ emit_continue_event (ptid_t ptid)
if (evregpy_no_listeners_p (gdb_py_events.cont))
return 0;
- gdbpy_ref<> event (create_continue_event_object (ptid));
+ gdbpy_ref<> event = create_continue_event_object (ptid);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.cont);
return -1;
diff --git a/gdb/python/py-exitedevent.c b/gdb/python/py-exitedevent.c
index 6a7c9f51fa1..22c095c7c79 100644
--- a/gdb/python/py-exitedevent.c
+++ b/gdb/python/py-exitedevent.c
@@ -20,10 +20,10 @@
#include "defs.h"
#include "py-event.h"
-static PyObject *
+static gdbpy_ref<>
create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
{
- gdbpy_ref<> exited_event (create_event_object (&exited_event_object_type));
+ gdbpy_ref<> exited_event = create_event_object (&exited_event_object_type);
if (exited_event == NULL)
return NULL;
@@ -45,7 +45,7 @@ create_exited_event_object (const LONGEST *exit_code, struct inferior *inf)
(PyObject *) inf_obj.get ()) < 0)
return NULL;
- return exited_event.release ();
+ return exited_event;
}
/* Callback that is used when an exit event occurs. This function
@@ -57,7 +57,7 @@ emit_exited_event (const LONGEST *exit_code, struct inferior *inf)
if (evregpy_no_listeners_p (gdb_py_events.exited))
return 0;
- gdbpy_ref<> event (create_exited_event_object (exit_code, inf));
+ gdbpy_ref<> event = create_exited_event_object (exit_code, inf);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.exited);
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 9f09ae93756..1cf37296973 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -256,7 +256,7 @@ python_new_inferior (struct inferior *inf)
return;
}
- gdbpy_ref<> event (create_event_object (&new_inferior_event_object_type));
+ gdbpy_ref<> event = create_event_object (&new_inferior_event_object_type);
if (event == NULL
|| evpy_add_attribute (event.get (), "inferior",
(PyObject *) inf_obj.get ()) < 0
@@ -284,7 +284,7 @@ python_inferior_deleted (struct inferior *inf)
return;
}
- gdbpy_ref<> event (create_event_object (&inferior_deleted_event_object_type));
+ gdbpy_ref<> event = create_event_object (&inferior_deleted_event_object_type);
if (event == NULL
|| evpy_add_attribute (event.get (), "inferior",
(PyObject *) inf_obj.get ()) < 0
@@ -353,8 +353,8 @@ add_thread_object (struct thread_info *tp)
if (evregpy_no_listeners_p (gdb_py_events.new_thread))
return;
- gdbpy_ref<> event (create_thread_event_object (&new_thread_event_object_type,
- (PyObject *) thread_obj));
+ gdbpy_ref<> event = create_thread_event_object (&new_thread_event_object_type,
+ (PyObject *) thread_obj);
if (event == NULL
|| evpy_emit_event (event.get (), gdb_py_events.new_thread) < 0)
gdbpy_print_stack ();
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index 2bd0bc7f2c4..0df7d96df30 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -24,7 +24,7 @@
/* Construct either a gdb.InferiorCallPreEvent or a
gdb.InferiorCallPostEvent. */
-static PyObject *
+static gdbpy_ref<>
create_inferior_call_event_object (inferior_call_kind flag, ptid_t ptid,
CORE_ADDR addr)
{
@@ -56,17 +56,17 @@ create_inferior_call_event_object (inferior_call_kind flag, ptid_t ptid,
if (evpy_add_attribute (event.get (), "address", addr_obj.get ()) < 0)
return NULL;
- return event.release ();
+ return event;
}
/* Construct a gdb.RegisterChangedEvent containing the affected
register number. */
-static PyObject *
+static gdbpy_ref<>
create_register_changed_event_object (struct frame_info *frame,
int regnum)
{
- gdbpy_ref<> event (create_event_object (®ister_changed_event_object_type));
+ gdbpy_ref<> event = create_event_object (®ister_changed_event_object_type);
if (event == NULL)
return NULL;
@@ -84,16 +84,16 @@ create_register_changed_event_object (struct frame_info *frame,
if (evpy_add_attribute (event.get (), "regnum", regnum_obj.get ()) < 0)
return NULL;
- return event.release ();
+ return event;
}
/* Construct a gdb.MemoryChangedEvent describing the extent of the
affected memory. */
-static PyObject *
+static gdbpy_ref<>
create_memory_changed_event_object (CORE_ADDR addr, ssize_t len)
{
- gdbpy_ref<> event (create_event_object (&memory_changed_event_object_type));
+ gdbpy_ref<> event = create_event_object (&memory_changed_event_object_type);
if (event == NULL)
return NULL;
@@ -112,7 +112,7 @@ create_memory_changed_event_object (CORE_ADDR addr, ssize_t len)
if (evpy_add_attribute (event.get (), "length", len_obj.get ()) < 0)
return NULL;
- return event.release ();
+ return event;
}
/* Callback function which notifies observers when an event occurs which
@@ -127,7 +127,7 @@ emit_inferior_call_event (inferior_call_kind flag, ptid_t thread,
if (evregpy_no_listeners_p (gdb_py_events.inferior_call))
return 0;
- gdbpy_ref<> event (create_inferior_call_event_object (flag, thread, addr));
+ gdbpy_ref<> event = create_inferior_call_event_object (flag, thread, addr);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.inferior_call);
return -1;
@@ -142,7 +142,7 @@ emit_memory_changed_event (CORE_ADDR addr, ssize_t len)
if (evregpy_no_listeners_p (gdb_py_events.memory_changed))
return 0;
- gdbpy_ref<> event (create_memory_changed_event_object (addr, len));
+ gdbpy_ref<> event = create_memory_changed_event_object (addr, len);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.memory_changed);
return -1;
@@ -157,7 +157,7 @@ emit_register_changed_event (struct frame_info* frame, int regnum)
if (evregpy_no_listeners_p (gdb_py_events.register_changed))
return 0;
- gdbpy_ref<> event (create_register_changed_event_object (frame, regnum));
+ gdbpy_ref<> event = create_register_changed_event_object (frame, regnum);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.register_changed);
return -1;
diff --git a/gdb/python/py-newobjfileevent.c b/gdb/python/py-newobjfileevent.c
index 86de9ac10ad..a9341a3be18 100644
--- a/gdb/python/py-newobjfileevent.c
+++ b/gdb/python/py-newobjfileevent.c
@@ -20,25 +20,23 @@
#include "defs.h"
#include "py-event.h"
-static PyObject *
+static gdbpy_ref<>
create_new_objfile_event_object (struct objfile *objfile)
{
- PyObject *py_objfile;
-
gdbpy_ref<> objfile_event
- (create_event_object (&new_objfile_event_object_type));
+ = create_event_object (&new_objfile_event_object_type);
if (objfile_event == NULL)
return NULL;
/* Note that objfile_to_objfile_object returns a borrowed reference,
so we don't need a decref here. */
- py_objfile = objfile_to_objfile_object (objfile);
+ PyObject *py_objfile = objfile_to_objfile_object (objfile);
if (!py_objfile || evpy_add_attribute (objfile_event.get (),
"new_objfile",
py_objfile) < 0)
return NULL;
- return objfile_event.release ();
+ return objfile_event;
}
/* Callback function which notifies observers when a new objfile event occurs.
@@ -51,7 +49,7 @@ emit_new_objfile_event (struct objfile *objfile)
if (evregpy_no_listeners_p (gdb_py_events.new_objfile))
return 0;
- gdbpy_ref<> event (create_new_objfile_event_object (objfile));
+ gdbpy_ref<> event = create_new_objfile_event_object (objfile);
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.new_objfile);
return -1;
@@ -60,25 +58,23 @@ emit_new_objfile_event (struct objfile *objfile)
\f
/* Subroutine of emit_clear_objfiles_event to simplify it. */
-static PyObject *
+static gdbpy_ref<>
create_clear_objfiles_event_object (void)
{
- PyObject *py_progspace;
-
gdbpy_ref<> objfile_event
- (create_event_object (&clear_objfiles_event_object_type));
+ = create_event_object (&clear_objfiles_event_object_type);
if (objfile_event == NULL)
return NULL;
/* Note that pspace_to_pspace_object returns a borrowed reference,
so we don't need a decref here. */
- py_progspace = pspace_to_pspace_object (current_program_space);
+ PyObject *py_progspace = pspace_to_pspace_object (current_program_space);
if (!py_progspace || evpy_add_attribute (objfile_event.get (),
"progspace",
py_progspace) < 0)
return NULL;
- return objfile_event.release ();
+ return objfile_event;
}
/* Callback function which notifies observers when the "clear objfiles"
@@ -92,7 +88,7 @@ emit_clear_objfiles_event (void)
if (evregpy_no_listeners_p (gdb_py_events.clear_objfiles))
return 0;
- gdbpy_ref<> event (create_clear_objfiles_event_object ());
+ gdbpy_ref<> event = create_clear_objfiles_event_object ();
if (event != NULL)
return evpy_emit_event (event.get (), gdb_py_events.clear_objfiles);
return -1;
diff --git a/gdb/python/py-signalevent.c b/gdb/python/py-signalevent.c
index d88aaeaedea..a2e7519460c 100644
--- a/gdb/python/py-signalevent.c
+++ b/gdb/python/py-signalevent.c
@@ -23,14 +23,13 @@
gdbpy_ref<>
create_signal_event_object (enum gdb_signal stop_signal)
{
- const char *signal_name;
gdbpy_ref<> signal_event_obj
- (create_stop_event_object (&signal_event_object_type));
+ = create_stop_event_object (&signal_event_object_type);
if (signal_event_obj == NULL)
return NULL;
- signal_name = gdb_signal_to_name (stop_signal);
+ const char *signal_name = gdb_signal_to_name (stop_signal);
gdbpy_ref<> signal_name_obj (PyString_FromString (signal_name));
if (signal_name_obj == NULL)
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index 1bc2fb06d54..a78f0a38310 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -50,7 +50,7 @@ create_thread_event_object (PyTypeObject *py_type, PyObject *thread)
{
gdb_assert (thread != NULL);
- gdbpy_ref<> thread_event_obj (create_event_object (py_type));
+ gdbpy_ref<> thread_event_obj = create_event_object (py_type);
if (thread_event_obj == NULL)
return NULL;
--
2.14.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
2018-08-24 19:44 ` [PATCH v2] " Pedro Alves
2018-08-24 19:47 ` Pedro Alves
2018-08-24 20:02 ` [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379) Simon Marchi
@ 2018-08-25 14:52 ` Tom Tromey
2018-08-25 15:56 ` Simon Marchi
2 siblings, 1 reply; 9+ messages in thread
From: Tom Tromey @ 2018-08-25 14:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: Simon Marchi, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> * python/py-threadevent.c (get_event_thread): Rename to ...
Pedro> (py_get_event_thread): ... this, make extern, add 'ptid' parameter
I get this warning now:
../../binutils-gdb/gdb/python/py-threadevent.c: In function ‘PyObject* py_get_event_thread(ptid_t)’:
../../binutils-gdb/gdb/python/py-threadevent.c:39:3: warning: ‘pythread’ may be used uninitialized in this function [-Wmaybe-uninitialized]
... which seems legitimate to me:
PyObject *pythread;
if (non_stop)
{
thread_info *thread = find_thread_ptid (ptid);
if (thread != nullptr)
pythread = (PyObject *) thread_to_thread_object (thread);
// ... else pythread is uninitialized ...
}
pythread should be set to nullptr where I stuck the comment (or
initialized to nullptr).
Tom
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379)
2018-08-25 14:52 ` Tom Tromey
@ 2018-08-25 15:56 ` Simon Marchi
0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2018-08-25 15:56 UTC (permalink / raw)
To: Tom Tromey, Pedro Alves; +Cc: Simon Marchi, gdb-patches
On 2018-08-25 10:52 a.m., Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> * python/py-threadevent.c (get_event_thread): Rename to ...
> Pedro> (py_get_event_thread): ... this, make extern, add 'ptid' parameter
>
> I get this warning now:
>
> ../../binutils-gdb/gdb/python/py-threadevent.c: In function âPyObject* py_get_event_thread(ptid_t)â:
> ../../binutils-gdb/gdb/python/py-threadevent.c:39:3: warning: âpythreadâ may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> ... which seems legitimate to me:
>
> PyObject *pythread;
>
> if (non_stop)
> {
> thread_info *thread = find_thread_ptid (ptid);
> if (thread != nullptr)
> pythread = (PyObject *) thread_to_thread_object (thread);
> // ... else pythread is uninitialized ...
> }
>
> pythread should be set to nullptr where I stuck the comment (or
> initialized to nullptr).
>
> Tom
>
Huh indeed, thanks for pointing it out. I wonder why I don't get the warning with my gcc 8.2.0...
I pushed this patch to master and the 8.2 branch.
From bbbbbceebc342d583057a11d88bae85f451cd904 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sat, 25 Aug 2018 11:52:24 -0400
Subject: [PATCH] Initialize variable in py_get_event_thread
The pythread variable could be used without being initialized, fix it by
initializing it to nullptr.
gdb/ChangeLog:
* python/py-threadevent.c (py_get_event_thread): Initialize
pythread.
---
gdb/ChangeLog | 5 +++++
gdb/python/py-threadevent.c | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 12dda6c541d9..9e3d6bc27acd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-08-25 Simon Marchi <simon.marchi@polymtl.ca>
+
+ * python/py-threadevent.c (py_get_event_thread): Initialize
+ pythread.
+
2018-08-24 Pedro Alves <palves@redhat.com>
* python/py-bpevent.c (create_breakpoint_event_object): Use
diff --git a/gdb/python/py-threadevent.c b/gdb/python/py-threadevent.c
index a78f0a38310c..4f822b4ae09c 100644
--- a/gdb/python/py-threadevent.c
+++ b/gdb/python/py-threadevent.c
@@ -25,7 +25,7 @@
PyObject *
py_get_event_thread (ptid_t ptid)
{
- PyObject *pythread;
+ PyObject *pythread = nullptr;
if (non_stop)
{
@@ -36,7 +36,7 @@ py_get_event_thread (ptid_t ptid)
else
pythread = Py_None;
- if (!pythread)
+ if (pythread == nullptr)
{
PyErr_SetString (PyExc_RuntimeError, "Could not find event thread");
return NULL;
--
2.18.0
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-08-25 15:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-24 17:46 [PATCH] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379) Pedro Alves
2018-08-24 18:53 ` Simon Marchi
2018-08-24 19:44 ` [PATCH v2] " Pedro Alves
2018-08-24 19:47 ` Pedro Alves
2018-08-24 20:06 ` Simon Marchi
2018-08-24 22:13 ` [pushed] gdb/python: Use copy-initialization more when possible Pedro Alves
2018-08-24 20:02 ` [PATCH v2] Fix 8.2 regression in gdb.python/py-evthreads.exp w/ gdbserver (PR gdb/23379) Simon Marchi
2018-08-25 14:52 ` Tom Tromey
2018-08-25 15:56 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).