| Message ID | 20210309141727.12522-4-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | semihosting/next (SYS_HEAPINFO fix) | expand |
On Tue, 9 Mar 2021 at 14:23, Alex Bennée <alex.bennee@linaro.org> wrote: > > As per the spec: > > the PARAMETER REGISTER contains the address of a pointer to a > four-field data block. > > So we need to follow the pointer and place the results of SYS_HEAPINFO > there. > > 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> > --- > semihosting/arm-compat-semi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c > index 733eea1e2d..2ac9226d29 100644 > --- a/semihosting/arm-compat-semi.c > +++ b/semihosting/arm-compat-semi.c > @@ -1210,6 +1210,8 @@ target_ulong do_common_semihosting(CPUState *cs) > retvals[2] = rambase + limit; /* Stack base */ > retvals[3] = rambase; /* Stack limit. */ > #endif > + /* The result array is pointed to by arg0 */ > + args = arg0; > > for (i = 0; i < ARRAY_SIZE(retvals); i++) { > bool fail; > -- No, 'args' is the argument array. That's not the same thing as the data block we're writing, and we shouldn't reassign that variable here. What was wrong with the old arm-semi.c code, which just did appropriate loads and stores here, and worked fine and was not buggy ? thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 9 Mar 2021 at 14:23, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> As per the spec: >> >> the PARAMETER REGISTER contains the address of a pointer to a >> four-field data block. >> >> So we need to follow the pointer and place the results of SYS_HEAPINFO >> there. >> >> 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> >> --- >> semihosting/arm-compat-semi.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c >> index 733eea1e2d..2ac9226d29 100644 >> --- a/semihosting/arm-compat-semi.c >> +++ b/semihosting/arm-compat-semi.c >> @@ -1210,6 +1210,8 @@ target_ulong do_common_semihosting(CPUState *cs) >> retvals[2] = rambase + limit; /* Stack base */ >> retvals[3] = rambase; /* Stack limit. */ >> #endif >> + /* The result array is pointed to by arg0 */ >> + args = arg0; >> >> for (i = 0; i < ARRAY_SIZE(retvals); i++) { >> bool fail; >> -- > > No, 'args' is the argument array. That's not the same thing > as the data block we're writing, and we shouldn't reassign > that variable here. > > What was wrong with the old arm-semi.c code, which just did > appropriate loads and stores here, and worked fine and was > not buggy ? I was just trying avoid repeating too much verbiage. But OK I've reverted to the original code with the new helper: for (i = 0; i < ARRAY_SIZE(retvals); i++) { bool fail; 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 */ errno = EFAULT; return set_swi_errno(cs, -1); } } return 0; > > thanks > -- PMM
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c index 733eea1e2d..2ac9226d29 100644 --- a/semihosting/arm-compat-semi.c +++ b/semihosting/arm-compat-semi.c @@ -1210,6 +1210,8 @@ target_ulong do_common_semihosting(CPUState *cs) retvals[2] = rambase + limit; /* Stack base */ retvals[3] = rambase; /* Stack limit. */ #endif + /* The result array is pointed to by arg0 */ + args = arg0; for (i = 0; i < ARRAY_SIZE(retvals); i++) { bool fail;
As per the spec: the PARAMETER REGISTER contains the address of a pointer to a four-field data block. So we need to follow the pointer and place the results of SYS_HEAPINFO there. 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> --- semihosting/arm-compat-semi.c | 2 ++ 1 file changed, 2 insertions(+)