xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] tools/tests: Add EIP check to test_x86_emulator.c
@ 2014-08-07 11:10 Razvan Cojocaru
  2014-08-07 11:29 ` Andrew Cooper
  0 siblings, 1 reply; 2+ messages in thread
From: Razvan Cojocaru @ 2014-08-07 11:10 UTC (permalink / raw)
  To: xen-devel
  Cc: ian.campbell, Razvan Cojocaru, stefano.stabellini, andrew.cooper3,
	ian.jackson, JBeulich

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.

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) )
             goto fail;
         printf("okay\n");
     }
@@ -625,19 +629,23 @@ int main(int argc, char **argv)
     printf("%-40s", "Testing movq (%edx),%mm5...");
     if ( stack_exec && cpu_has_mmx )
     {
-        extern const unsigned char movq_from_mem[];
+        extern const unsigned char movq_from_mem[], movq_from_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "pcmpgtb %%mm5, %%mm5\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "movq_from_mem: movq (%0), %%mm5\n"
+                       "movq_from_mem_end:\n"
                        ".popsection" :: "d" (NULL) );
 
+        instr_size = movq_from_mem_end - movq_from_mem;
         memcpy(instr, movq_from_mem, 15);
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = 0;
         regs.edx    = (unsigned long)res;
         rc = x86_emulate(&ctxt, &emulops);
-        if ( rc != X86EMUL_OKAY )
+        if ( rc != X86EMUL_OKAY ||
+             (regs.eip != (unsigned long)&instr[0] + instr_size) )
             goto fail;
         asm ( "pcmpeqb %%mm3, %%mm3\n\t"
               "pcmpeqb %%mm5, %%mm3\n\t"
@@ -652,20 +660,24 @@ int main(int argc, char **argv)
     printf("%-40s", "Testing movdqu %xmm2,(%ecx)...");
     if ( stack_exec && cpu_has_sse2 )
     {
-        extern const unsigned char movdqu_to_mem[];
+        extern const unsigned char movdqu_to_mem[], movdqu_to_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "pcmpeqb %%xmm2, %%xmm2\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "movdqu_to_mem: movdqu %%xmm2, (%0)\n"
+                       "movdqu_to_mem_end:\n"
                        ".popsection" :: "c" (NULL) );
 
+        instr_size = movdqu_to_mem_end - movdqu_to_mem;
         memcpy(instr, movdqu_to_mem, 15);
         memset(res, 0x55, 64);
         memset(res + 8, 0xff, 16);
         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) )
             goto fail;
         printf("okay\n");
     }
@@ -675,19 +687,23 @@ int main(int argc, char **argv)
     printf("%-40s", "Testing movdqu (%edx),%xmm4...");
     if ( stack_exec && cpu_has_sse2 )
     {
-        extern const unsigned char movdqu_from_mem[];
+        extern const unsigned char movdqu_from_mem[], movdqu_from_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "pcmpgtb %%xmm4, %%xmm4\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "movdqu_from_mem: movdqu (%0), %%xmm4\n"
+                       "movdqu_from_mem_end:\n"
                        ".popsection" :: "d" (NULL) );
 
+        instr_size = movdqu_from_mem_end - movdqu_from_mem;
         memcpy(instr, movdqu_from_mem, 15);
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = 0;
         regs.edx    = (unsigned long)res;
         rc = x86_emulate(&ctxt, &emulops);
-        if ( rc != X86EMUL_OKAY )
+        if ( rc != X86EMUL_OKAY ||
+             (regs.eip != (unsigned long)&instr[0] + instr_size) )
             goto fail;
         asm ( "pcmpeqb %%xmm2, %%xmm2\n\t"
               "pcmpeqb %%xmm4, %%xmm2\n\t"
@@ -702,13 +718,16 @@ int main(int argc, char **argv)
     printf("%-40s", "Testing vmovdqu %ymm2,(%ecx)...");
     if ( stack_exec && cpu_has_avx )
     {
-        extern const unsigned char vmovdqu_to_mem[];
+        extern const unsigned char vmovdqu_to_mem[], vmovdqu_to_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "vpcmpeqb %%xmm2, %%xmm2, %%xmm2\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "vmovdqu_to_mem: vmovdqu %%ymm2, (%0)\n"
+                       "vmovdqu_to_mem_end:\n"
                        ".popsection" :: "c" (NULL) );
 
+        instr_size = vmovdqu_to_mem_end - vmovdqu_to_mem;
         memcpy(instr, vmovdqu_to_mem, 15);
         memset(res, 0x55, 128);
         memset(res + 16, 0xff, 16);
@@ -716,7 +735,8 @@ int main(int argc, char **argv)
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = (unsigned long)res;
         rc = x86_emulate(&ctxt, &emulops);
-        if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 16, 64) )
+        if ( (rc != X86EMUL_OKAY) || memcmp(res, res + 16, 64) ||
+             (regs.eip != (unsigned long)&instr[0] + instr_size) )
             goto fail;
         printf("okay\n");
     }
@@ -726,7 +746,8 @@ int main(int argc, char **argv)
     printf("%-40s", "Testing vmovdqu (%edx),%ymm4...");
     if ( stack_exec && cpu_has_avx )
     {
-        extern const unsigned char vmovdqu_from_mem[];
+        extern const unsigned char vmovdqu_from_mem[], vmovdqu_from_mem_end[];
+        unsigned long instr_size;
 
 #if 0 /* Don't use AVX2 instructions for now */
         asm volatile ( "vpcmpgtb %%ymm4, %%ymm4, %%ymm4\n"
@@ -736,15 +757,18 @@ int main(int argc, char **argv)
 #endif
                        ".pushsection .test, \"a\", @progbits\n"
                        "vmovdqu_from_mem: vmovdqu (%0), %%ymm4\n"
+                       "vmovdqu_from_mem_end:\n"
                        ".popsection" :: "d" (NULL) );
 
+        instr_size = vmovdqu_from_mem_end - vmovdqu_from_mem;
         memcpy(instr, vmovdqu_from_mem, 15);
         memset(res + 4, 0xff, 16);
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = 0;
         regs.edx    = (unsigned long)res;
         rc = x86_emulate(&ctxt, &emulops);
-        if ( rc != X86EMUL_OKAY )
+        if ( rc != X86EMUL_OKAY ||
+             (regs.eip != (unsigned long)&instr[0] + instr_size) )
             goto fail;
 #if 0 /* Don't use AVX2 instructions for now */
         asm ( "vpcmpeqb %%ymm2, %%ymm2, %%ymm2\n\t"
@@ -771,20 +795,24 @@ int main(int argc, char **argv)
     memset(res + 10, 0x66, 8);
     if ( stack_exec && cpu_has_sse2 )
     {
-        extern const unsigned char movsd_to_mem[];
+        extern const unsigned char movsd_to_mem[], movsd_to_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "movlpd %0, %%xmm5\n\t"
                        "movhpd %0, %%xmm5\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "movsd_to_mem: movsd %%xmm5, (%1)\n"
+                       "movsd_to_mem_end:\n"
                        ".popsection" :: "m" (res[10]), "c" (NULL) );
 
+        instr_size = movsd_to_mem_end - movsd_to_mem;
         memcpy(instr, movsd_to_mem, 15);
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = (unsigned long)(res + 2);
         regs.edx    = 0;
         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) )
             goto fail;
         printf("okay\n");
     }
@@ -797,19 +825,23 @@ int main(int argc, char **argv)
     printf("%-40s", "Testing movaps (%edx),%xmm7...");
     if ( stack_exec && cpu_has_sse )
     {
-        extern const unsigned char movaps_from_mem[];
+        extern const unsigned char movaps_from_mem[], movaps_from_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "xorps %%xmm7, %%xmm7\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "movaps_from_mem: movaps (%0), %%xmm7\n"
+                       "movaps_from_mem_end:\n"
                        ".popsection" :: "d" (NULL) );
 
+        instr_size = movaps_from_mem_end - movaps_from_mem;
         memcpy(instr, movaps_from_mem, 15);
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = 0;
         regs.edx    = (unsigned long)res;
         rc = x86_emulate(&ctxt, &emulops);
-        if ( rc != X86EMUL_OKAY )
+        if ( rc != X86EMUL_OKAY ||
+             (regs.eip != (unsigned long)&instr[0] + instr_size) )
             goto fail;
         asm ( "cmpeqps %1, %%xmm7\n\t"
               "movmskps %%xmm7, %0" : "=r" (rc) : "m" (res[8]) );
@@ -825,19 +857,23 @@ int main(int argc, char **argv)
     memset(res + 10, 0x77, 8);
     if ( stack_exec && cpu_has_avx )
     {
-        extern const unsigned char vmovsd_to_mem[];
+        extern const unsigned char vmovsd_to_mem[], vmovsd_to_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "vbroadcastsd %0, %%ymm5\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "vmovsd_to_mem: vmovsd %%xmm5, (%1)\n"
+                       "vmovsd_to_mem_end:\n"
                        ".popsection" :: "m" (res[10]), "c" (NULL) );
 
+        instr_size = vmovsd_to_mem_end - vmovsd_to_mem;
         memcpy(instr, vmovsd_to_mem, 15);
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = (unsigned long)(res + 2);
         regs.edx    = 0;
         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) )
             goto fail;
         printf("okay\n");
     }
@@ -850,19 +886,23 @@ int main(int argc, char **argv)
     printf("%-40s", "Testing vmovaps (%edx),%ymm7...");
     if ( stack_exec && cpu_has_avx )
     {
-        extern const unsigned char vmovaps_from_mem[];
+        extern const unsigned char vmovaps_from_mem[], vmovaps_from_mem_end[];
+        unsigned long instr_size;
 
         asm volatile ( "vxorps %%ymm7, %%ymm7, %%ymm7\n"
                        ".pushsection .test, \"a\", @progbits\n"
                        "vmovaps_from_mem: vmovaps (%0), %%ymm7\n"
+                       "vmovaps_from_mem_end:\n"
                        ".popsection" :: "d" (NULL) );
 
+        instr_size = vmovaps_from_mem_end - vmovaps_from_mem;
         memcpy(instr, vmovaps_from_mem, 15);
         regs.eip    = (unsigned long)&instr[0];
         regs.ecx    = 0;
         regs.edx    = (unsigned long)res;
         rc = x86_emulate(&ctxt, &emulops);
-        if ( rc != X86EMUL_OKAY )
+        if ( rc != X86EMUL_OKAY ||
+             (regs.eip != (unsigned long)&instr[0] + instr_size) )
             goto fail;
         asm ( "vcmpeqps %1, %%ymm7, %%ymm0\n\t"
               "vmovmskps %%ymm0, %0" : "=r" (rc) : "m" (res[8]) );
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH V2] tools/tests: Add EIP check to test_x86_emulator.c
  2014-08-07 11:10 [PATCH V2] tools/tests: Add EIP check to test_x86_emulator.c Razvan Cojocaru
@ 2014-08-07 11:29 ` Andrew Cooper
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cooper @ 2014-08-07 11:29 UTC (permalink / raw)
  To: Razvan Cojocaru, xen-devel
  Cc: ian.jackson, ian.campbell, JBeulich, stefano.stabellini

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-08-07 11:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-07 11:10 [PATCH V2] tools/tests: Add EIP check to test_x86_emulator.c Razvan Cojocaru
2014-08-07 11:29 ` Andrew Cooper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).