Richard Henderson <richard.hender...@linaro.org> writes:
> On 1/20/20 1:48 AM, Alex Bennée wrote: >>> + default: >>> + sigsegv: >> >> this label looks a little extraneous. >> >> Otherwise: >> >> Reviewed-by: Alex Bennée <alex.ben...@linaro.org> >> > > Look a little further down: > >> + default: >> + sigsegv: >> + /* Like force_sig(SIGSEGV). */ >> + gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0); >> + return; >> + } >> + >> + /* >> + * Validate the return address. >> + * Note that the kernel treats this the same as an invalid entry point. >> + */ >> + if (get_user_u64(caller, env->regs[R_ESP])) { >> + goto sigsegv; >> + } Wouldn't this read better: /* * Validate the entry point. We have already validated the page * during translation, now verify the offset. */ switch (env->eip & ~TARGET_PAGE_MASK) { case 0x000: syscall = TARGET_NR_gettimeofday; break; case 0x400: syscall = TARGET_NR_time; break; case 0x800: syscall = TARGET_NR_getcpu; break; default: syscall = -1; break; } /* * If we have an invalid entry point or an invalid return address we * generate a SIGSEG. */ if (syscall < 0 || get_user_u64(caller, env->regs[R_ESP])) { gen_signal(env, TARGET_SIGSEGV, TARGET_SI_KERNEL, 0); return; } -- Alex Bennée