Re: [PATCH v2] gdbstub: Switch to the thread receiving a signal

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

Pavel Labath <pavel@labath.sk> writes:

> Respond with Txxthread:yyyy; instead of a plain Sxx to indicate which
> thread received the signal. Otherwise, the debugger will associate it
> with the main one. Also automatically select this thread, as that is
> what gdb expects.

Just for reference it's best to post vN's in a new thread as the
Replied-to field can confuse some of the automatic tools (b4, patchew
etc).

> Signed-off-by: Pavel Labath <pavel@labath.sk>
> ---
>  gdbstub.c                                     |  8 ++-
>  tests/tcg/multiarch/Makefile.target           | 10 +++-
>  .../gdbstub/test-thread-breakpoint.py         | 60 +++++++++++++++++++
>  3 files changed, 75 insertions(+), 3 deletions(-)
>  create mode 100644 tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 36b85aa..23baaef 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -3138,8 +3138,12 @@ gdb_handlesig(CPUState *cpu, int sig)
>      tb_flush(cpu);
>  
>      if (sig != 0) {
> -        snprintf(buf, sizeof(buf), "S%02x", target_signal_to_gdb(sig));
> -        put_packet(buf);
> +        gdb_set_stop_cpu(cpu);
> +        g_string_printf(gdbserver_state.str_buf,
> +                        "T%02xthread:", target_signal_to_gdb(sig));
> +        gdb_append_thread_id(cpu, gdbserver_state.str_buf);
> +        g_string_append_c(gdbserver_state.str_buf, ';');
> +        put_strbuf();
>      }
>      /* put_packet() might have detected that the peer terminated the
>         connection.  */
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index 6ccb592..c84683f 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -70,11 +70,19 @@ run-gdbstub-qxfer-auxv-read: sha1
>  		--bin $< --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \
>  	"basic gdbstub qXfer:auxv:read support")
>  
> +run-gdbstub-thread-breakpoint: testthread
> +	$(call run-test, $@, $(GDB_SCRIPT) \
> +		--gdb $(HAVE_GDB_BIN) \
> +		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> +		--bin $< --test $(MULTIARCH_SRC)/gdbstub/test-thread-breakpoint.py, \
> +	"hitting a breakpoint on non-main thread")
> +
>  else
>  run-gdbstub-%:
>  	$(call skip-test, "gdbstub test $*", "need working gdb")
>  endif
> -EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read
> +EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
> +	      run-gdbstub-thread-breakpoint
>  
>  # ARM Compatible Semi Hosting Tests
>  #
> diff --git a/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
> new file mode 100644
> index 0000000..798d508
> --- /dev/null
> +++ b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
> @@ -0,0 +1,60 @@
> +from __future__ import print_function
> +#
> +# Test auxiliary vector is loaded via gdbstub

I'm fairly sure this isn't what the test is doing...

> +#
> +# This is launched via tests/guest-debug/run-test.py
> +#
> +
> +import gdb
> +import sys
> +
> +failcount = 0
> +
> +def report(cond, msg):
> +    "Report success/fail of test"
> +    if cond:
> +        print ("PASS: %s" % (msg))
> +    else:
> +        print ("FAIL: %s" % (msg))
> +        global failcount
> +        failcount += 1
> +
> +def run_test():
> +    "Run through the tests one by one"
> +
> +    sym, ok = gdb.lookup_symbol("thread1_func")
> +    gdb.execute("b thread1_func")
> +    gdb.execute("c")
> +
> +    frame = gdb.selected_frame()
> +    report(str(frame.function()) == "thread1_func", "break @
> %s"%frame)

I think we can do better here by checking gdb.selected_thread() and
ensuring the num (or global_num) is not 1. Also maybe check the
is_stopped() status:

  https://sourceware.org/gdb/current/onlinedocs/gdb/Threads-In-Python.html

I noticed while running the test that output still continued for some
time from the other thread but it was still doing that pre this change
so I'm not quite sure what was going on there.

> +
> +#
> +# This runs as the script it sourced (via -x, via run-test.py)
> +#
> +try:
> +    inferior = gdb.selected_inferior()
> +    arch = inferior.architecture()
> +    print("ATTACHED: %s" % arch.name())
> +except (gdb.error, AttributeError):
> +    print("SKIPPING (not connected)", file=sys.stderr)
> +    exit(0)
> +
> +if gdb.parse_and_eval('$pc') == 0:
> +    print("SKIP: PC not set")
> +    exit(0)
> +
> +try:
> +    # These are not very useful in scripts
> +    gdb.execute("set pagination off")
> +    gdb.execute("set confirm off")
> +
> +    # Run the actual tests
> +    run_test()
> +except (gdb.error):
> +    print ("GDB Exception: %s" % (sys.exc_info()[0]))
> +    failcount += 1
> +    pass
> +
> +print("All tests complete: %d failures" % failcount)
> +exit(failcount)

I also tried some manual testing:

  ➜  ./qemu-aarch64 -g 1234 tests/tcg/aarch64-linux-user/testthread
  fish: “./qemu-aarch64 -g 1234 tests/tc…” terminated by signal SIGSEGV (Address boundary error)
  🕙12:33:49 alex@zen:qemu.git/builds/arm.all  on  gdbstub/next [$!?⇡] took 12s [⚡ SEGV] 
  ✗

where in the other window I did:

  0x00000000004005d0 in _start ()
  (gdb) hbreak thread2_func
  Hardware assisted breakpoint 1 at 0x400824: file /home/alex/lsrc/qemu.git/tests/tcg/multiarch/testthread.c, line 34.
  (gdb) hbreak thread1_func
  Hardware assisted breakpoint 2 at 0x400798: file /home/alex/lsrc/qemu.git/tests/tcg/multiarch/testthread.c, line 22.
  (gdb) c
  Continuing.
  [New Thread 1.2748248]
  Remote connection closed

which seems to indicate some problems with breaking on multiple threads.
Maybe this is related to the weird output I was seeing above?

-- 
Alex Bennée