[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 3/4] semihosting/arm-compat-semi: deref parameter register
From: |
Alex Bennée |
Subject: |
Re: [PATCH v2 3/4] semihosting/arm-compat-semi: deref parameter register for SYS_HEAPINFO |
Date: |
Tue, 09 Mar 2021 17:01:28 +0000 |
User-agent: |
mu4e 1.5.8; emacs 28.0.50 |
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
--
Alex Bennée