Re: [PATCH 1/2] x86: optimize XCHG to MOV for same-register forms

Michael Matz <[email protected]>
Newsgroups gmane.comp.debugging.valgrind.devel,gmane.comp.gnu.binutils
Message-ID <[email protected]> 
Hey,

On Tue, 7 Jul 2026, Mark Wielaard wrote:

> OK. So it is the best I am willing to do. Because I personally wouldn't 
> want to do all that unnecessary work if we can just document to not to 
> use an optimizing compiler when building and we can detect that at 
> runtime. Patches welcome of course if you (or someone else) believes 
> they can do better and are willing to do the extra work.

Attached.  Using .byte in the public header, recognizing the mov form and 
giving some hint on unrecognized bytes after recognized preamble.

Tested with a normal make check and with a program excercising the 
problematic client requests (just playing with VALGRIND_MAKE_MEM_NOACCESS 
and seeing that I get unrecognized insns before and proper behaviour 
after), on 32bit and 64bit.  (Of course with an assembler that does the 
rewriting).


Ciao,
Michael.

_______________________________________________
Valgrind-developers mailing list
Valgrind-developers-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/valgrind-developers
0001-x86-x86-64-deal-with-rewriting-assemblers.patch (text/x-patch, 12 KB) 
From ea89bf6e2c27f57032bd3fdc81f35497e3c629fe Mon Sep 17 00:00:00 2001
From: Michael Matz <[email protected]>
Date: Tue, 7 Jul 2026 15:50:39 +0200
Subject: [PATCH] x86/x86-64: deal with rewriting assemblers

some assemblers rewrite even harmless looking instructions
like "xchg %rbx,%rbx" into different forms ("mov %rbx,%rbx"),
when insn rewriting is enabled.  Make valgrind robust in face
of those: use ".byte" encodings of the well-known special
instruction sequences for valgrind client requests and also
accept the alternate insn forms that already are in the wild
(the above xchg->mov rewrite).

Also give a hint in the error message that unrecognized bytes
after the preamble are not just random unrecognized insns.

Signed-off-by: Michael Matz <[email protected]>
---
 VEX/priv/guest_amd64_toIR.c | 27 ++++++++++++++++++---------
 VEX/priv/guest_x86_toIR.c   | 23 ++++++++++++++++++-----
 include/valgrind.h.in       | 33 +++++++++++++++++++++------------
 3 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/VEX/priv/guest_amd64_toIR.c b/VEX/priv/guest_amd64_toIR.c
index d407fbe07..268b9e1bd 100644
--- a/VEX/priv/guest_amd64_toIR.c
+++ b/VEX/priv/guest_amd64_toIR.c
@@ -32165,8 +32165,8 @@ DisResult disInstr_AMD64_WRK (
           code[12] == 0x48 && code[13] == 0xC1 && code[14] == 0xC7 
                                                && code[15] == 0x33) {
          /* Got a "Special" instruction preamble.  Which one is it? */
-         if (code[16] == 0x48 && code[17] == 0x87 
-                              && code[18] == 0xDB /* xchgq %rbx,%rbx */) {
+         if (code[16] == 0x48 && (code[17] == 0x87 || code[17] == 0x89)
+                              && code[18] == 0xDB /* xchgq/movq %rbx,%rbx */) {
             /* %RDX = client_request ( %RAX ) */
             DIP("%%rdx = client_request ( %%rax )\n");
             delta += 19;
@@ -32175,8 +32175,8 @@ DisResult disInstr_AMD64_WRK (
             goto decode_success;
          }
          else
-         if (code[16] == 0x48 && code[17] == 0x87 
-                              && code[18] == 0xC9 /* xchgq %rcx,%rcx */) {
+         if (code[16] == 0x48 && (code[17] == 0x87 || code[17] == 0x89)
+                              && code[18] == 0xC9 /* xchgq/movq %rcx,%rcx */) {
             /* %RAX = guest_NRADDR */
             DIP("%%rax = guest_NRADDR\n");
             delta += 19;
@@ -32184,8 +32184,8 @@ DisResult disInstr_AMD64_WRK (
             goto decode_success;
          }
          else
-         if (code[16] == 0x48 && code[17] == 0x87 
-                              && code[18] == 0xD2 /* xchgq %rdx,%rdx */) {
+         if (code[16] == 0x48 && (code[17] == 0x87 || code[17] == 0x89)
+                              && code[18] == 0xD2 /* xchgq/movq %rdx,%rdx */) {
             /* call-noredir *%RAX */
             DIP("call-noredir *%%rax\n");
             delta += 19;
@@ -32200,8 +32200,8 @@ DisResult disInstr_AMD64_WRK (
             goto decode_success;
          }
          else
-         if (code[16] == 0x48 && code[17] == 0x87
-                              && code[18] == 0xff /* xchgq %rdi,%rdi */) {
+         if (code[16] == 0x48 && (code[17] == 0x87 || code[17] == 0x89)
+                              && code[18] == 0xff /* xchgq/movq %rdi,%rdi */) {
            /* IR injection */
             DIP("IR injection\n");
             vex_inject_ir(irsb, Iend_LE);
@@ -32219,7 +32219,16 @@ DisResult disInstr_AMD64_WRK (
             dres.whatNext    = Dis_StopHere;
             dres.jk_StopHere = Ijk_InvalICache;
             goto decode_success;
-         }
+         } else {
+	     if (sigill_diag) {
+		 vex_printf("vex amd64->IR: special insn preamble followed by unrecognized bytes: "
+			    "0x%x 0x%x 0x%x 0x%x\n",
+			    getUChar(delta_start+16),
+			    getUChar(delta_start+17),
+			    getUChar(delta_start+18),
+			    getUChar(delta_start+19) );
+	     }
+	 }
          /* We don't know what it is. */
          goto decode_failure;
          /*NOTREACHED*/
diff --git a/VEX/priv/guest_x86_toIR.c b/VEX/priv/guest_x86_toIR.c
index 70dd3d70c..d61268f1e 100644
--- a/VEX/priv/guest_x86_toIR.c
+++ b/VEX/priv/guest_x86_toIR.c
@@ -8396,7 +8396,8 @@ DisResult disInstr_X86_WRK (
           code[ 6] == 0xC1 && code[ 7] == 0xC7 && code[ 8] == 0x1D &&
           code[ 9] == 0xC1 && code[10] == 0xC7 && code[11] == 0x13) {
          /* Got a "Special" instruction preamble.  Which one is it? */
-         if (code[12] == 0x87 && code[13] == 0xDB /* xchgl %ebx,%ebx */) {
+         if ((code[12] == 0x87 || code[12] == 0x89) &&
+	     code[13] == 0xDB /* xchgl/movl %ebx,%ebx */) {
             /* %EDX = client_request ( %EAX ) */
             DIP("%%edx = client_request ( %%eax )\n");
             delta += 14;
@@ -8405,7 +8406,8 @@ DisResult disInstr_X86_WRK (
             goto decode_success;
          }
          else
-         if (code[12] == 0x87 && code[13] == 0xC9 /* xchgl %ecx,%ecx */) {
+         if ((code[12] == 0x87 || code[12] == 0x89) &&
+	     code[13] == 0xC9 /* xchgl/movl %ecx,%ecx */) {
             /* %EAX = guest_NRADDR */
             DIP("%%eax = guest_NRADDR\n");
             delta += 14;
@@ -8413,7 +8415,8 @@ DisResult disInstr_X86_WRK (
             goto decode_success;
          }
          else
-         if (code[12] == 0x87 && code[13] == 0xD2 /* xchgl %edx,%edx */) {
+         if ((code[12] == 0x87 || code[12] == 0x89) &&
+	     code[13] == 0xD2 /* xchgl/movl %edx,%edx */) {
             /* call-noredir *%EAX */
             DIP("call-noredir *%%eax\n");
             delta += 14;
@@ -8428,7 +8431,8 @@ DisResult disInstr_X86_WRK (
             goto decode_success;
          }
          else
-         if (code[12] == 0x87 && code[13] == 0xFF /* xchgl %edi,%edi */) {
+         if ((code[12] == 0x87 || code[12] == 0x89) &&
+	     code[13] == 0xFF /* xchgl/movl %edi,%edi */) {
             /* IR injection */
             DIP("IR injection\n");
             vex_inject_ir(irsb, Iend_LE);
@@ -8446,7 +8450,16 @@ DisResult disInstr_X86_WRK (
             dres.whatNext    = Dis_StopHere;
             dres.jk_StopHere = Ijk_InvalICache;
             goto decode_success;
-         }
+         } else {
+	     if (sigill_diag) {
+		 vex_printf("vex x86->IR: special instruction preamble followed by unrecognized bytes: "
+			    "0x%x 0x%x 0x%x 0x%x\n",
+			    getIByte(delta_start+12),
+			    getIByte(delta_start+13),
+			    getIByte(delta_start+14),
+			    getIByte(delta_start+15));
+	     }
+	 }
          /* We don't know what it is. */
          goto decode_failure;
          /*NOTREACHED*/
diff --git a/include/valgrind.h.in b/include/valgrind.h.in
index 1bcee52a2..63c85cf63 100644
--- a/include/valgrind.h.in
+++ b/include/valgrind.h.in
@@ -278,9 +278,13 @@ typedef
    }
    OrigFn;
 
+/* The standard meaning of these bytes is:
+       roll $3,  %edi ; roll $13, %edi
+       roll $29, %edi ; roll $19, %edi  */
+
 #define __SPECIAL_INSTRUCTION_PREAMBLE                            \
-                     "roll $3,  %%edi ; roll $13, %%edi\n\t"      \
-                     "roll $29, %%edi ; roll $19, %%edi\n\t"
+       ".byte 0xc1, 0xc7, 0x03, 0xc1, 0xc7, 0x0d\n\t"             \
+       ".byte 0xc1, 0xc7, 0x1d, 0xc1, 0xc7, 0x13\n\t"
 
 #define VALGRIND_DO_CLIENT_REQUEST_EXPR(                          \
         _zzq_default, _zzq_request,                               \
@@ -296,7 +300,7 @@ typedef
     _zzq_args[5] = (unsigned int)(_zzq_arg5);                     \
     __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE               \
                      /* %EDX = client_request ( %EAX ) */         \
-                     "xchgl %%ebx,%%ebx"                          \
+                     ".byte 0x87, 0xdb"                           \
                      : "=d" (_zzq_result)                         \
                      : "a" (&_zzq_args[0]), "0" (_zzq_default)    \
                      : "cc", "memory"                             \
@@ -309,7 +313,7 @@ typedef
     volatile unsigned int __addr;                                 \
     __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE               \
                      /* %EAX = guest_NRADDR */                    \
-                     "xchgl %%ecx,%%ecx"                          \
+                     ".byte 0x87, 0xc9"                           \
                      : "=a" (__addr)                              \
                      :                                            \
                      : "cc", "memory"                             \
@@ -320,12 +324,12 @@ typedef
 #define VALGRIND_CALL_NOREDIR_EAX                                 \
                      __SPECIAL_INSTRUCTION_PREAMBLE               \
                      /* call-noredir *%EAX */                     \
-                     "xchgl %%edx,%%edx\n\t"
+                     ".byte 0x87, 0xd2\n\t"
 
 #define VALGRIND_VEX_INJECT_IR()                                 \
  do {                                                            \
     __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE              \
-                     "xchgl %%edi,%%edi\n\t"                     \
+                     ".byte 0x87, 0xff\n\t"                     \
                      : : : "cc", "memory"                        \
                     );                                           \
  } while (0)
@@ -419,9 +423,14 @@ typedef
    }
    OrigFn;
 
+/* The standard meaning of these bytes is:
+     rolq $3,  %rdi ; rolq $13, %rdi
+     rolq $61, %rdi ; rolq $51, %%rdi  */
+
 #define __SPECIAL_INSTRUCTION_PREAMBLE                            \
-                     "rolq $3,  %%rdi ; rolq $13, %%rdi\n\t"      \
-                     "rolq $61, %%rdi ; rolq $51, %%rdi\n\t"
+       ".byte 0x48, 0xc1, 0xc7, 0x03, 0x48, 0xc1, 0xc7, 0x0d\n\t" \
+       ".byte 0x48, 0xc1, 0xc7, 0x3d, 0x48, 0xc1, 0xc7, 0x33\n\t"
+
 
 #define VALGRIND_DO_CLIENT_REQUEST_EXPR(                          \
         _zzq_default, _zzq_request,                               \
@@ -437,7 +446,7 @@ typedef
     _zzq_args[5] = (unsigned long int)(_zzq_arg5);                \
     __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE               \
                      /* %RDX = client_request ( %RAX ) */         \
-                     "xchgq %%rbx,%%rbx"                          \
+                     ".byte 0x48, 0x87, 0xdb"                     \
                      : "=d" (_zzq_result)                         \
                      : "a" (&_zzq_args[0]), "0" (_zzq_default)    \
                      : "cc", "memory"                             \
@@ -450,7 +459,7 @@ typedef
     volatile unsigned long int __addr;                            \
     __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE               \
                      /* %RAX = guest_NRADDR */                    \
-                     "xchgq %%rcx,%%rcx"                          \
+                     ".byte 0x48, 0x87, 0xc9"                          \
                      : "=a" (__addr)                              \
                      :                                            \
                      : "cc", "memory"                             \
@@ -461,12 +470,12 @@ typedef
 #define VALGRIND_CALL_NOREDIR_RAX                                 \
                      __SPECIAL_INSTRUCTION_PREAMBLE               \
                      /* call-noredir *%RAX */                     \
-                     "xchgq %%rdx,%%rdx\n\t"
+                     ".byte 0x48, 0x87, 0xd2\n\t"
 
 #define VALGRIND_VEX_INJECT_IR()                                 \
  do {                                                            \
     __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE              \
-                     "xchgq %%rdi,%%rdi\n\t"                     \
+                     ".byte 0x48, 0x87, 0xff\n\t"                     \
                      : : : "cc", "memory"                        \
                     );                                           \
  } while (0)
-- 
2.48.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.