[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Fix for RSP vCont packet
From: |
Luc Michel |
Subject: |
Re: [Qemu-devel] [PATCH] Fix for RSP vCont packet |
Date: |
Fri, 1 Feb 2019 14:33:10 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
Hi Lucien,
On 1/31/19 5:48 AM, Lucien Murray-Pitts wrote:
> The result is that vCont now does not recognise the case where no
> process/thread is provided after the action.
>
> This may not show up with GDB, but using Lauterbach Trace32, and Hexrays IDA
> Pro this issue is immediately seen.
> The response is a "$#00" empty packet, showing it is unsupported packet.
>
> This is defined in the RSP document as "An action with no thread-id matches
> all threads."
> (https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#vCont-packet )
>
> Thus the valid vCont packets now are as below, however parsing is still not
> very strict.
> vCont;c/s - Step/Continue all threads
> vCont;c/s:[pX.]Y - Step/Continue optional process X, thread Y
> vCont;C##/S##:[pX.]Y - Step/Continue with signal ## on optional
> process X, thread Y
> * If X or Y are -1 then it applies the action to all processes/threads.
>
> Signed-off-by: Lucien Murray-Pitts <address@hidden>
> ---
> gdbstub.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index bfc7afb509..ce0dde2e24 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1169,6 +1169,7 @@ static int is_query_packet(const char *p, const char
> *query, char separator)
> */
> static int gdb_handle_vcont(GDBState *s, const char *p)
> {
> + GDBThreadIdKind vcontThreadType ;
The coding style for variable names is lower_case_with_underscores (see
CODING_STYLE). I think you can go with a simpler name like
GDBThreadIdKind kind;
> int res, signal = 0;
> char cur_action;
> char *newstates;
> @@ -1218,12 +1219,23 @@ static int gdb_handle_vcont(GDBState *s, const char
> *p)
> goto out;
> }
>
> - if (*p++ != ':') {
> + /*
> + * In the case we have vCont;c or vCont;s - action is on all threads
> + * Alternatively vCont;c;s:p1.1 is a possible, but meaningless
> format,
> + * And in the else the "vCont;c:p1.1;... format is supported.
> + */> + if (*p == '\0' || *p == ';') {
> + vcontThreadType = GDB_ALL_THREAD ;> + pid = 1 ;
The spec is not clear but I would opt for GDB_ALL_PROCESSES instead of
GDB_ALL_THREAD here. pid = 1; is clearly wrong since you don't know if
this PID exists or is currently attached.
> + tid = 1 ;
This one is not useful either (not used in the switch..case bellow).
Thanks
Luc
> + } else if (*p++ == ':') {
> + vcontThreadType = read_thread_id(p, &p, &pid, &tid) ;
> + } else {
> res = -ENOTSUP;
> goto out;
> }
>
> - switch (read_thread_id(p, &p, &pid, &tid)) {
> + switch (vcontThreadType) {
> case GDB_READ_THREAD_ERR:
> res = -EINVAL;
> goto out;
>
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH] Fix for RSP vCont packet,
Luc Michel <=