* [PATCH 1/2] Use target_continue{,_no_signal} instead of target_resume
2016-08-29 4:12 [PATCH 0/2] Consolidate target_{wait,resume} calls between GDB and gdbserver Sergio Durigan Junior
@ 2016-08-29 4:12 ` Sergio Durigan Junior
2016-08-31 10:25 ` Pedro Alves
2016-08-29 4:12 ` [PATCH 2/2] Share target_wait prototype between GDB and gdbserver Sergio Durigan Junior
1 sibling, 1 reply; 8+ messages in thread
From: Sergio Durigan Junior @ 2016-08-29 4:12 UTC (permalink / raw)
To: GDB Patches; +Cc: Sergio Durigan Junior
This commit implements a new function, target_continue, on top of the
target_resume function. Then, it replaces all calls to target_resume
by calls to target_continue or to the already existing
target_continue_no_signal.
This is one of the (many) necessary steps needed to consolidate the
target interface between GDB and gdbserver. In particular, I am
interested in the impact this change will have on the unification of
the fork_inferior function (which I have been working on).
Tested on the BuildBot, no regressions introduced.
gdb/gdbserver/ChangeLog:
2016-08-29 Sergio Durigan Junior <sergiodj@redhat.com>
* server.c (start_inferior): New variable 'ptid'. Replace calls
to the_target->resume by target_continue{,_no_signal}, depending
on the case.
* target.c (target_stop_and_wait): Call target_continue_no_signal
instead of the_target->resume.
(target_continue): New function.
gdb/ChangeLog:
2016-08-29 Sergio Durigan Junior <sergiodj@redhat.com>
* fork-child.c (startup_inferior): Replace calls to target_resume
by target_continue{,_no_signal}, depending on the case.
* linux-nat.c (cleanup_target_stop): Call
target_continue_no_signal instead of target_resume.
* procfs.c (procfs_wait): Likewise.
* target.c (target_continue): New function.
* target/target.h (target_continue): New prototype.
---
gdb/fork-child.c | 4 ++--
gdb/gdbserver/server.c | 25 +++++++------------------
gdb/gdbserver/target.c | 19 ++++++++++++++-----
gdb/linux-nat.c | 2 +-
gdb/procfs.c | 4 ++--
gdb/target.c | 8 ++++++++
gdb/target/target.h | 6 ++++++
7 files changed, 40 insertions(+), 28 deletions(-)
diff --git a/gdb/fork-child.c b/gdb/fork-child.c
index 6856cf6..3e29a1f 100644
--- a/gdb/fork-child.c
+++ b/gdb/fork-child.c
@@ -514,7 +514,7 @@ startup_inferior (int ntraps)
if (resume_signal != GDB_SIGNAL_TRAP)
{
/* Let shell child handle its own signals in its own way. */
- target_resume (resume_ptid, 0, resume_signal);
+ target_continue (resume_ptid, resume_signal);
}
else
{
@@ -540,7 +540,7 @@ startup_inferior (int ntraps)
break;
/* Just make it go on. */
- target_resume (resume_ptid, 0, GDB_SIGNAL_0);
+ target_continue_no_signal (resume_ptid);
}
}
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 6fbd61d..9c06443 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -258,12 +258,7 @@ start_inferior (char **argv)
if (wrapper_argv != NULL)
{
- struct thread_resume resume_info;
-
- memset (&resume_info, 0, sizeof (resume_info));
- resume_info.thread = pid_to_ptid (signal_pid);
- resume_info.kind = resume_continue;
- resume_info.sig = 0;
+ ptid_t ptid = pid_to_ptid (signal_pid);
last_ptid = mywait (pid_to_ptid (signal_pid), &last_status, 0, 0);
@@ -271,7 +266,7 @@ start_inferior (char **argv)
{
do
{
- (*the_target->resume) (&resume_info, 1);
+ target_continue_no_signal (ptid);
last_ptid = mywait (pid_to_ptid (signal_pid), &last_status, 0, 0);
if (last_status.kind != TARGET_WAITKIND_STOPPED)
@@ -3929,7 +3924,6 @@ process_serial_event (void)
if ((tracing && disconnected_tracing) || any_persistent_commands ())
{
- struct thread_resume resume_info;
struct process_info *process = find_process_pid (pid);
if (process == NULL)
@@ -3965,10 +3959,7 @@ process_serial_event (void)
process->gdb_detached = 1;
/* Detaching implicitly resumes all threads. */
- resume_info.thread = minus_one_ptid;
- resume_info.kind = resume_continue;
- resume_info.sig = 0;
- (*the_target->resume) (&resume_info, 1);
+ target_continue_no_signal (minus_one_ptid);
write_ok (own_buf);
break; /* from switch/case */
@@ -4428,7 +4419,7 @@ handle_target_event (int err, gdb_client_data client_data)
/* A thread stopped with a signal, but gdb isn't
connected to handle it. Pass it down to the
inferior, as if it wasn't being traced. */
- struct thread_resume resume_info;
+ enum gdb_signal signal;
if (debug_threads)
debug_printf ("GDB not connected; forwarding event %d for"
@@ -4436,13 +4427,11 @@ handle_target_event (int err, gdb_client_data client_data)
(int) last_status.kind,
target_pid_to_str (last_ptid));
- resume_info.thread = last_ptid;
- resume_info.kind = resume_continue;
if (last_status.kind == TARGET_WAITKIND_STOPPED)
- resume_info.sig = gdb_signal_to_host (last_status.value.sig);
+ signal = last_status.value.sig;
else
- resume_info.sig = 0;
- (*the_target->resume) (&resume_info, 1);
+ signal = GDB_SIGNAL_0;
+ target_continue (last_ptid, signal);
}
}
else
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 42d22b4..8435dc2 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -248,12 +248,8 @@ target_stop_and_wait (ptid_t ptid)
{
struct target_waitstatus status;
int was_non_stop = non_stop;
- struct thread_resume resume_info;
- resume_info.thread = ptid;
- resume_info.kind = resume_stop;
- resume_info.sig = GDB_SIGNAL_0;
- (*the_target->resume) (&resume_info, 1);
+ target_continue_no_signal (ptid);
non_stop = 1;
mywait (ptid, &status, 0, 0);
@@ -273,6 +269,19 @@ target_continue_no_signal (ptid_t ptid)
(*the_target->resume) (&resume_info, 1);
}
+/* See target/target.h. */
+
+void
+target_continue (ptid_t ptid, enum gdb_signal signal)
+{
+ struct thread_resume resume_info;
+
+ resume_info.thread = ptid;
+ resume_info.kind = resume_continue;
+ resume_info.sig = gdb_signal_to_host (signal);
+ (*the_target->resume) (&resume_info, 1);
+}
+
int
start_non_stop (int nonstop)
{
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 5d5efa0..7410f8e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4289,7 +4289,7 @@ cleanup_target_stop (void *arg)
gdb_assert (arg != NULL);
/* Unpause all */
- target_resume (*ptid, 0, GDB_SIGNAL_0);
+ target_continue_no_signal (*ptid);
}
static VEC(static_tracepoint_marker_p) *
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 67b424f..0e0641e 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -3716,7 +3716,7 @@ wait_again:
else
{
/* How to keep going without returning to wfi: */
- target_resume (ptid, 0, GDB_SIGNAL_0);
+ target_continue_no_signal (ptid);
goto wait_again;
}
}
@@ -3742,7 +3742,7 @@ wait_again:
/* This is an internal event and should be transparent
to wfi, so resume the execution and wait again. See
comment in procfs_init_inferior() for more details. */
- target_resume (ptid, 0, GDB_SIGNAL_0);
+ target_continue_no_signal (ptid);
goto wait_again;
}
#endif
diff --git a/gdb/target.c b/gdb/target.c
index 13e3cdb..87c9681 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3449,6 +3449,14 @@ target_continue_no_signal (ptid_t ptid)
target_resume (ptid, 0, GDB_SIGNAL_0);
}
+/* See target/target.h. */
+
+void
+target_continue (ptid_t ptid, enum gdb_signal signal)
+{
+ target_resume (ptid, 0, signal);
+}
+
/* Concatenate ELEM to LIST, a comma separate list, and return the
result. The LIST incoming argument is released. */
diff --git a/gdb/target/target.h b/gdb/target/target.h
index 76af732..1bfa1b0 100644
--- a/gdb/target/target.h
+++ b/gdb/target/target.h
@@ -70,4 +70,10 @@ extern void target_stop_and_wait (ptid_t ptid);
extern void target_continue_no_signal (ptid_t ptid);
+/* Restart a target previously stopped by target_stop_and_wait.
+ SIGNAL is delivered to the target. This function must be provided
+ by the client. */
+
+extern void target_continue (ptid_t ptid, enum gdb_signal signal);
+
#endif /* TARGET_COMMON_H */
--
2.4.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] Consolidate target_{wait,resume} calls between GDB and gdbserver
@ 2016-08-29 4:12 Sergio Durigan Junior
2016-08-29 4:12 ` [PATCH 1/2] Use target_continue{,_no_signal} instead of target_resume Sergio Durigan Junior
2016-08-29 4:12 ` [PATCH 2/2] Share target_wait prototype between GDB and gdbserver Sergio Durigan Junior
0 siblings, 2 replies; 8+ messages in thread
From: Sergio Durigan Junior @ 2016-08-29 4:12 UTC (permalink / raw)
To: GDB Patches; +Cc: Sergio Durigan Junior
Hi,
This simple patch series proposes a consolidation of the calls to
target_{wait,resume} functions between GDB and gdbserver. This is a
necessary and helpful step in the direction that we're heading, and is
particularly important for my work on the unification of fork_inferior.
The first patch implements a new function, target_continue, on top of
target_resume (or the_target->resume, depending on where you're
looking), and replaces all calls to target_resume (or
the_target->resume) by either target_continue or
target_continue_no_signal (this last one has been a GDB citizen for
some time). Note that I haven't touched infrun.c's
"do_target_resume", which is a wrapper around target_resume and does
many other things.
The second patch, which is even simpler, is meant to pay a technical
debt on gdbserver: the lack of the "target_wait" function. So there
it is now, properly implemented, even though everyone else will still
use mywait because it accounts for many aspects that the regular
target_wait doesn't. This is actually another place for future
improvement: make the two versions of target_wait identical API-wise,
so that sharing code is easier.
Both patches do not introduce regressions (I've tested them using the
Try server feature on BuildBot, BTW).
OK to apply?
Thanks,
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] Share target_wait prototype between GDB and gdbserver
2016-08-29 4:12 [PATCH 0/2] Consolidate target_{wait,resume} calls between GDB and gdbserver Sergio Durigan Junior
2016-08-29 4:12 ` [PATCH 1/2] Use target_continue{,_no_signal} instead of target_resume Sergio Durigan Junior
@ 2016-08-29 4:12 ` Sergio Durigan Junior
2016-08-31 10:29 ` Pedro Alves
1 sibling, 1 reply; 8+ messages in thread
From: Sergio Durigan Junior @ 2016-08-29 4:12 UTC (permalink / raw)
To: GDB Patches; +Cc: Sergio Durigan Junior
This commit moves the target_wait prototype from the GDB-specific
target.h header to the common target/target.h header. Then, it
creates a compatible implementation of target_wait on gdbserver using
the_target->wait, and adjusts the (only) caller (mywait function).
Pretty straightforward, no regressions introduced.
gdb/gdbserver/ChangeLog:
2016-08-29 Sergio Durigan Junior <sergiodj@redhat.com>
* target.c (mywait): Call target_wait instead of
the_target->wait.
(target_wait): New function.
gdb/ChangeLog:
2016-08-29 Sergio Durigan Junior <sergiodj@redhat.com>
* target.c (target_wait): Mention that the function's prototype
can be found at target/target.h.
* target.h (target_wait): Move prototype from here...
* target/target.h (target_wait): ... to here.
---
gdb/gdbserver/target.c | 10 +++++++++-
gdb/target.c | 2 ++
gdb/target.h | 12 ------------
gdb/target/target.h | 12 ++++++++++++
4 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 8435dc2..053629c 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -211,7 +211,7 @@ mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
if (connected_wait)
server_waiting = 1;
- ret = (*the_target->wait) (ptid, ourstatus, options);
+ ret = target_wait (ptid, ourstatus, options);
/* We don't expose _LOADED events to gdbserver core. See the
`dlls_changed' global. */
@@ -258,6 +258,14 @@ target_stop_and_wait (ptid_t ptid)
/* See target/target.h. */
+ptid_t
+target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
+{
+ return (*the_target->wait) (ptid, status, options);
+}
+
+/* See target/target.h. */
+
void
target_continue_no_signal (ptid_t ptid)
{
diff --git a/gdb/target.c b/gdb/target.c
index 87c9681..bca25bd 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2283,6 +2283,8 @@ target_disconnect (const char *args, int from_tty)
current_target.to_disconnect (¤t_target, args, from_tty);
}
+/* See target/target.h. */
+
ptid_t
target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
{
diff --git a/gdb/target.h b/gdb/target.h
index 9506e04..2af405a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1341,18 +1341,6 @@ extern void target_disconnect (const char *, int);
extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
-/* Wait for process pid to do something. PTID = -1 to wait for any
- pid to do something. Return pid of child, or -1 in case of error;
- store status through argument pointer STATUS. Note that it is
- _NOT_ OK to throw_exception() out of target_wait() without popping
- the debugging target from the stack; GDB isn't prepared to get back
- to the prompt with a debugging target but without the frame cache,
- stop_pc, etc., set up. OPTIONS is a bitwise OR of TARGET_W*
- options. */
-
-extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
- int options);
-
/* The default target_ops::to_wait implementation. */
extern ptid_t default_target_wait (struct target_ops *ops,
diff --git a/gdb/target/target.h b/gdb/target/target.h
index 1bfa1b0..a95fbe4 100644
--- a/gdb/target/target.h
+++ b/gdb/target/target.h
@@ -76,4 +76,16 @@ extern void target_continue_no_signal (ptid_t ptid);
extern void target_continue (ptid_t ptid, enum gdb_signal signal);
+/* Wait for process pid to do something. PTID = -1 to wait for any
+ pid to do something. Return pid of child, or -1 in case of error;
+ store status through argument pointer STATUS. Note that it is
+ _NOT_ OK to throw_exception() out of target_wait() without popping
+ the debugging target from the stack; GDB isn't prepared to get back
+ to the prompt with a debugging target but without the frame cache,
+ stop_pc, etc., set up. OPTIONS is a bitwise OR of TARGET_W*
+ options. */
+
+extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
+ int options);
+
#endif /* TARGET_COMMON_H */
--
2.4.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Use target_continue{,_no_signal} instead of target_resume
2016-08-29 4:12 ` [PATCH 1/2] Use target_continue{,_no_signal} instead of target_resume Sergio Durigan Junior
@ 2016-08-31 10:25 ` Pedro Alves
2016-09-01 18:57 ` Sergio Durigan Junior
0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2016-08-31 10:25 UTC (permalink / raw)
To: Sergio Durigan Junior, GDB Patches
On 08/29/2016 05:11 AM, Sergio Durigan Junior wrote:
> diff --git a/gdb/target/target.h b/gdb/target/target.h
> index 76af732..1bfa1b0 100644
> --- a/gdb/target/target.h
> +++ b/gdb/target/target.h
> @@ -70,4 +70,10 @@ extern void target_stop_and_wait (ptid_t ptid);
>
> extern void target_continue_no_signal (ptid_t ptid);
>
> +/* Restart a target previously stopped by target_stop_and_wait.
Given you're using this function in other contexts, this sentence
doesn't sound right. Likewise, the existing comment on top of
target_continue_no_signal is no longer accurate.
Otherwise LGTM.
> + SIGNAL is delivered to the target. This function must be provided
> + by the client. */
> +
> +extern void target_continue (ptid_t ptid, enum gdb_signal signal);
> +
> #endif /* TARGET_COMMON_H */
>
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Share target_wait prototype between GDB and gdbserver
2016-08-29 4:12 ` [PATCH 2/2] Share target_wait prototype between GDB and gdbserver Sergio Durigan Junior
@ 2016-08-31 10:29 ` Pedro Alves
2016-08-31 10:30 ` Pedro Alves
2016-09-01 18:58 ` Sergio Durigan Junior
0 siblings, 2 replies; 8+ messages in thread
From: Pedro Alves @ 2016-08-31 10:29 UTC (permalink / raw)
To: Sergio Durigan Junior, GDB Patches
On 08/29/2016 05:11 AM, Sergio Durigan Junior wrote:
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -1341,18 +1341,6 @@ extern void target_disconnect (const char *, int);
>
> extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
>
> -/* Wait for process pid to do something. PTID = -1 to wait for any
> - pid to do something. Return pid of child, or -1 in case of error;
> - store status through argument pointer STATUS. Note that it is
> - _NOT_ OK to throw_exception() out of target_wait() without popping
> - the debugging target from the stack; GDB isn't prepared to get back
> - to the prompt with a debugging target but without the frame cache,
> - stop_pc, etc., set up. OPTIONS is a bitwise OR of TARGET_W*
> - options. */
> -
> -extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
> - int options);
> -
Please leave a breadcrumb comment here, like was done for
target_read_memory, for example:
/* For target_read_memory see target/target.h. */
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Share target_wait prototype between GDB and gdbserver
2016-08-31 10:29 ` Pedro Alves
@ 2016-08-31 10:30 ` Pedro Alves
2016-09-01 18:58 ` Sergio Durigan Junior
1 sibling, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2016-08-31 10:30 UTC (permalink / raw)
To: Sergio Durigan Junior, GDB Patches
On 08/31/2016 11:29 AM, Pedro Alves wrote:
> Please leave a breadcrumb comment here, like was done for
> target_read_memory, for example:
>
> /* For target_read_memory see target/target.h. */
>
Hit send too soon. Meant to say:
OK with that change.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Use target_continue{,_no_signal} instead of target_resume
2016-08-31 10:25 ` Pedro Alves
@ 2016-09-01 18:57 ` Sergio Durigan Junior
0 siblings, 0 replies; 8+ messages in thread
From: Sergio Durigan Junior @ 2016-09-01 18:57 UTC (permalink / raw)
To: Pedro Alves; +Cc: GDB Patches
On Wednesday, August 31 2016, Pedro Alves wrote:
> On 08/29/2016 05:11 AM, Sergio Durigan Junior wrote:
>
>> diff --git a/gdb/target/target.h b/gdb/target/target.h
>> index 76af732..1bfa1b0 100644
>> --- a/gdb/target/target.h
>> +++ b/gdb/target/target.h
>> @@ -70,4 +70,10 @@ extern void target_stop_and_wait (ptid_t ptid);
>>
>> extern void target_continue_no_signal (ptid_t ptid);
>>
>> +/* Restart a target previously stopped by target_stop_and_wait.
>
> Given you're using this function in other contexts, this sentence
> doesn't sound right. Likewise, the existing comment on top of
> target_continue_no_signal is no longer accurate.
Thanks, fixed and pushed.
049a857091cff98371b5688140832a3cf767153c
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Share target_wait prototype between GDB and gdbserver
2016-08-31 10:29 ` Pedro Alves
2016-08-31 10:30 ` Pedro Alves
@ 2016-09-01 18:58 ` Sergio Durigan Junior
1 sibling, 0 replies; 8+ messages in thread
From: Sergio Durigan Junior @ 2016-09-01 18:58 UTC (permalink / raw)
To: Pedro Alves; +Cc: GDB Patches
On Wednesday, August 31 2016, Pedro Alves wrote:
> On 08/29/2016 05:11 AM, Sergio Durigan Junior wrote:
>
>> --- a/gdb/target.h
>> +++ b/gdb/target.h
>> @@ -1341,18 +1341,6 @@ extern void target_disconnect (const char *, int);
>>
>> extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
>>
>> -/* Wait for process pid to do something. PTID = -1 to wait for any
>> - pid to do something. Return pid of child, or -1 in case of error;
>> - store status through argument pointer STATUS. Note that it is
>> - _NOT_ OK to throw_exception() out of target_wait() without popping
>> - the debugging target from the stack; GDB isn't prepared to get back
>> - to the prompt with a debugging target but without the frame cache,
>> - stop_pc, etc., set up. OPTIONS is a bitwise OR of TARGET_W*
>> - options. */
>> -
>> -extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
>> - int options);
>> -
>
> Please leave a breadcrumb comment here, like was done for
> target_read_memory, for example:
>
> /* For target_read_memory see target/target.h. */
Fixed and pushed:
f2b9e3dfd4bc3c5149496fdbeaa5f0907220685f
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-01 18:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-29 4:12 [PATCH 0/2] Consolidate target_{wait,resume} calls between GDB and gdbserver Sergio Durigan Junior
2016-08-29 4:12 ` [PATCH 1/2] Use target_continue{,_no_signal} instead of target_resume Sergio Durigan Junior
2016-08-31 10:25 ` Pedro Alves
2016-09-01 18:57 ` Sergio Durigan Junior
2016-08-29 4:12 ` [PATCH 2/2] Share target_wait prototype between GDB and gdbserver Sergio Durigan Junior
2016-08-31 10:29 ` Pedro Alves
2016-08-31 10:30 ` Pedro Alves
2016-09-01 18:58 ` Sergio Durigan Junior
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).