Re: [PATCH 3/3] objtool/x86: Fix NOP decode

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Sep 25, 2025 at 02:43:32PM +0200, Peter Zijlstra wrote:

> But yes.. I happen to have an insn_is_nop() function that can be used on
> userspace, and that certainly wants to be taught about these... x86 is
> such a pain :/

This is the delta I ended up with for my insn_is_nop() function to
support *VEX*/REX2.

I'll post the whole thing later.

--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -138,6 +138,10 @@ struct insn {
 #define X86_VEX_V(vex)	(((vex) & 0x78) >> 3)	/* VEX3 Byte2, VEX2 Byte1 */
 #define X86_VEX_P(vex)	((vex) & 0x03)		/* VEX3 Byte2, VEX2 Byte1 */
 #define X86_VEX_M_MAX	0x1f			/* VEX3.M Maximum value */
+/* EVEX bit fields */
+#define X86_EVEX_R4(vex) ((vex) & 0x10)		/* EVEX Byte1 */
+#define X86_EVEX_B4(vex) ((vex) & 0x08)		/* EVEX Byte1 */
+#define X86_EVEX_X4(vex) ((vex) & 0x04)		/* EVEX Byte2 */
 /* XOP bit fields */
 #define X86_XOP_R(xop)	((xop) & 0x80)	/* XOP Byte2 */
 #define X86_XOP_X(xop)	((xop) & 0x40)	/* XOP Byte2 */
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -1689,33 +1689,62 @@ enum insn_mmio_type insn_decode_mmio(str
  */
 bool insn_is_nop(struct insn *insn)
 {
-	u8 rex, rex_b = 0, rex_x = 0, rex_r = 0, rex_w = 0;
+	u8 b3 = 0, x3 = 0, r3 = 0, w = 0;
+	u8 b4 = 0, x4 = 0, r4 = 0;
 	u8 modrm, modrm_mod, modrm_reg, modrm_rm;
 	u8 sib = 0, sib_scale, sib_index, sib_base;
 	insn_byte_t p;
+	u8 nrex, rex;
 	int i;
 
-	if (insn->rex_prefix.nbytes) {
-		rex = insn->rex_prefix.bytes[0];
-		rex_w = !!X86_REX_W(rex);
-		rex_r = !!X86_REX_R(rex);
-		rex_x = !!X86_REX_X(rex);
-		rex_b = !!X86_REX_B(rex);
+	if ((nrex = insn->rex_prefix.nbytes)) {
+		rex = insn->rex_prefix.bytes[nrex-1];
+
+		w  = !!X86_REX_W(rex);
+		r3 = !!X86_REX_R(rex);
+		x3 = !!X86_REX_X(rex);
+		b3 = !!X86_REX_B(rex);
+		if (nrex > 1) {
+			r4 = !!X86_REX2_R(rex);
+			x4 = !!X86_REX2_X(rex);
+			b4 = !!X86_REX2_B(rex);
+		}
+
+	} else switch (insn->vex_prefix.nbytes) {
+	case 2: /* VEX2 */
+		r3 =  !X86_VEX_R(insn->vex_prefix.bytes[1]);
+		break;
+	case 3: /* VEX3 */
+		r3 =  !X86_VEX_R(insn->vex_prefix.bytes[1]);
+		x3 =  !X86_VEX_X(insn->vex_prefix.bytes[1]);
+		b3 =  !X86_VEX_B(insn->vex_prefix.bytes[1]);
+		w  = !!X86_VEX_W(insn->vex_prefix.bytes[2]);
+		break;
+	case 4: /* EVEX */
+		r3 =  !X86_VEX_R(insn->vex_prefix.bytes[1]);
+		x3 =  !X86_VEX_X(insn->vex_prefix.bytes[1]);
+		b3 =  !X86_VEX_B(insn->vex_prefix.bytes[1]);
+		w  = !!X86_VEX_W(insn->vex_prefix.bytes[2]);
+		r4 =  !X86_EVEX_R4(insn->vex_prefix.bytes[1]);
+		b4 = !!X86_EVEX_B4(insn->vex_prefix.bytes[1]);
+		x4 =  !X86_EVEX_X4(insn->vex_prefix.bytes[2]);
+		break;
+	default: break;
 	}
 
 	if (insn->modrm.nbytes) {
 		modrm = insn->modrm.bytes[0];
 		modrm_mod = X86_MODRM_MOD(modrm);
-		modrm_reg = X86_MODRM_REG(modrm) + 8*rex_r;
-		modrm_rm  = X86_MODRM_RM(modrm)  + 8*rex_b;
+		modrm_reg = X86_MODRM_REG(modrm) + 8*r3 + 16*r4;
+		modrm_rm  = X86_MODRM_RM(modrm)  + 8*b3 + 16*b4;
 		modrm = 1;
 	}
 
 	if (insn->sib.nbytes) {
 		sib = insn->sib.bytes[0];
 		sib_scale = X86_SIB_SCALE(sib);
-		sib_index = X86_SIB_INDEX(sib) + 8*rex_x;
-		sib_base  = X86_SIB_BASE(sib)  + 8*rex_b;
+		sib_index = X86_SIB_INDEX(sib) + 8*x3 + 16*x4;
+		sib_base  = X86_SIB_BASE(sib)  + 8*b3 + 16*b4;
 		sib = 1;
 
 		modrm_rm = sib_base;
@@ -1729,7 +1758,7 @@ bool insn_is_nop(struct insn *insn)
 		if (modrm_mod != 3) /* register-direct */
 			return false;
 
-		if (insn->x86_64 && !rex_w) /* native size */
+		if (insn->x86_64 && !w) /* native size */
 			return false;
 
 		for_each_insn_prefix(insn, i, p) {
@@ -1743,7 +1772,7 @@ bool insn_is_nop(struct insn *insn)
 		if (modrm_mod == 0 || modrm_mod == 3) /* register-indirect with disp */
 			return false;
 
-		if (insn->x86_64 && !rex_w) /* native size */
+		if (insn->x86_64 && !w) /* native size */
 			return false;
 
 		if (insn->displacement.value != 0)
@@ -1760,7 +1789,7 @@ bool insn_is_nop(struct insn *insn)
 		return modrm_reg == modrm_rm; /* LEA 0(%reg), %reg */
 
 	case 0x90: /* NOP */
-		if (rex_b) /* XCHG %r8,%rax */
+		if (b3 || b4) /* XCHG %r8,%rax */
 			return false;
 
 		for_each_insn_prefix(insn, i, p) {