[PULL for 5.0-rc3 0/8] a few small fixes (docker, user, pie and gdbstub)

Alex Bennée posted 8 patches 4 years ago
Only 3 patches received!
configure                                  |  5 +++-
include/exec/gdbstub.h                     | 18 +++++++++++++
linux-user/syscall.c                       | 43 +++++++++++++-----------------
target/arm/gdbstub.c                       |  3 +--
target/i386/gdbstub.c                      |  2 +-
target/m68k/helper.c                       |  4 +--
target/sh4/gdbstub.c                       |  6 ++---
target/xtensa/gdbstub.c                    |  6 ++---
tests/docker/dockerfiles/debian10.docker   |  2 ++
tests/docker/dockerfiles/debian9.docker    |  2 --
tests/docker/dockerfiles/fedora.docker     |  2 +-
tests/docker/dockerfiles/travis.docker     |  2 +-
tests/docker/dockerfiles/ubuntu.docker     |  2 +-
tests/docker/dockerfiles/ubuntu1804.docker |  2 +-
tests/docker/test-misc                     |  2 ++
15 files changed, 57 insertions(+), 44 deletions(-)
[PULL for 5.0-rc3 0/8] a few small fixes (docker, user, pie and gdbstub)
Posted by Alex Bennée 4 years ago
The following changes since commit 2f7cc1fbd6f6655d900ca7f45973b9bd5330c6dd:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2020-04-14 20:09:52 +0100)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-more-fixes-150420-1

for you to fetch changes up to 377f8f08bebea7cd44617b0ac0a2baf307f5f055:

  gdbstub: Introduce gdb_get_float32() to get 32-bit float registers (2020-04-15 11:38:23 +0100)

----------------------------------------------------------------
More small fixes for rc3

  - tweak docker FEATURE flags for document building
  - include sphinx configure check in config.log
  - disable PIE for Windows builds
  - fix /proc/self/stat handling
  - a number of gdbstub fixups following GByteArray conversion

----------------------------------------------------------------
Alex Bennée (4):
      tests/docker: add docs FEATURE flag and use for test-misc
      configure: redirect sphinx-build check to config.log
      configure: disable PIE for Windows builds
      linux-user: fix /proc/self/stat handling

Peter Xu (1):
      gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb

Philippe Mathieu-Daudé (3):
      target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
      gdbstub: Do not use memset() on GByteArray
      gdbstub: Introduce gdb_get_float32() to get 32-bit float registers

 configure                                  |  5 +++-
 include/exec/gdbstub.h                     | 18 +++++++++++++
 linux-user/syscall.c                       | 43 +++++++++++++-----------------
 target/arm/gdbstub.c                       |  3 +--
 target/i386/gdbstub.c                      |  2 +-
 target/m68k/helper.c                       |  4 +--
 target/sh4/gdbstub.c                       |  6 ++---
 target/xtensa/gdbstub.c                    |  6 ++---
 tests/docker/dockerfiles/debian10.docker   |  2 ++
 tests/docker/dockerfiles/debian9.docker    |  2 --
 tests/docker/dockerfiles/fedora.docker     |  2 +-
 tests/docker/dockerfiles/travis.docker     |  2 +-
 tests/docker/dockerfiles/ubuntu.docker     |  2 +-
 tests/docker/dockerfiles/ubuntu1804.docker |  2 +-
 tests/docker/test-misc                     |  2 ++
 15 files changed, 57 insertions(+), 44 deletions(-)

-- 
2.20.1


Re: [PULL for 5.0-rc3 0/8] a few small fixes (docker, user, pie and gdbstub)
Posted by Peter Maydell 4 years ago
On Wed, 15 Apr 2020 at 11:42, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The following changes since commit 2f7cc1fbd6f6655d900ca7f45973b9bd5330c6dd:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2020-04-14 20:09:52 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-more-fixes-150420-1
>
> for you to fetch changes up to 377f8f08bebea7cd44617b0ac0a2baf307f5f055:
>
>   gdbstub: Introduce gdb_get_float32() to get 32-bit float registers (2020-04-15 11:38:23 +0100)
>
> ----------------------------------------------------------------
> More small fixes for rc3
>
>   - tweak docker FEATURE flags for document building
>   - include sphinx configure check in config.log
>   - disable PIE for Windows builds
>   - fix /proc/self/stat handling
>   - a number of gdbstub fixups following GByteArray conversion
>


Applied, thanks.

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

-- PMM

[PULL 2/8] configure: redirect sphinx-build check to config.log
Posted by Alex Bennée 4 years ago
Otherwise it's hard to debug whats going on.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200414200631.12799-8-alex.bennee@linaro.org>

diff --git a/configure b/configure
index 9b1f5b33e45..25f7d915720 100755
--- a/configure
+++ b/configure
@@ -4942,7 +4942,9 @@ has_sphinx_build() {
     # sphinx-build doesn't exist at all or if it is too old.
     mkdir -p "$TMPDIR1/sphinx"
     touch "$TMPDIR1/sphinx/index.rst"
-    "$sphinx_build" $sphinx_werror -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
+    "$sphinx_build" $sphinx_werror -c "$source_path/docs" \
+                    -b html "$TMPDIR1/sphinx" \
+                    "$TMPDIR1/sphinx/out"  >> config.log 2>&1
 }
 
 # Check if tools are available to build documentation.
-- 
2.20.1


[PULL 3/8] configure: disable PIE for Windows builds
Posted by Alex Bennée 4 years ago
It seems on some compilers the test can pass but still give you
broken binaries.

Fixes: d2cd29e30736
Fixes: https://bugs.launchpad.net/qemu/+bug/1871798
Cc: Bug 1871798 <1871798@bugs.launchpad.net>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Howard Spoelstra <hsp.cat7@gmail.com>
Tested-by: James Le Cuirot <chewi@aura-online.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200414200631.12799-9-alex.bennee@linaro.org>

diff --git a/configure b/configure
index 25f7d915720..23b5e93752b 100755
--- a/configure
+++ b/configure
@@ -807,6 +807,7 @@ MINGW32*)
     audio_drv_list=""
   fi
   supported_os="yes"
+  pie="no"
 ;;
 GNU/kFreeBSD)
   bsd="yes"
-- 
2.20.1


[PULL 4/8] linux-user: fix /proc/self/stat handling
Posted by Alex Bennée 4 years ago
In the original bug report long files names in Guix caused
/proc/self/stat be truncated without the trailing ") " as specified in
proc manpage which says:
    (2) comm  %s
           The  filename of the executable, in parentheses.  This
           is visible whether or not the  executable  is  swapped
           out.

In the kernel this is currently done by do_task_stat calling
proc_task_name() which uses a structure limited by TASK_COMM_LEN (16).

Additionally it should only be reporting the executable name rather
than the full path. Fix both these failings while cleaning up the code
to use GString to build up the reported values. As the whole function
is cleaned up also adjust the white space to the current coding style.

Message-ID: <fb4c55fa-d539-67ee-c6c9-de8fb63c8488@inria.fr>
Reported-by: Brice Goglin <Brice.Goglin@inria.fr>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200414200631.12799-10-alex.bennee@linaro.org>

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6495ddc4cda..674f70e70a5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7295,34 +7295,29 @@ static int open_self_stat(void *cpu_env, int fd)
 {
     CPUState *cpu = env_cpu((CPUArchState *)cpu_env);
     TaskState *ts = cpu->opaque;
-    abi_ulong start_stack = ts->info->start_stack;
+    g_autoptr(GString) buf = g_string_new(NULL);
     int i;
 
     for (i = 0; i < 44; i++) {
-      char buf[128];
-      int len;
-      uint64_t val = 0;
-
-      if (i == 0) {
-        /* pid */
-        val = getpid();
-        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
-      } else if (i == 1) {
-        /* app name */
-        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
-      } else if (i == 27) {
-        /* stack bottom */
-        val = start_stack;
-        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
-      } else {
-        /* for the rest, there is MasterCard */
-        snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' ');
-      }
+        if (i == 0) {
+            /* pid */
+            g_string_printf(buf, FMT_pid " ", getpid());
+        } else if (i == 1) {
+            /* app name */
+            gchar *bin = g_strrstr(ts->bprm->argv[0], "/");
+            bin = bin ? bin + 1 : ts->bprm->argv[0];
+            g_string_printf(buf, "(%.15s) ", bin);
+        } else if (i == 27) {
+            /* stack bottom */
+            g_string_printf(buf, TARGET_ABI_FMT_ld " ", ts->info->start_stack);
+        } else {
+            /* for the rest, there is MasterCard */
+            g_string_printf(buf, "0%c", i == 43 ? '\n' : ' ');
+        }
 
-      len = strlen(buf);
-      if (write(fd, buf, len) != len) {
-          return -1;
-      }
+        if (write(fd, buf->str, buf->len) != buf->len) {
+            return -1;
+        }
     }
 
     return 0;
-- 
2.20.1


[PULL 5/8] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
Posted by Alex Bennée 4 years ago
From: Philippe Mathieu-Daudé <philmd@redhat.com>

Since a010bdbe719 the gdbstub API takes a GByteArray*. Unfortunately
we forgot to update the gdb_get_reg*() calls. Do it now.

Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Reported-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200409172509.4078-1-philmd@redhat.com>
Message-Id: <20200414200631.12799-11-alex.bennee@linaro.org>

diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 014657c6372..cad40838956 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env, GByteArray *mem_buf, int n)
 {
     if (n < 8) {
         int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
-        len += gdb_get_reg16(mem_buf + len, 0);
-        len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
+        len += gdb_get_reg16(mem_buf, 0);
+        len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
         return len;
     }
     switch (n) {
-- 
2.20.1


[PULL 6/8] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
Posted by Alex Bennée 4 years ago
From: Peter Xu <peterx@redhat.com>

We should only pass in gdb_get_reg16() with the GByteArray* object
itself, no need to shift.  Without this patch, gdb remote attach will
crash QEMU:

  (gdb) target remote :1234
  Remote debugging using :1234
  Remote communication error.  Target disconnected.: Connection reset by peer.
  $ qemu-system-x86_64 -m 1G -smp 4 ... -s
  ERROR:qemu/gdbstub.c:1843:handle_read_all_regs: assertion failed: (len == gdbserver_state.mem_buf->len)
  Bail out! ERROR:qemu/gdbstub.c:1843:handle_read_all_regs: assertion failed: (len == gdbserver_state.mem_buf->len)

Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200409164954.36902-3-peterx@redhat.com>
Message-Id: <20200414200631.12799-12-alex.bennee@linaro.org>

diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614ee..b98a99500ae 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
         floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
         int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
-        len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+        len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
         return len;
     } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
         n -= IDX_XMM_REGS;
-- 
2.20.1