On 9/16/19 4:15 PM, Peter Maydell wrote:
> If we fail a semihosting call we should always set the
> semihosting errno to something; we were failing to do
> this for some of the "check inputs for sanity" cases.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> target/arm/arm-semi.c | 45 ++++++++++++++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> index 03e60105c05..51b55816faf 100644
> --- a/target/arm/arm-semi.c
> +++ b/target/arm/arm-semi.c
> @@ -232,11 +232,13 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
> #define GET_ARG(n) do { \
> if (is_a64(env)) { \
> if (get_user_u64(arg ## n, args + (n) * 8)) { \
> - return -1; \
> + errno = EFAULT; \
> + return set_swi_errno(ts, -1); \
> } \
> } else { \
> if (get_user_u32(arg ## n, args + (n) * 4)) { \
> - return -1; \
> + errno = EFAULT; \
> + return set_swi_errno(ts, -1); \
> } \
> } \
> } while (0)
> @@ -287,12 +289,13 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> GET_ARG(2);
> s = lock_user_string(arg0);
> if (!s) {
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - return (uint32_t)-1;
> + errno = EFAULT;
> + return set_swi_errno(ts, -1);
> }
> if (arg1 >= 12) {
> unlock_user(s, arg0, 0);
> - return (uint32_t)-1;
> + errno = EINVAL;
> + return set_swi_errno(ts, -1);
> }
> if (strcmp(s, ":tt") == 0) {
> int result_fileno = arg1 < 4 ? STDIN_FILENO : STDOUT_FILENO;
> @@ -413,8 +416,8 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> } else {
> s = lock_user_string(arg0);
> if (!s) {
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - return (uint32_t)-1;
> + errno = EFAULT;
> + return set_swi_errno(ts, -1);
> }
> ret = set_swi_errno(ts, remove(s));
> unlock_user(s, arg0, 0);
> @@ -432,11 +435,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> char *s2;
> s = lock_user_string(arg0);
> s2 = lock_user_string(arg2);
> - if (!s || !s2)
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - ret = (uint32_t)-1;
> - else
> + if (!s || !s2) {
> + errno = EFAULT;
> + ret = set_swi_errno(ts, -1);
> + } else {
> ret = set_swi_errno(ts, rename(s, s2));
> + }
> if (s2)
> unlock_user(s2, arg2, 0);
> if (s)
> @@ -456,8 +460,8 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> } else {
> s = lock_user_string(arg0);
> if (!s) {
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - return (uint32_t)-1;
> + errno = EFAULT;
> + return set_swi_errno(ts, -1);
> }
> ret = set_swi_errno(ts, system(s));
> unlock_user(s, arg0, 0);
> @@ -517,19 +521,22 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>
> if (output_size > input_size) {
> /* Not enough space to store command-line arguments. */
> - return -1;
> + errno = E2BIG;
> + return set_swi_errno(ts, -1);
> }
>
> /* Adjust the command-line length. */
> if (SET_ARG(1, output_size - 1)) {
> /* Couldn't write back to argument block */
> - return -1;
> + errno = EFAULT;
> + return set_swi_errno(ts, -1);
> }
>
> /* Lock the buffer on the ARM side. */
> output_buffer = lock_user(VERIFY_WRITE, arg0, output_size, 0);
> if (!output_buffer) {
> - return -1;
> + errno = EFAULT;
> + return set_swi_errno(ts, -1);
> }
>
> /* Copy the command-line arguments. */
> @@ -544,7 +551,8 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>
> if (copy_from_user(output_buffer, ts->info->arg_start,
> output_size)) {
> - status = -1;
> + errno = EFAULT;
> + status = set_swi_errno(ts, -1);
> goto out;
> }
>
> @@ -614,7 +622,8 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>
> if (fail) {
> /* Couldn't write back to argument block */
> - return -1;
> + errno = EFAULT;
> + return set_swi_errno(ts, -1);
> }
> }
> return 0;
>