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 <lucienmp_antis...@yahoo.com> > --- > 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
