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

Peter Zijlstra
2025-09-25T14:11:52Z

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) {