Richard Henderson <richard.hender...@linaro.org> writes:
> On 1/21/20 12:13 AM, Alex Bennée wrote:
>>
>> 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;
>> }
>>
>
> Only if you have a violent goto allergy.
gotos have their place but jumping backwards is confusing to eye. If the
compiler want to mess with layout after then it is free to do so.
--
Alex Bennée
