[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V2] tools/tests: Add EIP check to test_x86_emulator.c



On 07/08/14 12:10, Razvan Cojocaru wrote:
> The test now also checks that EIP was modified after emulating
> instructions after (and including) the "movq %mm3,(%ecx)..."
> code block.
>
> Changes since V1:
>  - Now checking if the value in EIP is correct instead of simply
>    checking that EIP has been modified.

You should state in the commit message that this change depends on Jan's
fix to the emulator itself.  (I cant remember whether OSSTest runs this
test harness)

>
> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> ---
>  tools/tests/x86_emulator/test_x86_emulator.c |   80 
> +++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 20 deletions(-)
>
> diff --git a/tools/tests/x86_emulator/test_x86_emulator.c 
> b/tools/tests/x86_emulator/test_x86_emulator.c
> index 0a00d5a..8d9894b 100644
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -602,20 +602,24 @@ int main(int argc, char **argv)
>      printf("%-40s", "Testing movq %mm3,(%ecx)...");
>      if ( stack_exec && cpu_has_mmx )
>      {
> -        extern const unsigned char movq_to_mem[];
> +        extern const unsigned char movq_to_mem[], movq_to_mem_end[];
> +        unsigned long instr_size;
>  
>          asm volatile ( "pcmpeqb %%mm3, %%mm3\n"
>                         ".pushsection .test, \"a\", @progbits\n"
>                         "movq_to_mem: movq %%mm3, (%0)\n"
> +                       "movq_to_mem_end:\n"
>                         ".popsection" :: "c" (NULL) );
>  
> +        instr_size = movq_to_mem_end - movq_to_mem;
>          memcpy(instr, movq_to_mem, 15);
>          memset(res, 0x33, 64);
>          memset(res + 8, 0xff, 8);
>          regs.eip    = (unsigned long)&instr[0];
>          regs.ecx    = (unsigned long)res;
>          rc = x86_emulate(&ctxt, &emulops);
> -        if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) )
> +        if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 8, 32) ||
> +             (regs.eip != (unsigned long)&instr[0] + instr_size) )

I presume that this pointer arithmetic works out correctly, but Xen
style would insist on brackets, and it would be rather more clear as:

(unsigned long)(&instr[0] + instr_size)

With that change, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.