Inconsistent usage on onebyte_modrm and twobyte_modrm table in x86 disassembler and gdb?

[Schimpe, Christina]

Hi all, 

Thanks a lot for working on this. I have followed this thread and also discussed this with Klaus a bit.
I still wonder, would it make sense to align the logic in x86 disassembler and gdb ?

Kind Regards,
Christina

> -----Original Message-----
> From: Tom de Vries <tdevries@suse.de>
> Sent: Tuesday, August 26, 2025 11:32 AM
> To: Gerlicher, Klaus <klaus.gerlicher@intel.com>; Sam James <sam@gentoo.org>;
> Beulich, Jan <JBeulich@suse.com>
> Cc: Jiang, Haochen <haochen.jiang@intel.com>; Binutils
> <binutils@sourceware.org>; gdb-patches@sourceware.org; H.J.Lu
> <hjl.tools@gmail.com>; Alexander Monakov <amonakov@ispras.ru>
> Subject: Re: Inconsistent usage on onebyte_modrm and twobyte_modrm table
> in x86 disassembler and gdb?
> 
> On 8/26/25 10:19, Gerlicher, Klaus wrote:
> > FWIW I'll attach my fix for the (V)PBLENDW. It has a testcase and a unittest
> case.
> 
> Hi Klaus,
> 
> thanks for working on this.
> 
> The patch LGTM, other than this:
> ...
> $ git show --pretty=%s --check
> Fix vpblendw
> 
> gdb/amd64-tdep.c:1409: space before tab in indent.
> +      	  need_modrm = 1;
> ...
> 
> I've tested both the unit test, and the updated test-case.
> 
> I've also tested the unit test I wrote, which uses vex3 instead of evex:
> ...
> +
> +  /* INSN: vpblendw $0x7,%xmm4,%xmm6,%xmm2, vex3 prefix.  */  insn = {
> + 0xc4, 0xe3, 0x49, 0x0e, 0xd4, 0x07 };  amd64_get_insn_details
> + (insn.data (), &details);  SELF_CHECK (details.opcode_len == 3);
> + SELF_CHECK (details.enc_prefix_offset == 0);  SELF_CHECK
> + (details.opcode_offset == 3);  SELF_CHECK (details.modrm_offset == 4);
> +
> +  /* INSN: vpblendw $0x7,0xff(%rip),%ymm6,%ymm2, vex3 prefix.  */  insn
> + = { 0xc4, 0xe3, 0x4d, 0x0e, 0x15, 0xff, 0x00, 0x00, 0x00, 0x07 };
> + amd64_get_insn_details (insn.data (), &details);  SELF_CHECK
> + (details.opcode_len == 3);  SELF_CHECK (details.enc_prefix_offset ==
> + 0);  SELF_CHECK (details.opcode_offset == 3);  SELF_CHECK
> + (details.modrm_offset == 4);
> +
> +  /* INSN: vpblendw $0x7,0xff(%ecx),%ymm6,%ymm2, vex3 prefix.  */
> + fixup_riprel (details, insn.data (), ECX_REG_NUM);  updated_insn
> +    = { 0xc4, 0xe3, 0x4d, 0x0e, 0x91, 0xff, 0x00, 0x00, 0x00, 0x07 };
> + SELF_CHECK (insn == updated_insn);
> ...
> 
> You could add this as well, but it's not required.
> 
> [ Note that you could drop the test-case update and use fixup_riprel on the unit
> test you added instead, like I did here and is done elsewhere in the unit test. ]
> 
> Please commit this with some appropriate $subject.
> 
> Approved-By: Tom de Vries <tdevries@suse.de>
> 
> Thanks,
> - Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928