Re: [PATCH v2 3/5] linux-user/i386: Emulate x86_64 vsyscalls

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@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.bennee@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