Bug 478624 - Valgrind incompatibility with binutils-2.42 on x86 with new nop patterns...

It was a bit of a struggle to get the testcase to build
with both clang and gcc (oddly enough gcc was more difficult) so
I just resorted to using .byte arrays.

diff --git a/.gitignore b/.gitignore
index f250b39c95fa82a26bf0c66ff26024b80cc6b8a0..88a6522605dbaf92bc19cb82fac03f0802b8d451 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -2216,6 +2216,7 @@
 /none/tests/x86/fpu_lazy_eflags
 /none/tests/x86/fxtract
 /none/tests/x86/getseg
+/none/tests/x86/gnu_binutils_nop
 /none/tests/x86/incdec_alt
 /none/tests/x86/insn_basic
 /none/tests/x86/insn_basic.c

diff --git a/NEWS b/NEWS
index 2c50a2639cc0a105fa7c046223c9c41797b2637f..839407479f54473c79dfcab164f8d9e3d8ab8bc4 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -47,6 +47,8 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 477630  Include ucontext.h rather than sys/ucontext.h in Solaris sources
 477719  vgdb incorrectly replies to qRcmd packet
 478211  Redundant code for vgdb.c and Valgrind core tools
+478624  Valgrind incompatibility with binutils-2.42 on x86 with new nop patterns
+        (unhandled instruction bytes: 0x2E 0x8D 0xB4 0x26
 n-i-bz  Add redirect for memccpy
 
 To see details of a given bug, visit

diff --git a/VEX/priv/guest_x86_toIR.c b/VEX/priv/guest_x86_toIR.c
index 5d6e6dc64fddbe379c3322b9d099be259522bb5e..3b6efb38730deade43755dccc4d6a8ad3f6e2f51 100644 (file)
--- a/VEX/priv/guest_x86_toIR.c
+++ b/VEX/priv/guest_x86_toIR.c
@@ -8198,7 +8198,7 @@ DisResult disInstr_X86_WRK (
          delta += 5;
          goto decode_success;
       }
-      /* Don't barf on recent binutils padding,
+      /* Don't barf on recent (2010) binutils padding,
          all variants of which are: nopw %cs:0x0(%eax,%eax,1)
          66 2e 0f 1f 84 00 00 00 00 00
          66 66 2e 0f 1f 84 00 00 00 00 00
@@ -8223,6 +8223,26 @@ DisResult disInstr_X86_WRK (
          }
       }
 
+      /* bug478624 GNU binutils uses a leal of esi into itself with
+         a zero offset and CS prefix as an 8 byte no-op (Dec 2023).
+         Since the CS prefix is hardly ever used we don't do much
+         to decode it, just a few cases for conditional branches.
+         So add handling here with other pseudo-no-ops.
+       */
+      if (code[0] == 0x2E && code[1] == 0x8D) {
+         if (code[2] == 0x74 && code[3] == 0x26 && code[4] == 0x00) {
+            DIP("leal %%cs:0(%%esi,%%eiz,1),%%esi\n");
+            delta += 5;
+            goto decode_success;
+         }
+         if (code[2] == 0xB4 && code[3] == 0x26 && code[4] == 0x00
+             && code[5] == 0x00 && code[6] == 0x00 && code[7] == 0x00) {
+            DIP("leal %%cs:0(%%esi,%%eiz,1),%%esi\n");
+            delta += 8;
+            goto decode_success;
+         }
+      }
+
       // Intel CET requires the following opcodes to be treated as NOPs
       // with any prefix and ModRM, SIB and disp combination:
       // "0F 19", "0F 1C", "0F 1D", "0F 1E", "0F 1F"

diff --git a/none/tests/x86/Makefile.am b/none/tests/x86/Makefile.am
index 3ecd1ad3c253fa74546b66e878fb4503a366471f..dbae86571298656312f51940884ed14f8a60aba1 100644 (file)
--- a/none/tests/x86/Makefile.am
+++ b/none/tests/x86/Makefile.am
@@ -52,6 +52,7 @@ EXTRA_DIST = \
        fxtract.stdout.exp fxtract.stderr.exp fxtract.vgtest \
        fxtract.stdout.exp-older-glibc \
        getseg.stdout.exp getseg.stderr.exp getseg.vgtest \
+       gnu_binutils_nop.stderr.exp gnu_binutils_nop.vgtest \
        incdec_alt.stdout.exp incdec_alt.stderr.exp incdec_alt.vgtest \
        int.stderr.exp int.stdout.exp int.disabled \
        $(addsuffix .stderr.exp,$(INSN_TESTS)) \
@@ -100,6 +101,7 @@ check_PROGRAMS = \
        fpu_lazy_eflags \
        fxtract \
        getseg \
+       gnu_binutils_nop \
        incdec_alt \
        $(INSN_TESTS) \
        int \

diff --git a/none/tests/x86/gnu_binutils_nop.c b/none/tests/x86/gnu_binutils_nop.c
new file mode 100644 (file)
index 0000000..412a4c2
--- /dev/null
+++ b/none/tests/x86/gnu_binutils_nop.c
@@ -0,0 +1,34 @@
+int main(void)
+{
+    // GNU binutils uses various opcodes as alternatives for nop
+    // the idea is that it is faster to execute one large opcode
+    // with no side-effects than multiple repetitions of the
+    // single byte 'nop'. This gives more choice when code
+    // needs to be padded.
+   
+   // the following is based on
+   // https://sourceware.org/cgit/binutils-gdb/tree/gas/config/tc-i386.c#n1256
+
+    // one byte
+    __asm__ __volatile__("nop");
+    // two bytes
+    __asm__ __volatile__("xchg %ax,%ax");
+    // three bytes
+    //__asm__ __volatile__("leal 0(%esi),%esi");
+    __asm__ __volatile__(".byte 0x8d,0x76,0x00");
+    // four bytes
+    //__asm__ __volatile__("leal 0(%esi,%eiz),%esi");
+    __asm__ __volatile__(".byte 0x8d,0x74,0x26,0x00");
+    // five bytes
+    //__asm__ __volatile__("leal %cs:0(%esi,%eiz),%esi");
+    __asm__ __volatile__(".byte 0x2e,0x8d,0x74,0x26,0x00");
+    // six bytes
+    //__asm__ __volatile__("leal 0L(%esi),%esi");
+    __asm__ __volatile__(".byte 0x8d,0xb6,0x00,0x00,0x00,0x00");
+    // seven bytes
+    //__asm__ __volatile__("leal 0L(%esi,%eiz),%esi");
+    __asm__ __volatile__(".byte 0x8d,0xb4,0x26,0x00,0x00,0x00,0x00");
+    // eight bytes
+    //__asm__ __volatile__("leal %cs:0L(%esi,%eiz),%esi");
+    __asm__ __volatile__(".byte 0x2e,0x8d,0xb4,0x26,0x00,0x00,0x00,0x00");
+}

diff --git a/none/tests/x86/gnu_binutils_nop.stderr.exp b/none/tests/x86/gnu_binutils_nop.stderr.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/none/tests/x86/gnu_binutils_nop.vgtest b/none/tests/x86/gnu_binutils_nop.vgtest
new file mode 100644 (file)
index 0000000..7f378dd
--- /dev/null
+++ b/none/tests/x86/gnu_binutils_nop.vgtest
@@ -0,0 +1,2 @@
+prog: gnu_binutils_nop
+vgopts: -q