semihosting/arm-compat-semi.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
The previous numbers were a guess at best. While we could extract the
information from a loaded ELF file via -kernel we could still get
tripped up by self decompressing or relocating code. Besides sane
library code has access to the same symbols in run time to make a
determination of the location of the heap.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Andrew <astrauss11@gmail.com>
---
semihosting/arm-compat-semi.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 1c29146dcf..041b4f6c04 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -1165,12 +1165,10 @@ target_ulong do_common_semihosting(CPUState *cs)
case TARGET_SYS_HEAPINFO:
{
target_ulong retvals[4];
- target_ulong limit;
int i;
#ifdef CONFIG_USER_ONLY
+ target_ulong limit;
TaskState *ts = cs->opaque;
-#else
- target_ulong rambase = common_semi_rambase(cs);
#endif
GET_ARG(0);
@@ -1201,12 +1199,15 @@ target_ulong do_common_semihosting(CPUState *cs)
retvals[2] = ts->stack_base;
retvals[3] = 0; /* Stack limit. */
#else
- limit = current_machine->ram_size;
- /* TODO: Make this use the limit of the loaded application. */
- retvals[0] = rambase + limit / 2;
- retvals[1] = rambase + limit;
- retvals[2] = rambase + limit; /* Stack base */
- retvals[3] = rambase; /* Stack limit. */
+ /*
+ * Reporting 0 indicates we couldn't calculate the real
+ * values which should force most software to fall back to
+ * using information it has.
+ */
+ retvals[0] = 0; /* Heap Base */
+ retvals[1] = 0; /* Heap Limit */
+ retvals[2] = 0; /* Stack base */
+ retvals[3] = 0; /* Stack limit. */
#endif
for (i = 0; i < ARRAY_SIZE(retvals); i++) {
--
2.20.1
On Tue, 1 Jun 2021 at 10:12, Alex Bennée <alex.bennee@linaro.org> wrote: > > The previous numbers were a guess at best. While we could extract the > information from a loaded ELF file via -kernel we could still get > tripped up by self decompressing or relocating code. Besides sane > library code has access to the same symbols in run time to make a > determination of the location of the heap. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Andrew <astrauss11@gmail.com> > --- > semihosting/arm-compat-semi.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) This seems like a pretty good candidate for breaking existing working binaries. How much testing against different varieties of guest-code-using-semihosting have you done ? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 1 Jun 2021 at 10:12, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> The previous numbers were a guess at best. While we could extract the >> information from a loaded ELF file via -kernel we could still get >> tripped up by self decompressing or relocating code. Besides sane >> library code has access to the same symbols in run time to make a >> determination of the location of the heap. >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: Andrew <astrauss11@gmail.com> >> --- >> semihosting/arm-compat-semi.c | 19 ++++++++++--------- >> 1 file changed, 10 insertions(+), 9 deletions(-) > > This seems like a pretty good candidate for breaking existing > working binaries. How much testing against different varieties of > guest-code-using-semihosting have you done ? None, which is why it's an RFC - but at least one user reported newlib attempts to use the numbers we gave it rather than falling back to numbers it knew from the build and getting it wrong. I suspect any code that doesn't have a fallback path is getting it right more by luck than judgement though. I'd be curious to hear of code that relies on the numbers it gets from QEMU. > > thanks > -- PMM -- Alex Bennée
On Tue, 1 Jun 2021 at 11:07, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Tue, 1 Jun 2021 at 10:12, Alex Bennée <alex.bennee@linaro.org> wrote: > >> > >> The previous numbers were a guess at best. While we could extract the > >> information from a loaded ELF file via -kernel we could still get > >> tripped up by self decompressing or relocating code. Besides sane > >> library code has access to the same symbols in run time to make a > >> determination of the location of the heap. > >> > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >> Cc: Andrew <astrauss11@gmail.com> > >> --- > >> semihosting/arm-compat-semi.c | 19 ++++++++++--------- > >> 1 file changed, 10 insertions(+), 9 deletions(-) > > > > This seems like a pretty good candidate for breaking existing > > working binaries. How much testing against different varieties of > > guest-code-using-semihosting have you done ? > > None, which is why it's an RFC - but at least one user reported newlib > attempts to use the numbers we gave it rather than falling back to > numbers it knew from the build and getting it wrong. I suspect any code > that doesn't have a fallback path is getting it right more by luck than > judgement though. I'd be curious to hear of code that relies on the > numbers it gets from QEMU. Well, newlib is the main one I had in mind -- does it have a fallback codepath? -- PMM
Yeah, newlib falls back to using the symbols __stack and end to figure out where to start the stack and heap if 0 is returned by SYS_HEAPINFO. -- Andrew Strauss On Tue, Jun 1, 2021 at 6:31 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Tue, 1 Jun 2021 at 11:07, Alex Bennée <alex.bennee@linaro.org> wrote: > > > > > > Peter Maydell <peter.maydell@linaro.org> writes: > > > > > On Tue, 1 Jun 2021 at 10:12, Alex Bennée <alex.bennee@linaro.org> > wrote: > > >> > > >> The previous numbers were a guess at best. While we could extract the > > >> information from a loaded ELF file via -kernel we could still get > > >> tripped up by self decompressing or relocating code. Besides sane > > >> library code has access to the same symbols in run time to make a > > >> determination of the location of the heap. > > >> > > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > >> Cc: Andrew <astrauss11@gmail.com> > > >> --- > > >> semihosting/arm-compat-semi.c | 19 ++++++++++--------- > > >> 1 file changed, 10 insertions(+), 9 deletions(-) > > > > > > This seems like a pretty good candidate for breaking existing > > > working binaries. How much testing against different varieties of > > > guest-code-using-semihosting have you done ? > > > > None, which is why it's an RFC - but at least one user reported newlib > > attempts to use the numbers we gave it rather than falling back to > > numbers it knew from the build and getting it wrong. I suspect any code > > that doesn't have a fallback path is getting it right more by luck than > > judgement though. I'd be curious to hear of code that relies on the > > numbers it gets from QEMU. > > Well, newlib is the main one I had in mind -- does it have a fallback > codepath? > > -- PMM >
Tested-by: Andrew Strauss <astrauss11@gmail.com>
Reviewed-by: Andrew Strauss <astrauss11@gmail.com>
On Tue, Jun 1, 2021 at 5:07 AM Alex Bennée <alex.bennee@linaro.org> wrote:
> The previous numbers were a guess at best. While we could extract the
> information from a loaded ELF file via -kernel we could still get
> tripped up by self decompressing or relocating code. Besides sane
> library code has access to the same symbols in run time to make a
> determination of the location of the heap.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Andrew <astrauss11@gmail.com>
> ---
> semihosting/arm-compat-semi.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index 1c29146dcf..041b4f6c04 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -1165,12 +1165,10 @@ target_ulong do_common_semihosting(CPUState *cs)
> case TARGET_SYS_HEAPINFO:
> {
> target_ulong retvals[4];
> - target_ulong limit;
> int i;
> #ifdef CONFIG_USER_ONLY
> + target_ulong limit;
> TaskState *ts = cs->opaque;
> -#else
> - target_ulong rambase = common_semi_rambase(cs);
> #endif
>
> GET_ARG(0);
> @@ -1201,12 +1199,15 @@ target_ulong do_common_semihosting(CPUState *cs)
> retvals[2] = ts->stack_base;
> retvals[3] = 0; /* Stack limit. */
> #else
> - limit = current_machine->ram_size;
> - /* TODO: Make this use the limit of the loaded application.
> */
> - retvals[0] = rambase + limit / 2;
> - retvals[1] = rambase + limit;
> - retvals[2] = rambase + limit; /* Stack base */
> - retvals[3] = rambase; /* Stack limit. */
> + /*
> + * Reporting 0 indicates we couldn't calculate the real
> + * values which should force most software to fall back to
> + * using information it has.
> + */
> + retvals[0] = 0; /* Heap Base */
> + retvals[1] = 0; /* Heap Limit */
> + retvals[2] = 0; /* Stack base */
> + retvals[3] = 0; /* Stack limit. */
> #endif
>
> for (i = 0; i < ARRAY_SIZE(retvals); i++) {
> --
> 2.20.1
>
>
© 2016 - 2024 Red Hat, Inc.