From ea89bf6e2c27f57032bd3fdc81f35497e3c629fe Mon Sep 17 00:00:00 2001 From: Michael Matz 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 --- 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