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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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@bitdefender.com>
> ---
>  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@citrix.com>

~Andrew