| Message ID | 20190916141544.17540-3-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement semihosting v2.0 | expand |
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; >
On 9/16/19 7:15 AM, 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> > --- > target/arm/arm-semi.c | 45 ++++++++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 18 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
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;