The original fix caused problems with spurious characters on other
system emulation. So:

  - instead of spamming output make the warning a trace point
  - ensure we only allow a stop reply if it was 0x3

Suggested-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <456ed3318421dd7946bdfb5ceda7e05332da368c.1690910333.git.quic_mathbern@quicinc.com>
---
 gdbstub/gdbstub.c    | 5 +++--
 gdbstub/trace-events | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index e74ecc78cc..20b6fe03fb 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -2059,9 +2059,10 @@ void gdb_read_byte(uint8_t ch)
          * here, but it does expect a stop reply.
          */
         if (ch != 0x03) {
-            warn_report("gdbstub: client sent packet while target running\n");
+            trace_gdbstub_err_unexpected_runpkt(ch);
+        } else {
+            gdbserver_state.allow_stop_reply = true;
         }
-        gdbserver_state.allow_stop_reply = true;
         vm_stop(RUN_STATE_PAUSED);
     } else
 #endif
diff --git a/gdbstub/trace-events b/gdbstub/trace-events
index 0c18a4d70a..b383bf8d29 100644
--- a/gdbstub/trace-events
+++ b/gdbstub/trace-events
@@ -26,6 +26,7 @@ gdbstub_err_invalid_repeat(uint8_t ch) "got invalid RLE count: 0x%02x"
 gdbstub_err_invalid_rle(void) "got invalid RLE sequence"
 gdbstub_err_checksum_invalid(uint8_t ch) "got invalid command checksum digit: 0x%02x"
 gdbstub_err_checksum_incorrect(uint8_t expected, uint8_t got) "got command packet with incorrect checksum, expected=0x%02x, received=0x%02x"
+gdbstub_err_unexpected_runpkt(uint8_t ch) "unexpected packet (%c) while target running"
 
 # softmmu.c
 gdbstub_hit_watchpoint(const char *type, int cpu_gdb_index, uint64_t vaddr) "Watchpoint hit, type=\"%s\" cpu=%d, vaddr=0x%" PRIx64 ""
-- 
2.39.2

On 8/10/23 08:36, Alex Bennée wrote:
> The original fix caused problems with spurious characters on other
> system emulation. So:
> 
>    - instead of spamming output make the warning a trace point
>    - ensure we only allow a stop reply if it was 0x3
> 
> Suggested-by: Matheus Tavares Bernardino<quic_mathbern@quicinc.com>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> Message-Id:<456ed3318421dd7946bdfb5ceda7e05332da368c.1690910333.git.quic_mathbern@quicinc.com>
> ---
>   gdbstub/gdbstub.c    | 5 +++--
>   gdbstub/trace-events | 1 +
>   2 files changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Richard Henderson <richard.henderson@linaro.org>


r~

On 10/8/23 17:36, Alex Bennée wrote:
> The original fix caused problems with spurious characters on other
> system emulation. So:
> 
>    - instead of spamming output make the warning a trace point
>    - ensure we only allow a stop reply if it was 0x3
> 
> Suggested-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <456ed3318421dd7946bdfb5ceda7e05332da368c.1690910333.git.quic_mathbern@quicinc.com>
> ---
>   gdbstub/gdbstub.c    | 5 +++--
>   gdbstub/trace-events | 1 +
>   2 files changed, 4 insertions(+), 2 deletions(-)


> diff --git a/gdbstub/trace-events b/gdbstub/trace-events
> index 0c18a4d70a..b383bf8d29 100644
> --- a/gdbstub/trace-events
> +++ b/gdbstub/trace-events
> @@ -26,6 +26,7 @@ gdbstub_err_invalid_repeat(uint8_t ch) "got invalid RLE count: 0x%02x"
>   gdbstub_err_invalid_rle(void) "got invalid RLE sequence"
>   gdbstub_err_checksum_invalid(uint8_t ch) "got invalid command checksum digit: 0x%02x"
>   gdbstub_err_checksum_incorrect(uint8_t expected, uint8_t got) "got command packet with incorrect checksum, expected=0x%02x, received=0x%02x"
> +gdbstub_err_unexpected_runpkt(uint8_t ch) "unexpected packet (%c) while target running"

Since unexpected packet can be non-ASCII, better log its hexa value.

Otherwise,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>