[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls |
Date: |
Tue, 21 Jan 2020 17:23:07 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 |
On 21/01/20 17:15, Alex Bennée wrote:
>
> Richard Henderson <address@hidden> writes:
>
>> On 1/21/20 12:13 AM, Alex Bennée wrote:
>>>
>>> Richard Henderson <address@hidden> 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 <address@hidden>
>>>>>
>>>>
>>>> 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.
I agree, if anything I'd place the sigsegv label at the end of the
function but Alex's version is just fine too.
Paolo
[PATCH v2 4/5] linux-user: Add x86_64 vsyscall page to /proc/self/maps, Richard Henderson, 2020/01/16
[PATCH v2 5/5] linux-user: Flush out implementation of gettimeofday, Richard Henderson, 2020/01/16
Re: [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls, Richard Henderson, 2020/01/16
Re: [PATCH v2 0/5] linux-user: Implement x86_64 vsyscalls, Laurent Vivier, 2020/01/28