qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	keithp@keithp.com, qemu-arm@nongnu.org, qemu-riscv@nongnu.org,
	"Bug 1915925" <1915925@bugs.launchpad.net>
Subject: [PATCH v5 3/5] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO
Date: Fri, 12 Mar 2021 10:20:27 +0000	[thread overview]
Message-ID: <20210312102029.17017-4-alex.bennee@linaro.org> (raw)
In-Reply-To: <20210312102029.17017-1-alex.bennee@linaro.org>

As per the spec:

  the PARAMETER REGISTER contains the address of a pointer to a
  four-field data block.

So we need to follow arg0 and place the results of SYS_HEAPINFO there.

Fixes: 3c37cfe0b1 ("semihosting: Change internal common-semi interfaces to use CPUState *")
Bug: https://bugs.launchpad.net/bugs/1915925
Cc: Bug 1915925 <1915925@bugs.launchpad.net>
Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - just revert the old behaviour
---
 semihosting/arm-compat-semi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 0f0e129a7c..fe079ca93a 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -1214,7 +1214,11 @@ target_ulong do_common_semihosting(CPUState *cs)
             for (i = 0; i < ARRAY_SIZE(retvals); i++) {
                 bool fail;
 
-                fail = SET_ARG(i, retvals[i]);
+                if (is_64bit_semihosting(env)) {
+                    fail = put_user_u64(retvals[i], arg0 + i * 8);
+                } else {
+                    fail = put_user_u32(retvals[i], arg0 + i * 4);
+                }
 
                 if (fail) {
                     /* Couldn't write back to argument block */
-- 
2.20.1



WARNING: multiple messages have this Message-ID (diff)
From: "Alex Bennée" <1915925@bugs.launchpad.net>
To: qemu-devel@nongnu.org
Subject: [Bug 1915925] [PATCH v5 3/5] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO
Date: Fri, 12 Mar 2021 10:20:27 -0000	[thread overview]
Message-ID: <20210312102029.17017-4-alex.bennee@linaro.org> (raw)
Message-ID: <20210312102027.glRGB1kZvaQki0M-W35n8fJDZapt7IW0mVInEkU2QjE@z> (raw)
In-Reply-To: 161356438332.24036.4652954745285513495.malonedeb@chaenomeles.canonical.com

As per the spec:

  the PARAMETER REGISTER contains the address of a pointer to a
  four-field data block.

So we need to follow arg0 and place the results of SYS_HEAPINFO there.

Fixes: 3c37cfe0b1 ("semihosting: Change internal common-semi interfaces to use CPUState *")
Bug: https://bugs.launchpad.net/bugs/1915925
Cc: Bug 1915925 <1915925@bugs.launchpad.net>
Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - just revert the old behaviour
---
 semihosting/arm-compat-semi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 0f0e129a7c..fe079ca93a 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -1214,7 +1214,11 @@ target_ulong do_common_semihosting(CPUState *cs)
             for (i = 0; i < ARRAY_SIZE(retvals); i++) {
                 bool fail;
 
-                fail = SET_ARG(i, retvals[i]);
+                if (is_64bit_semihosting(env)) {
+                    fail = put_user_u64(retvals[i], arg0 + i * 8);
+                } else {
+                    fail = put_user_u32(retvals[i], arg0 + i * 4);
+                }
 
                 if (fail) {
                     /* Couldn't write back to argument block */
-- 
2.20.1

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1915925

Title:
  ARM semihosting HEAPINFO results wrote to wrong address

Status in QEMU:
  Confirmed

Bug description:
  This affects latest development branch of QEMU.

  According to the ARM spec of the HEAPINFO semihosting call:

  https://developer.arm.com/documentation/100863/0300/Semihosting-
  operations/SYS-HEAPINFO--0x16-?lang=en

  > the PARAMETER REGISTER contains the address of a pointer to a four-
  field data block.

  However, QEMU treated the PARAMETER REGISTER as pointing to a four-
  field data block directly.

  Here is a simple program that can demonstrate this problem:
  https://github.com/iNvEr7/qemu-learn/tree/newlib-bug/semihosting-
  newlib

  This code links with newlib with semihosting mode, which will call the
  HEAPINFO SVC during crt0 routine. When running in QEMU (make run), it
  may crash the program either because of invalid write or memory
  curruption, depending on the compiled program structure.

  Also refer to my discussion with newlib folks:
  https://sourceware.org/pipermail/newlib/2021/018260.html

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1915925/+subscriptions


  parent reply	other threads:[~2021-03-12 10:29 UTC|newest]

Thread overview: 149+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-12 10:20 [PATCH v5 0/5] semihosting/next (SYS_HEAPINFO) Alex Bennée
2021-03-12 10:20 ` [PATCH v5 1/5] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-12 10:20 ` [PATCH v5 2/5] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-12 10:20 ` Alex Bennée [this message]
2021-03-12 10:20   ` [Bug 1915925] [PATCH v5 3/5] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-12 10:32   ` Peter Maydell
2021-03-12 10:32     ` [Bug 1915925] " Peter Maydell
2021-03-12 10:20 ` [PATCH v5 4/5] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-12 10:20 ` [PATCH v5 5/5] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-12 10:35   ` Peter Maydell
2021-03-12 11:23     ` Alex Bennée
2021-03-12 11:27       ` Peter Maydell
2021-03-12 14:08         ` Alex Bennée
  -- strict thread matches above, loose matches on Subject: below --
2021-03-24 14:29 [PULL for 6.0 00/22] various fixes (kernel-doc, semihosting, testing) Alex Bennée
2021-03-24 14:30 ` [PULL 01/22] scripts/kernel-doc: strip QEMU_ from function definitions Alex Bennée
2021-03-24 14:30 ` [PULL 02/22] docs/devel: include the plugin API information from the headers Alex Bennée
2021-03-24 14:30 ` [PULL 03/22] docs/devel: expand style section of memory management Alex Bennée
2021-03-24 14:30 ` [PULL 04/22] tools/virtiofsd: include --socket-group in help Alex Bennée
2021-03-24 14:30 ` [PULL 05/22] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-24 14:30 ` [PULL 06/22] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-24 14:30 ` [PULL 07/22] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-24 14:30   ` [Bug 1915925] " Alex Bennée
2021-03-24 14:30 ` [PULL 08/22] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-24 14:30 ` [PULL 09/22] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-24 14:30 ` [PULL 10/22] gitlab-ci.yml: Merge the trace-backend testing into other jobs Alex Bennée
2021-03-24 14:30 ` [PULL 11/22] configure: Don't use the __atomic_*_16 functions for testing 128-bit support Alex Bennée
2021-03-24 14:30 ` [PULL 12/22] cirrus.yml: Update the FreeBSD task to version 12.2 Alex Bennée
2021-03-24 14:30 ` [PULL 13/22] utils: Tighter tests for qemu_strtosz Alex Bennée
2021-03-24 14:30 ` [PULL 14/22] utils: Work around mingw strto*l bug with 0x Alex Bennée
2021-03-24 14:30 ` [PULL 15/22] gitlab: extend timeouts for CFI builds Alex Bennée
2021-03-24 14:30 ` [PULL 16/22] qdev: define list of archs with virtio-pci or virtio-ccw Alex Bennée
2021-03-24 14:30 ` [PULL 17/22] m68k: add the virtio devices aliases Alex Bennée
2021-03-24 14:30 ` [PULL 18/22] blockdev: with -drive if=virtio, use generic virtio-blk Alex Bennée
2021-03-24 14:30 ` [PULL 19/22] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182" Alex Bennée
2021-03-24 14:30 ` [PULL 20/22] iotests: test m68k with the virt machine Alex Bennée
2021-03-24 14:30 ` [PULL 21/22] iotests: iothreads need ioeventfd Alex Bennée
2021-03-24 14:30 ` [PULL 22/22] gitlab: default to not building the documentation Alex Bennée
2021-03-24 14:57 ` [PULL for 6.0 00/22] various fixes (kernel-doc, semihosting, testing) no-reply
2021-03-24 17:41 ` Peter Maydell
2021-03-23 16:52 [PATCH for 6.0 v2 00/22] fixes for rc1 pre-PR " Alex Bennée
2021-03-23 16:52 ` [PATCH v2 01/22] scripts/kernel-doc: strip QEMU_ from function definitions Alex Bennée
2021-03-23 16:52 ` [PATCH v2 02/22] docs/devel: include the plugin API information from the headers Alex Bennée
2021-03-23 16:52 ` [PATCH v2 03/22] docs/devel: expand style section of memory management Alex Bennée
2021-03-23 16:52 ` [PATCH v2 04/22] tools/virtiofsd: include --socket-group in help Alex Bennée
2021-03-23 16:52 ` [PATCH v2 05/22] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-23 16:52 ` [PATCH v2 06/22] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-23 16:52 ` [PATCH v2 07/22] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-23 16:52   ` [Bug 1915925] " Alex Bennée
2021-03-23 16:52 ` [PATCH v2 08/22] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-23 21:29   ` Alistair Francis
2021-03-23 16:52 ` [PATCH v2 09/22] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-24  6:11   ` Thomas Huth
2021-03-24 14:09   ` Richard Henderson
2021-03-23 16:52 ` [PATCH v2 10/22] gitlab-ci.yml: Merge the trace-backend testing into other jobs Alex Bennée
2021-03-23 16:52 ` [PATCH v2 11/22] configure: Don't use the __atomic_*_16 functions for testing 128-bit support Alex Bennée
2021-03-23 16:52 ` [PATCH v2 12/22] cirrus.yml: Update the FreeBSD task to version 12.2 Alex Bennée
2021-03-23 16:52 ` [PATCH v2 13/22] utils: Tighter tests for qemu_strtosz Alex Bennée
2021-03-23 16:53 ` [PATCH v2 14/22] utils: Work around mingw strto*l bug with 0x Alex Bennée
2021-03-23 16:53 ` [PATCH v2 15/22] gitlab: extend timeouts for CFI builds Alex Bennée
2021-03-23 16:53 ` [PATCH v2 16/22] qdev: define list of archs with virtio-pci or virtio-ccw Alex Bennée
2021-03-23 16:53 ` [PATCH v2 17/22] m68k: add the virtio devices aliases Alex Bennée
2021-03-23 16:53 ` [PATCH v2 18/22] blockdev: with -drive if=virtio, use generic virtio-blk Alex Bennée
2021-03-23 16:53 ` [PATCH v2 19/22] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182" Alex Bennée
2021-03-23 16:53 ` [PATCH v2 20/22] iotests: test m68k with the virt machine Alex Bennée
2021-03-23 16:53 ` [PATCH v2 21/22] iotests: iothreads need ioeventfd Alex Bennée
2021-03-23 16:53 ` [PATCH v2 22/22] gitlab: default to not building the documentation Alex Bennée
2021-03-23 18:21 ` [PATCH for 6.0 v2 00/22] fixes for rc1 pre-PR (kernel-doc, semihosting, testing) no-reply
2021-03-24 13:40 ` Peter Maydell
2021-03-24 14:22   ` Alex Bennée
2021-03-24 15:58     ` Peter Maydell
2021-03-20 13:36 [PATCH for 6.0 v1 00/14] fixes for rc1 " Alex Bennée
2021-03-20 13:36 ` [PATCH v1 01/14] scripts/kernel-doc: strip QEMU_ from function definitions Alex Bennée
2021-03-20 16:04   ` Richard Henderson
2021-03-20 13:36 ` [PATCH v1 02/14] docs/devel: include the plugin API information from the headers Alex Bennée
2021-03-20 13:36 ` [PATCH v1 03/14] docs/devel: expand style section of memory management Alex Bennée
2021-03-20 16:00   ` Richard Henderson
2021-03-20 13:36 ` [PATCH v1 04/14] tools/virtiofsd: include --socket-group in help Alex Bennée
2021-03-22 11:46   ` Stefan Hajnoczi
2021-03-20 13:36 ` [PATCH v1 05/14] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-20 16:03   ` Richard Henderson
2021-03-20 13:36 ` [PATCH v1 06/14] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-20 13:36 ` [PATCH v1 07/14] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-20 13:36   ` [Bug 1915925] " Alex Bennée
2021-03-20 13:37 ` [PATCH v1 08/14] linux-user/riscv: initialise the TaskState heap/stack info Alex Bennée
2021-03-20 16:05   ` Richard Henderson
2021-03-20 13:37 ` [PATCH v1 09/14] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-20 16:11   ` Richard Henderson
2021-03-20 13:37 ` [PATCH v1 10/14] gitlab-ci.yml: Merge the trace-backend testing into other jobs Alex Bennée
2021-03-22 17:03   ` Willian Rampazzo
2021-03-20 13:37 ` [PATCH v1 11/14] configure: Don't use the __atomic_*_16 functions for testing 128-bit support Alex Bennée
2021-03-20 13:37 ` [PATCH v1 12/14] cirrus.yml: Update the FreeBSD task to version 12.2 Alex Bennée
2021-03-20 13:37 ` [PATCH v1 13/14] utils: Tighter tests for qemu_strtosz Alex Bennée
2021-03-20 13:37 ` [PATCH v1 14/14] utils: Work around mingw strto*l bug with 0x Alex Bennée
2021-03-20 13:54 ` [PATCH for 6.0 v1 00/14] fixes for rc1 (kernel-doc, semihosting, testing) no-reply
2021-03-09 17:21 [PATCH v3 0/4] semihosting/next (SYS_HEAPINFO) Alex Bennée
2021-03-09 17:21 ` [PATCH v3 1/4] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-09 17:21 ` [PATCH v3 2/4] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-09 17:21 ` [PATCH v3 4/4] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-11 13:32 ` [PATCH v3 0/4] semihosting/next (SYS_HEAPINFO) Peter Maydell
2021-03-11 20:05   ` Alex Bennée
2021-03-09 14:17 [PATCH v2 0/4] semihosting/next (SYS_HEAPINFO fix) Alex Bennée
2021-03-09 14:17 ` [PATCH v2 1/4] semihosting: move semihosting tests to multiarch Alex Bennée
2021-03-09 14:17 ` [PATCH v2 2/4] semihosting/arm-compat-semi: unify GET/SET_ARG helpers Alex Bennée
2021-03-09 16:33   ` Peter Maydell
2021-03-09 17:02   ` Keith Packard via
2021-03-09 17:24     ` Alex Bennée
2021-03-09 14:17 ` [PATCH v2 3/4] semihosting/arm-compat-semi: deref parameter register for SYS_HEAPINFO Alex Bennée
2021-03-09 14:17   ` [Bug 1915925] " Alex Bennée
2021-03-09 16:35   ` [Bug 1915925] " Peter Maydell
2021-03-09 16:35     ` Peter Maydell
2021-03-09 17:01     ` Alex Bennée
2021-03-09 17:01       ` [Bug 1915925] " Alex Bennée
2021-03-09 14:17 ` [PATCH v2 4/4] tests/tcg: add HeapInfo checking to semihosting test Alex Bennée
2021-03-09 17:08   ` Keith Packard via
2021-03-05 13:54 [PATCH v1 0/3] semihosting/next (move from hw, heapinfo) Alex Bennée
2021-03-05 13:54 ` [PATCH v1 1/3] semihosting: Move include/hw/semihosting/ -> include/semihosting/ Alex Bennée
2021-03-05 13:54 ` [PATCH v1 2/3] semihosting: Move hw/semihosting/ -> semihosting/ Alex Bennée
2021-03-05 13:54 ` [PATCH v1 3/3] semihosting/arg-compat: fix up handling of SYS_HEAPINFO Alex Bennée
2021-03-05 13:54   ` [Bug 1915925] " Alex Bennée
2021-03-05 14:10   ` Peter Maydell
2021-03-05 14:10     ` [Bug 1915925] " Peter Maydell
2021-03-05 20:22     ` Keith Packard via
2021-03-05 20:22       ` [Bug 1915925] " Keith Packard
2021-03-05 22:54       ` Peter Maydell
2021-03-05 22:54         ` [Bug 1915925] " Peter Maydell
2021-03-05 23:54         ` Keith Packard via
2021-03-05 23:54           ` [Bug 1915925] " Keith Packard
2021-03-06  1:27           ` Richard Henderson
2021-03-06 14:07           ` Peter Maydell
2021-03-06 14:07             ` [Bug 1915925] " Peter Maydell
2021-03-06 16:54             ` Keith Packard via
2021-03-06 16:54               ` [Bug 1915925] " Keith Packard
2021-03-08 10:09               ` Peter Maydell
2021-03-08 10:09                 ` [Bug 1915925] " Peter Maydell
2021-03-08 13:36                 ` Alistair Francis
2021-03-08 17:28                   ` Keith Packard via
2021-03-08 17:28                     ` [Bug 1915925] " Keith Packard
2021-03-05 20:19   ` Keith Packard via
2021-03-05 20:19     ` [Bug 1915925] " Keith Packard
2021-02-17 12:19 [Bug 1915925] [NEW] ARM semihosting HEAPINFO results wrote to wrong address iNvEr7
2021-02-17 14:50 ` [Bug 1915925] " Peter Maydell
2021-02-17 15:03 ` Peter Maydell
2021-02-17 16:13 ` Philippe Mathieu-Daudé
2021-03-05 13:33 ` Alex Bennée
2021-03-09 17:21 ` [Bug 1915925] [PATCH v3 3/4] semihosting/arm-compat-semi: don't use SET_ARG to report SYS_HEAPINFO Alex Bennée
2021-03-09 17:21   ` Alex Bennée
2021-03-15 12:46 ` [Bug 1915925] Re: ARM semihosting HEAPINFO results wrote to wrong address Alex Bennée
2021-03-25 11:47 ` Alex Bennée
2021-04-30  8:54 ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210312102029.17017-4-alex.bennee@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=1915925@bugs.launchpad.net \
    --cc=keithp@keithp.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).