* [PATCH 2/3] Update target_stop's documentation
2014-09-18 9:59 [PATCH 0/3] target_{stop,continue} things Gary Benson
2014-09-18 9:59 ` [PATCH 3/3] Wait for target to stop before running agent commands Gary Benson
2014-09-18 9:59 ` [PATCH 1/3] Rename target_{stop,continue}_ptid Gary Benson
@ 2014-09-18 9:59 ` Gary Benson
2014-09-19 16:38 ` Pedro Alves
2 siblings, 1 reply; 8+ messages in thread
From: Gary Benson @ 2014-09-18 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Doug Evans
This commit updates target_stop's documentation to clarify that
it is asynchronous.
gdb/ChangeLog:
* target.c (target_stop): Updated comment.
---
gdb/ChangeLog | 4 ++++
gdb/target.h | 6 ++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/gdb/target.h b/gdb/target.h
index 2ea7a2d..24fb757 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1573,8 +1573,10 @@ extern int target_thread_alive (ptid_t ptid);
extern void target_find_new_threads (void);
/* Make target stop in a continuable fashion. (For instance, under
- Unix, this should act like SIGSTOP). This function is normally
- used by GUIs to implement a stop button. */
+ Unix, this should act like SIGSTOP). Note that this function is
+ asynchronous: it does not wait for the target to become stopped
+ before returning. If this is the behaviour you want please use
+ target_stop_and_wait. */
extern void target_stop (ptid_t ptid);
--
1.7.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] Rename target_{stop,continue}_ptid
2014-09-18 9:59 [PATCH 0/3] target_{stop,continue} things Gary Benson
2014-09-18 9:59 ` [PATCH 3/3] Wait for target to stop before running agent commands Gary Benson
@ 2014-09-18 9:59 ` Gary Benson
2014-09-19 16:16 ` Pedro Alves
2014-09-18 9:59 ` [PATCH 2/3] Update target_stop's documentation Gary Benson
2 siblings, 1 reply; 8+ messages in thread
From: Gary Benson @ 2014-09-18 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Doug Evans
This commit renames target_stop_ptid as target_stop_and_wait and
target_continue_ptid as target_continue_no_signal. Comments are
updated to more fully describe the functions' behaviour.
gdb/ChangeLog:
* target/target.h (target_stop_ptid): Renamed as...
(target_stop_and_wait): New function. Updated comment.
All uses updated.
(target_continue_ptid): Renamed as...
(target_continue_no_signal): New function. Updated comment.
All uses updated.
---
gdb/ChangeLog | 9 +++++++++
gdb/common/agent.c | 4 ++--
gdb/gdbserver/target.c | 4 ++--
gdb/target.c | 4 ++--
gdb/target/target.h | 16 +++++++++-------
5 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 854a997..0c3d7a3 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -204,7 +204,7 @@ agent_run_command (int pid, const char *cmd, int len)
DEBUG_AGENT ("agent: resumed helper thread\n");
/* Resume helper thread. */
- target_continue_ptid (ptid);
+ target_continue_no_signal (ptid);
fd = gdb_connect_sync_socket (pid);
if (fd >= 0)
@@ -238,7 +238,7 @@ agent_run_command (int pid, const char *cmd, int len)
{
/* Stop thread PTID. */
DEBUG_AGENT ("agent: stop helper thread\n");
- target_stop_ptid (ptid);
+ target_stop_and_wait (ptid);
}
if (fd >= 0)
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index c95b2f0..d63e211 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -137,7 +137,7 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
/* See target/target.h. */
void
-target_stop_ptid (ptid_t ptid)
+target_stop_and_wait (ptid_t ptid)
{
struct target_waitstatus status;
int was_non_stop = non_stop;
@@ -156,7 +156,7 @@ target_stop_ptid (ptid_t ptid)
/* See target/target.h. */
void
-target_continue_ptid (ptid_t ptid)
+target_continue_no_signal (ptid_t ptid)
{
struct thread_resume resume_info;
diff --git a/gdb/target.c b/gdb/target.c
index 339b1d1..c1b5182 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3030,7 +3030,7 @@ target_stop (ptid_t ptid)
/* See target/target.h. */
void
-target_stop_ptid (ptid_t ptid)
+target_stop_and_wait (ptid_t ptid)
{
struct target_waitstatus status;
int was_non_stop = non_stop;
@@ -3047,7 +3047,7 @@ target_stop_ptid (ptid_t ptid)
/* See target/target.h. */
void
-target_continue_ptid (ptid_t ptid)
+target_continue_no_signal (ptid_t ptid)
{
target_resume (ptid, 0, GDB_SIGNAL_0);
}
diff --git a/gdb/target/target.h b/gdb/target/target.h
index 99a7966..c8b9c80 100644
--- a/gdb/target/target.h
+++ b/gdb/target/target.h
@@ -59,15 +59,17 @@ extern int target_read_uint32 (CORE_ADDR memaddr, uint32_t *result);
extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
ssize_t len);
-/* Make target stop in a continuable fashion. (For instance, under
- Unix, this should act like SIGSTOP). This function must be
- provided by the client. */
+/* Cause the target to stop in a continuable fashion--for instance,
+ under Unix, this should act like SIGSTOP--and wait for the target
+ to be stopped before returning. This function must be provided by
+ the client. */
-extern void target_stop_ptid (ptid_t ptid);
+extern void target_stop_and_wait (ptid_t ptid);
-/* Restart a target that was previously stopped by target_stop_ptid.
- This function must be provided by the client. */
+/* Restart a target previously stopped by target_stop_and_wait.
+ No signal is delivered to the target. This function must be
+ provided by the client. */
-extern void target_continue_ptid (ptid_t ptid);
+extern void target_continue_no_signal (ptid_t ptid);
#endif /* TARGET_COMMON_H */
--
1.7.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/3] target_{stop,continue} things
@ 2014-09-18 9:59 Gary Benson
2014-09-18 9:59 ` [PATCH 3/3] Wait for target to stop before running agent commands Gary Benson
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Gary Benson @ 2014-09-18 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Doug Evans
Hi all,
This series fixes three issues raised in this thread:
https://sourceware.org/ml/gdb-patches/2014-09/msg00338.html
Patch 1 renames target_{stop,continue}_ptid, patch 2 updates
target_stop's documentation to indicate that it is asynchronous,
and patch 3 fixes a bug in linux-nat.c where agent commands are
run on inferiors that may not have stopped.
Build and regtested on x86_64 RHEL 6.5.
Is this ok to commit?
Thanks,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] Wait for target to stop before running agent commands
2014-09-18 9:59 [PATCH 0/3] target_{stop,continue} things Gary Benson
@ 2014-09-18 9:59 ` Gary Benson
2014-09-19 16:16 ` Pedro Alves
2014-09-18 9:59 ` [PATCH 1/3] Rename target_{stop,continue}_ptid Gary Benson
2014-09-18 9:59 ` [PATCH 2/3] Update target_stop's documentation Gary Benson
2 siblings, 1 reply; 8+ messages in thread
From: Gary Benson @ 2014-09-18 9:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Doug Evans
This commit fixes a bug on where agent_run_command is called on
GNU/Linux targets that may not have stopped yet.
gdb/ChangeLog:
* linux-nat.c (linux_child_static_tracepoint_markers_by_strid):
Use target_stop_and_wait in place of target_stop.
---
gdb/ChangeLog | 5 +++++
gdb/linux-nat.c | 2 +-
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 5a82d23..f2895ff 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4458,7 +4458,7 @@ linux_child_static_tracepoint_markers_by_strid (struct target_ops *self,
ptid_t ptid = ptid_build (pid, 0, 0);
/* Pause all */
- target_stop (ptid);
+ target_stop_and_wait (ptid);
memcpy (s, "qTfSTM", sizeof ("qTfSTM"));
s[sizeof ("qTfSTM")] = 0;
--
1.7.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] Rename target_{stop,continue}_ptid
2014-09-18 9:59 ` [PATCH 1/3] Rename target_{stop,continue}_ptid Gary Benson
@ 2014-09-19 16:16 ` Pedro Alves
0 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2014-09-19 16:16 UTC (permalink / raw)
To: Gary Benson, gdb-patches; +Cc: Doug Evans
On 09/18/2014 10:59 AM, Gary Benson wrote:
> This commit renames target_stop_ptid as target_stop_and_wait and
> target_continue_ptid as target_continue_no_signal. Comments are
> updated to more fully describe the functions' behaviour.
>
> gdb/ChangeLog:
>
> * target/target.h (target_stop_ptid): Renamed as...
> (target_stop_and_wait): New function. Updated comment.
> All uses updated.
> (target_continue_ptid): Renamed as...
> (target_continue_no_signal): New function. Updated comment.
> All uses updated.
Fine with me.
> ---
> gdb/ChangeLog | 9 +++++++++
> gdb/common/agent.c | 4 ++--
> gdb/gdbserver/target.c | 4 ++--
> gdb/target.c | 4 ++--
> gdb/target/target.h | 16 +++++++++-------
> 5 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/common/agent.c b/gdb/common/agent.c
> index 854a997..0c3d7a3 100644
> --- a/gdb/common/agent.c
> +++ b/gdb/common/agent.c
> @@ -204,7 +204,7 @@ agent_run_command (int pid, const char *cmd, int len)
> DEBUG_AGENT ("agent: resumed helper thread\n");
>
> /* Resume helper thread. */
> - target_continue_ptid (ptid);
> + target_continue_no_signal (ptid);
>
> fd = gdb_connect_sync_socket (pid);
> if (fd >= 0)
> @@ -238,7 +238,7 @@ agent_run_command (int pid, const char *cmd, int len)
> {
> /* Stop thread PTID. */
> DEBUG_AGENT ("agent: stop helper thread\n");
> - target_stop_ptid (ptid);
> + target_stop_and_wait (ptid);
> }
>
> if (fd >= 0)
> diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
> index c95b2f0..d63e211 100644
> --- a/gdb/gdbserver/target.c
> +++ b/gdb/gdbserver/target.c
> @@ -137,7 +137,7 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
> /* See target/target.h. */
>
> void
> -target_stop_ptid (ptid_t ptid)
> +target_stop_and_wait (ptid_t ptid)
> {
> struct target_waitstatus status;
> int was_non_stop = non_stop;
> @@ -156,7 +156,7 @@ target_stop_ptid (ptid_t ptid)
> /* See target/target.h. */
>
> void
> -target_continue_ptid (ptid_t ptid)
> +target_continue_no_signal (ptid_t ptid)
> {
> struct thread_resume resume_info;
>
> diff --git a/gdb/target.c b/gdb/target.c
> index 339b1d1..c1b5182 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -3030,7 +3030,7 @@ target_stop (ptid_t ptid)
> /* See target/target.h. */
>
> void
> -target_stop_ptid (ptid_t ptid)
> +target_stop_and_wait (ptid_t ptid)
> {
> struct target_waitstatus status;
> int was_non_stop = non_stop;
> @@ -3047,7 +3047,7 @@ target_stop_ptid (ptid_t ptid)
> /* See target/target.h. */
>
> void
> -target_continue_ptid (ptid_t ptid)
> +target_continue_no_signal (ptid_t ptid)
> {
> target_resume (ptid, 0, GDB_SIGNAL_0);
> }
> diff --git a/gdb/target/target.h b/gdb/target/target.h
> index 99a7966..c8b9c80 100644
> --- a/gdb/target/target.h
> +++ b/gdb/target/target.h
> @@ -59,15 +59,17 @@ extern int target_read_uint32 (CORE_ADDR memaddr, uint32_t *result);
> extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
> ssize_t len);
>
> -/* Make target stop in a continuable fashion. (For instance, under
> - Unix, this should act like SIGSTOP). This function must be
> - provided by the client. */
> +/* Cause the target to stop in a continuable fashion--for instance,
> + under Unix, this should act like SIGSTOP--and wait for the target
> + to be stopped before returning. This function must be provided by
> + the client. */
>
> -extern void target_stop_ptid (ptid_t ptid);
> +extern void target_stop_and_wait (ptid_t ptid);
>
> -/* Restart a target that was previously stopped by target_stop_ptid.
> - This function must be provided by the client. */
> +/* Restart a target previously stopped by target_stop_and_wait.
> + No signal is delivered to the target. This function must be
> + provided by the client. */
>
> -extern void target_continue_ptid (ptid_t ptid);
> +extern void target_continue_no_signal (ptid_t ptid);
>
> #endif /* TARGET_COMMON_H */
>
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] Wait for target to stop before running agent commands
2014-09-18 9:59 ` [PATCH 3/3] Wait for target to stop before running agent commands Gary Benson
@ 2014-09-19 16:16 ` Pedro Alves
0 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2014-09-19 16:16 UTC (permalink / raw)
To: Gary Benson, gdb-patches; +Cc: Doug Evans
On 09/18/2014 10:59 AM, Gary Benson wrote:
> This commit fixes a bug on where agent_run_command is called on
> GNU/Linux targets that may not have stopped yet.
>
> gdb/ChangeLog:
>
> * linux-nat.c (linux_child_static_tracepoint_markers_by_strid):
> Use target_stop_and_wait in place of target_stop.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/linux-nat.c | 2 +-
> 2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 5a82d23..f2895ff 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4458,7 +4458,7 @@ linux_child_static_tracepoint_markers_by_strid (struct target_ops *self,
> ptid_t ptid = ptid_build (pid, 0, 0);
>
> /* Pause all */
> - target_stop (ptid);
> + target_stop_and_wait (ptid);
More would be needed for fix this fully.
Unlike the comment above suggests, target_stop_and_wait only
waits for one thread to stop, not all.
See the gdbserver version (run_inferior_command) -- this is also
missing uninserting breakpoints after fully pausing the target.
If we handle this internally to linux-nat.c, then similarly
to gdbserver's version, this could use
stop_callback/stop_wait_callback to leave pending any events
that pausing might trip on. Then we'd need to be careful to
only re-resume threads that were already resumed before, and
leave those with pending events stopped. If we want to
use existing target methods, then we need to teach gdb's
struct thread_info layer about leaving events pending, like in:
https://sourceware.org/ml/gdb-patches/2011-12/msg00564.html
Alternative, we can would promote target_stop_and_wait to
a real target method that handles the leaving of events pending,
like gdbserver's pause_all/unpause_all methods.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] Update target_stop's documentation
2014-09-18 9:59 ` [PATCH 2/3] Update target_stop's documentation Gary Benson
@ 2014-09-19 16:38 ` Pedro Alves
2014-09-22 10:35 ` Gary Benson
0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2014-09-19 16:38 UTC (permalink / raw)
To: Gary Benson, gdb-patches; +Cc: Doug Evans
On 09/18/2014 10:59 AM, Gary Benson wrote:
> This commit updates target_stop's documentation to clarify that
> it is asynchronous.
> + before returning. If this is the behaviour you want please use
US spelling: "behavior".
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] Update target_stop's documentation
2014-09-19 16:38 ` Pedro Alves
@ 2014-09-22 10:35 ` Gary Benson
0 siblings, 0 replies; 8+ messages in thread
From: Gary Benson @ 2014-09-22 10:35 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Doug Evans
Pedro Alves wrote:
> On 09/18/2014 10:59 AM, Gary Benson wrote:
> > This commit updates target_stop's documentation to clarify that
> > it is asynchronous.
>
> > + before returning. If this is the behaviour you want please use
>
> US spelling: "behavior".
>
> LGTM.
Ok, I have fixed this and pushed the first two patches of this series.
Thanks,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-09-22 10:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-18 9:59 [PATCH 0/3] target_{stop,continue} things Gary Benson
2014-09-18 9:59 ` [PATCH 3/3] Wait for target to stop before running agent commands Gary Benson
2014-09-19 16:16 ` Pedro Alves
2014-09-18 9:59 ` [PATCH 1/3] Rename target_{stop,continue}_ptid Gary Benson
2014-09-19 16:16 ` Pedro Alves
2014-09-18 9:59 ` [PATCH 2/3] Update target_stop's documentation Gary Benson
2014-09-19 16:38 ` Pedro Alves
2014-09-22 10:35 ` Gary Benson
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).