[Qemu-devel] [PULL for 4.1 0/4] testing and gdbstub fixes

[Qemu-devel] [PULL for 4.1 0/4] testing and gdbstub fixes

[Alex Bennée]

The following changes since commit 6df2cdf44a82426f7a59dcb03f0dd2181ed7fdfa:

  Update version for v4.1.0-rc0 release (2019-07-09 17:21:53 +0100)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-testing-and-gdbstub-100719-1

for you to fetch changes up to 94b2a62bb65b80760bcc59737bec908c9175abf4:

  gdbstub: revert to previous set_reg behaviour (2019-07-10 10:54:46 +0100)

----------------------------------------------------------------
Testing and gdbstub fixes:

  - fix diff-out pass in check-tcg
  - ensure generation of fprem reference
  - fix gdb set_reg fallback

----------------------------------------------------------------
Alex Bennée (4):
      tests/tcg: fix up test-i386-fprem.ref generation
      tests/tcg: fix diff-out pass to properly report failure
      gdbstub: add some notes to the header comment
      gdbstub: revert to previous set_reg behaviour

 gdbstub.c                      | 24 ++++++++++++++++++------
 tests/tcg/Makefile             |  6 +++++-
 tests/tcg/i386/Makefile.target |  4 ++--
 3 files changed, 25 insertions(+), 9 deletions(-)

-- 
2.20.1

[Qemu-devel] [PULL 1/4] tests/tcg: fix up test-i386-fprem.ref generation

[Alex Bennée]

We never shipped the reference data in the source tree because it's
quite big (64M). As a result the only option is to generate it
locally. Although we have a rule to generate the reference file we
missed the dependency and location changes, probably because it's only
run for SLOW test runs.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
index b4033ba3d1..d0eb7023e5 100644
--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -35,9 +35,9 @@ test-i386-fprem.ref: test-i386-fprem
 	$(call quiet-command, ./$< > $@,"GENREF","generating $@")
 
 run-test-i386-fprem: TIMEOUT=60
-run-test-i386-fprem: test-i386-fprem
+run-test-i386-fprem: test-i386-fprem test-i386-fprem.ref
 	$(call run-test,test-i386-fprem, $(QEMU) $<,"$< on $(TARGET_NAME)")
-	$(call diff-out,test-i386-fprem, $(I386_SRC)/$<.ref)
+	$(call diff-out,test-i386-fprem, test-i386-fprem.ref)
 else
 run-test-i386-fprem: test-i386-fprem
 	$(call skip-test, $<, "SLOW")
-- 
2.20.1

[Qemu-devel] [PULL 2/4] tests/tcg: fix diff-out pass to properly report failure

[Alex Bennée]

A side effect of piping the output to head is squash the exit status
of the diff command. Fix this by only doing the pipe if the diff
failed and then ensuring the status is non-zero.

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

diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
index 6fa63cc8d5..9f56768624 100644
--- a/tests/tcg/Makefile
+++ b/tests/tcg/Makefile
@@ -45,7 +45,11 @@ run-test = $(call quiet-command, timeout $(TIMEOUT) $2,"TEST",$3)
 endif
 
 # $1 = test name, $2 = reference
-diff-out = $(call quiet-command, diff -u $1.out $2 | head -n 10,"DIFF","$1.out with $2")
+# to work around the pipe squashing the status we only pipe the result if
+# we know it failed and then force failure at the end.
+diff-out = $(call quiet-command, diff -q $1.out $2 || \
+                                 (diff -u $1.out $2 | head -n 10 && false), \
+                                 "DIFF","$1.out with $2")
 
 # $1 = test name, $2 = reason
 skip-test = @printf "  SKIPPED %s on $(TARGET_NAME) because %s\n" $1 $2
-- 
2.20.1

[Qemu-devel] [PULL 3/4] gdbstub: add some notes to the header comment

[Alex Bennée]

Add a link to the remote protocol spec and an SPDX tag.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

diff --git a/gdbstub.c b/gdbstub.c
index 687c02e598..8363683852 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1,6 +1,10 @@
 /*
  * gdb server stub
  *
+ * This implements a subset of the remote protocol as described in:
+ *
+ *   https://sourceware.org/gdb/onlinedocs/gdb/Remote-Protocol.html
+ *
  * Copyright (c) 2003-2005 Fabrice Bellard
  *
  * This library is free software; you can redistribute it and/or
@@ -15,6 +19,8 @@
  *
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * SPDX-License-Identifier: LGPL-2.0+
  */
 
 #include "qemu/osdep.h"
-- 
2.20.1

[Qemu-devel] [PULL 4/4] gdbstub: revert to previous set_reg behaviour

[Alex Bennée]

The refactoring of handle_set_reg missed the fact we previously had
responded with an empty packet when we were not using XML based
protocols. This broke the fallback behaviour for architectures that
don't have registers defined in QEMU's gdb-xml directory.

Revert to the previous behaviour and clean up the commentary for what
is going on.

Fixes: 62b3320bddd
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Cc: Jon Doron <arilou@gmail.com>

diff --git a/gdbstub.c b/gdbstub.c
index 8363683852..b470aec8ea 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1673,12 +1673,23 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet(gdb_ctx->s, "E22");
 }
 
+/*
+ * handle_set/get_reg
+ *
+ * Older gdb are really dumb, and don't use 'G/g' if 'P/p' is available.
+ * This works, but can be very slow. Anything new enough to understand
+ * XML also knows how to use this properly. However to use this we
+ * need to define a local XML file as well as be talking to a
+ * reasonably modern gdb. Responding with an empty packet will cause
+ * the remote gdb to fallback to older methods.
+ */
+
 static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     int reg_size;
 
     if (!gdb_has_xml) {
-        put_packet(gdb_ctx->s, "E00");
+        put_packet(gdb_ctx->s, "");
         return;
     }
 
@@ -1698,11 +1709,6 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
     int reg_size;
 
-    /*
-     * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
-     * This works, but can be very slow.  Anything new enough to
-     * understand XML also knows how to use this properly.
-     */
     if (!gdb_has_xml) {
         put_packet(gdb_ctx->s, "");
         return;
-- 
2.20.1

Re: [Qemu-devel] [PULL for 4.1 0/4] testing and gdbstub fixes

[Peter Maydell]

On Wed, 10 Jul 2019 at 11:25, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The following changes since commit 6df2cdf44a82426f7a59dcb03f0dd2181ed7fdfa:
>
>   Update version for v4.1.0-rc0 release (2019-07-09 17:21:53 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-testing-and-gdbstub-100719-1
>
> for you to fetch changes up to 94b2a62bb65b80760bcc59737bec908c9175abf4:
>
>   gdbstub: revert to previous set_reg behaviour (2019-07-10 10:54:46 +0100)
>
> ----------------------------------------------------------------
> Testing and gdbstub fixes:
>
>   - fix diff-out pass in check-tcg
>   - ensure generation of fprem reference
>   - fix gdb set_reg fallback
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM