Re: [PATCH] gdbstub: reset supported features on Supported request

["Alex Bennée" <alex.bennee@linaro.org>]

Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:

> From: Justin Sapp <jsapp@qti.qualcomm.com>
>
> https://sourceware.org/gdb/current/onlinedocs/gdb.html/General-Query-Packets.html#qSupported
> ```
> Whenever the stub receives a ‘qSupported’ request, the supplied set
> of GDB features should override any previous request. This allows GDB to
> put the stub in a known state, even if the stub had previously been
> communicating with a different version of GDB.
> ```
>
> We exposed this by connecting to gdbstub using a gdb client first, and
> then WinDbg (EXDI) for windows kernel debugging. Since they both
> differ in their multiprocess support, this was confusing EXDI.
>
> While we are here, fix the same for fork_events in user mode.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
> ---
>  gdbstub/gdbstub.c | 6 ++++++
>  gdbstub/user.c    | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 0a328b0dd48..dbb2000c817 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -1679,6 +1679,12 @@ void gdb_extend_qsupported_features(char *qflags)
>  
>  static void handle_query_supported(GArray *params, void *user_ctx)
>  {
> +    /*
> +     * Reset multiprocess support as a new gdb client may have different support
> +     * than a previous one.
> +     */
> +    gdbserver_state.multiprocess = false;
> +

This looks fine to me based on the commit write-up. Although it does
highlight that struct GDBState is due a clean-up to separate what is
negotiated features, temporary workspace and accelerator supported
features. Currently its just a random collection of stuff.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>      g_string_printf(gdbserver_state.str_buf, "PacketSize=%x", MAX_PACKET_LENGTH);
>      if (gdb_get_core_xml_file(first_cpu)) {
>          g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+");
> diff --git a/gdbstub/user.c b/gdbstub/user.c
> index d810f0f38c3..9b6d42d210a 100644
> --- a/gdbstub/user.c
> +++ b/gdbstub/user.c
> @@ -701,6 +701,7 @@ fail:
>  
>  void gdb_handle_query_supported_user(const char *gdb_supported)
>  {
> +    gdbserver_user_state.fork_events = false;
>      if (strstr(gdb_supported, "fork-events+")) {
>          gdbserver_user_state.fork_events = true;
>      }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro