Re: [Qemu-devel] [PULL 09/23] gdbstub: Fix vCont behaviour

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 19879 bytes --]

On 2018-02-17 09:56, Jan Kiszka wrote:
> On 2017-02-16 15:31, Paolo Bonzini wrote:
>> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>>
>> When GDB issues a "vCont", QEMU was not handling it correctly when
>> multiple VCPUs are active.
>> For vCont, for each thread (VCPU), it can be specified whether to
>> single step, continue or stop that thread. The default is to stop a
>> thread.
>> However, when (for example) "vCont;s:2" is issued, all VCPUs continue
>> to run, although all but VCPU nr 2 are to be stopped.
>>
>> This patch completely rewrites the vCont parsing code.
>>
>> Please note that this improvement only works in system emulation mode,
>> when in userspace emulation mode the old behaviour is preserved.
>>
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>> Message-Id: <1487092068-16562-3-git-send-email-imbrenda@linux.vnet.ibm.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  gdbstub.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 162 insertions(+), 47 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index 755a8e3..9911153 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -387,6 +387,60 @@ static inline void gdb_continue(GDBState *s)
>>  #endif
>>  }
>>  
>> +/*
>> + * Resume execution, per CPU actions. For user-mode emulation it's
>> + * equivalent to gdb_continue.
>> + */
>> +static int gdb_continue_partial(GDBState *s, char *newstates)
>> +{
>> +    CPUState *cpu;
>> +    int res = 0;
>> +#ifdef CONFIG_USER_ONLY
>> +    /*
>> +     * This is not exactly accurate, but it's an improvement compared to the
>> +     * previous situation, where only one CPU would be single-stepped.
>> +     */
>> +    CPU_FOREACH(cpu) {
>> +        if (newstates[cpu->cpu_index] == 's') {
>> +            cpu_single_step(cpu, sstep_flags);
>> +        }
>> +    }
>> +    s->running_state = 1;
>> +#else
>> +    int flag = 0;
>> +
>> +    if (!runstate_needs_reset()) {
>> +        if (vm_prepare_start()) {
>> +            return 0;
>> +        }
>> +
>> +        CPU_FOREACH(cpu) {
>> +            switch (newstates[cpu->cpu_index]) {
>> +            case 0:
>> +            case 1:
>> +                break; /* nothing to do here */
>> +            case 's':
>> +                cpu_single_step(cpu, sstep_flags);
>> +                cpu_resume(cpu);
>> +                flag = 1;
>> +                break;
>> +            case 'c':
>> +                cpu_resume(cpu);
>> +                flag = 1;
>> +                break;
>> +            default:
>> +                res = -1;
>> +                break;
>> +            }
>> +        }
>> +    }
>> +    if (flag) {
>> +        qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true);
>> +    }
>> +#endif
>> +    return res;
>> +}
>> +
>>  static void put_buffer(GDBState *s, const uint8_t *buf, int len)
>>  {
>>  #ifdef CONFIG_USER_ONLY
>> @@ -785,6 +839,107 @@ static int is_query_packet(const char *p, const char *query, char separator)
>>          (p[query_len] == '\0' || p[query_len] == separator);
>>  }
>>  
>> +/**
>> + * gdb_handle_vcont - Parses and handles a vCont packet.
>> + * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
>> + *         a format error, 0 on success.
>> + */
>> +static int gdb_handle_vcont(GDBState *s, const char *p)
>> +{
>> +    int res, idx, signal = 0;
>> +    char cur_action;
>> +    char *newstates;
>> +    unsigned long tmp;
>> +    CPUState *cpu;
>> +#ifdef CONFIG_USER_ONLY
>> +    int max_cpus = 1; /* global variable max_cpus exists only in system mode */
>> +
>> +    CPU_FOREACH(cpu) {
>> +        max_cpus = max_cpus <= cpu->cpu_index ? cpu->cpu_index + 1 : max_cpus;
>> +    }
>> +#endif
>> +    /* uninitialised CPUs stay 0 */
>> +    newstates = g_new0(char, max_cpus);
>> +
>> +    /* mark valid CPUs with 1 */
>> +    CPU_FOREACH(cpu) {
>> +        newstates[cpu->cpu_index] = 1;
>> +    }
>> +
>> +    /*
>> +     * res keeps track of what error we are returning, with -ENOTSUP meaning
>> +     * that the command is unknown or unsupported, thus returning an empty
>> +     * packet, while -EINVAL and -ERANGE cause an E22 packet, due to invalid,
>> +     *  or incorrect parameters passed.
>> +     */
>> +    res = 0;
>> +    while (*p) {
>> +        if (*p++ != ';') {
>> +            res = -ENOTSUP;
>> +            goto out;
>> +        }
>> +
>> +        cur_action = *p++;
>> +        if (cur_action == 'C' || cur_action == 'S') {
>> +            cur_action = tolower(cur_action);
>> +            res = qemu_strtoul(p + 1, &p, 16, &tmp);
>> +            if (res) {
>> +                goto out;
>> +            }
>> +            signal = gdb_signal_to_target(tmp);
>> +        } else if (cur_action != 'c' && cur_action != 's') {
>> +            /* unknown/invalid/unsupported command */
>> +            res = -ENOTSUP;
>> +            goto out;
>> +        }
>> +        /* thread specification. special values: (none), -1 = all; 0 = any */
>> +        if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
>> +            if (*p == ':') {
>> +                p += 3;
>> +            }
>> +            for (idx = 0; idx < max_cpus; idx++) {
>> +                if (newstates[idx] == 1) {
>> +                    newstates[idx] = cur_action;
>> +                }
>> +            }
>> +        } else if (*p == ':') {
>> +            p++;
>> +            res = qemu_strtoul(p, &p, 16, &tmp);
>> +            if (res) {
>> +                goto out;
>> +            }
>> +            idx = tmp;
>> +            /* 0 means any thread, so we pick the first valid CPU */
>> +            if (!idx) {
>> +                idx = cpu_index(first_cpu);
>> +            }
>> +
>> +            /*
>> +             * If we are in user mode, the thread specified is actually a
>> +             * thread id, and not an index. We need to find the actual
>> +             * CPU first, and only then we can use its index.
>> +             */
>> +            cpu = find_cpu(idx);
>> +            /* invalid CPU/thread specified */
>> +            if (!idx || !cpu) {
>> +                res = -EINVAL;
>> +                goto out;
>> +            }
>> +            /* only use if no previous match occourred */
>> +            if (newstates[cpu->cpu_index] == 1) {
>> +                newstates[cpu->cpu_index] = cur_action;
>> +            }
>> +        }
>> +    }
>> +    s->signal = signal;
>> +    gdb_continue_partial(s, newstates);
>> +
>> +out:
>> +    g_free(newstates);
>> +
>> +    return res;
>> +}
>> +
>>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>>  {
>>      CPUState *cpu;
>> @@ -830,60 +985,20 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>>          return RS_IDLE;
>>      case 'v':
>>          if (strncmp(p, "Cont", 4) == 0) {
>> -            int res_signal, res_thread;
>> -
>>              p += 4;
>>              if (*p == '?') {
>>                  put_packet(s, "vCont;c;C;s;S");
>>                  break;
>>              }
>> -            res = 0;
>> -            res_signal = 0;
>> -            res_thread = 0;
>> -            while (*p) {
>> -                int action, signal;
>> -
>> -                if (*p++ != ';') {
>> -                    res = 0;
>> -                    break;
>> -                }
>> -                action = *p++;
>> -                signal = 0;
>> -                if (action == 'C' || action == 'S') {
>> -                    signal = gdb_signal_to_target(strtoul(p, (char **)&p, 16));
>> -                    if (signal == -1) {
>> -                        signal = 0;
>> -                    }
>> -                } else if (action != 'c' && action != 's') {
>> -                    res = 0;
>> -                    break;
>> -                }
>> -                thread = 0;
>> -                if (*p == ':') {
>> -                    thread = strtoull(p+1, (char **)&p, 16);
>> -                }
>> -                action = tolower(action);
>> -                if (res == 0 || (res == 'c' && action == 's')) {
>> -                    res = action;
>> -                    res_signal = signal;
>> -                    res_thread = thread;
>> -                }
>> -            }
>> +
>> +            res = gdb_handle_vcont(s, p);
>> +
>>              if (res) {
>> -                if (res_thread != -1 && res_thread != 0) {
>> -                    cpu = find_cpu(res_thread);
>> -                    if (cpu == NULL) {
>> -                        put_packet(s, "E22");
>> -                        break;
>> -                    }
>> -                    s->c_cpu = cpu;
>> -                }
>> -                if (res == 's') {
>> -                    cpu_single_step(s->c_cpu, sstep_flags);
>> +                if ((res == -EINVAL) || (res == -ERANGE)) {
>> +                    put_packet(s, "E22");
>> +                    break;
>>                  }
>> -                s->signal = res_signal;
>> -                gdb_continue(s);
>> -                return RS_IDLE;
>> +                goto unknown_command;
>>              }
>>              break;
>>          } else {
>>
> 
> Seems like no one is doing guest debugging with kvm on x86 except me,
> and I'm only doing it too infrequently now: This one broke that use case
> for SMP guests long ago. How was it tested?
> 
> To reproduce the bug: set up an x86-64 guest kernel with > 1 core, break
> on some prominent syscall entry (e.g. sys_execve), continue the guest on
> hit and it will quickly lock up, even after disabling the breakpoint
> again. Kernel version doesn't matter (was my first guess), gdb is
> 7.7.50.20140604-cvs (OpenSUSE) here.
> 

I case it helps:

GNU gdb (GDB) 7.7.50.20140604-cvs
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from vmlinux...done.
(gdb) tar rem :1234
Remote debugging using :1234
0xffffffff81719d51 in native_safe_halt () at ../arch/x86/include/asm/irqflags.h:54
warning: Source file is more recent than executable.
54              asm volatile("hlt": : :"memory");
(gdb) c
Continuing.
^C
Program received signal SIGINT, Interrupt.
0xffffffff81719d51 in native_safe_halt () at ../arch/x86/include/asm/irqflags.h:54
54              asm volatile("hlt": : :"memory");
(gdb) set debug remote 1
(gdb) b sys_execve
Sending packet: $Hg4#e3...Ack
Packet received: OK
Sending packet: $g#67...Ack
Packet received: c023ef390088ffff000000000000000044be080000c9ffff00f70000000000000100000000000000c023ef390088ffff0000000000000000c8be080000c9ffff0100000000000000010000000000000010be080000c9ffff089a0082ffffffffc023ef390088ffffc023ef390088ffff0300000000000000c023ef390088ffff519d7181ffffffff0602000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000000000000000000000000000ff000000ffffffffffffffffffffffff00ffffff70617273655f616e645f657865637574414d4500000000000000000000000000000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $mffffffff811a3800,40#f3...Ack
Packet received: 5b5d415ce9aa65ebffe8a2e265004883ec10488974240848891424e84c620000488b14244889c7488b742408e8cdfdffff48984883c410c3e873e26500554489
Sending packet: $mffffffff811a3809,1#c9...Ack
Packet received: e8
Breakpoint 1 at 0xffffffff811a3809: file ../fs/exec.c, line 1923.
(gdb) c
Continuing.
Sending packet: $Z0,ffffffff811a3809,1#12...Ack
Packet received: OK
Packet Z0 (software-breakpoint) is supported
Sending packet: $vCont;c#a8...Ack
Packet received: T05thread:01;
Sending packet: $g#67...Ack
Packet received: 3b0000000000000058bfc00000c9ffff60f1d015ff7f0000e0f073000000000090b673000000000090436b00000000003b0000000000000030bfc00000c9ffff40bd73000000000060b37300000000000000000000000000000000000000000000a852370088ffff00000000000000000000000000000000000000000000000009381a81ffffffff0202000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000ff000000000000000000000000ffffff00000000000000000000000000000000ff000000000000000000000000ffffff000000000000000000ff00ffffffffff000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $Hg4#e3...Ack
Packet received: OK
Sending packet: $g#67...Ack
Packet received: c023ef390088ffff000000000000000001000000000000004013da3f0088ffff0100000000000000c023ef390088ffff0000000000000000c8be080000c9ffff0100000000000000010000000000000080bd080000c9ffff88af0383ffffffffc023ef390088ffffc023ef390088ffff0300000000000000c023ef390088ffff519d7181ffffffff0602000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f0300000001000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffff000000000000000000000000000000000000000000000000000000000000000000000000ff00000000000000ff00000074726163743d2f646275732d7666732d00000000008064400000000000000000fefdfdfdfdfded3f0000000000000000000000000000f03f0000000000000000000000000000944000000000000000000000000000008e400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a01f0000
Sending packet: $z0,ffffffff811a3809,1#32...Ack
Packet received: OK

Sending packet: $Hg1#e0...Ack
Packet received: OK
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Breakpoint 1, SyS_execve (filename=7029648, argv=7583376, envp=7598304) at ../fs/exec.c:1923
warning: Source file is more recent than executable.
(gdb) c
Continuing.
Sending packet: $vCont;s:1#23...Ack
Packet received: T05thread:01;
Sending packet: $g#67...Ack
Packet received: 0020000000c9ffff46000000000000000c3fc03f0088ffff8287ac5600000000f1a60381fffffffff1a60381ffffffff000000006622ab56383fc03f0088ffff01000000000000000100000000000000d83ec03f0088ffff88af0383ffffffff18bec13f0088ffff98bec13f0088ffff18bfc13f0088ffff0a180000000000004da70381ffffffff4600000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000ff000000000000000000000000ffffff00000000000000000000000000000000ff000000000000000000000000ffffff000000000000000000ff00ffffffffff000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $Z0,ffffffff811a3809,1#12...Ack
Packet received: OK
Sending packet: $vCont;c#a8...Ack
Packet received: T05thread:01;
Sending packet: $g#67...Ack
Packet received: 3b0000000000000058bfc00000c9ffff60f1d015ff7f0000e0f073000000000090b673000000000090436b00000000003b0000000000000030bfc00000c9ffff40bd73000000000060b37300000000000000000000000000000000000000000000a852370088ffff00000000000000000000000000000000000000000000000009381a81ffffffff0203000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f03000000000000000000000000000000000000000000000000000000000000ff000000000000000000000000ffffff00000000000000000000000000000000ff000000000000000000000000ffffff000000000000000000ff00ffffffffff000000000000000031000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000801f0000
Sending packet: $Hg4#e3...Ack
Packet received: OK
Sending packet: $g#67...Ack
Packet received: c023ef390088ffff000000000000000001000000000000004013da3f0088ffff0100000000000000c023ef390088ffff0000000000000000c8be080000c9ffff0100000000000000010000000000000080bd080000c9ffff88af0383ffffffffc023ef390088ffffc023ef390088ffff0300000000000000c023ef390088ffff519d7181ffffffff0602000010000000180000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007f0300000001000000000000000000000000000000000000000000000000000000000000000000000000ffffffffffff000000000000000000000000000000000000000000000000000000000000000000000000ff00000000000000ff00000074726163743d2f646275732d7666732d00000000008064400000000000000000fefdfdfdfdfded3f0000000000000000000000000000f03f0000000000000000000000000000944000000000000000000000000000008e400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a01f0000
Sending packet: $z0,ffffffff811a3809,1#32...Ack
Packet received: OK

Sending packet: $Hg1#e0...Ack
Packet received: OK
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Sending packet: $mffffc90000c0bf30,8#83...Ack
Packet received: ae180081ffffffff
Breakpoint 1, SyS_execve (filename=7029648, argv=7583376, envp=7598304) at ../fs/exec.c:1923
(gdb) c
Continuing.
Sending packet: $vCont;s:1#23...Ack

...and now the guest is dead. I can still interrupt it, but it's
otherwise not working properly.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]