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

[Alexander Monakov <amonakov@ispras.ru>]

Hi,

On Mon, 25 Aug 2025, Sam James wrote:

> Jan Beulich <jbeulich@suse.com> writes:
> 
> > On 25.08.2025 04:42, Jiang, Haochen wrote:
> >> Hi all,
> >> 
> >> Recently I happened to have a look at the gdb code. At gdb/amd64-tdep.c
> >> L1102 comment, it mentioned that:
> >> 
> >> /* WARNING: Keep onebyte_has_modrm, twobyte_has_modrm in sync with
> >>    ../opcodes/i386-dis.c (until libopcodes exports them, or an alternative,
> >>    at which point delete these in favor of libopcodes' versions).  */
> >> 
> >> This means the table content and usage should be the same as gas.
> >> 
> >> However, when we are using the table in disassembler at opcode/i386-dis.c
> >> L9877, it is:
> >> 
> >>   /* REX2.M in rex2 prefix represents map0 or map1.  */
> >>   if (ins.last_rex2_prefix < 0 ? *ins.codep == 0x0f : (ins.rex2 & REX2_M))
> >>     {
> >>       if (!ins.rex2)
> >>         {
> >>           ins.codep++;
> >>           if (!fetch_code (info, ins.codep + 1))
> >>             goto fetch_error_out;
> >>         }
> >> 
> >>       dp = &dis386_twobyte[*ins.codep];
> >>       ins.need_modrm = twobyte_has_modrm[*ins.codep];
> >>     }
> >>   else
> >>     {
> >>       dp = &dis386[*ins.codep];
> >>       ins.need_modrm = onebyte_has_modrm[*ins.codep];
> >>     }
> >> 
> >> It will use the very first byte of the bytecode.
> >> 
> >> On the other hand, in gdb, let's take VEX prefix as example at
> >> gdb/amd64-tdep.c L1349, the logic is:
> >> 
> >>   /* Skip REX/VEX instruction encoding prefixes.  */
> >>   ...
> >>   else if (vex2_prefix_p (*insn))
> >>     {
> >>       details->enc_prefix_offset = insn - start;
> >>       insn += 2;
> >>     }
> >>   else if (vex3_prefix_p (*insn))
> >>     {
> >>       details->enc_prefix_offset = insn - start;
> >>       insn += 3;
> >>     }
> >>   ...
> >>   if (prefix != nullptr && rex2_prefix_p (*prefix))
> >>     {
> >>       ...
> >>     }
> >>   else if (prefix != nullptr && vex2_prefix_p (*prefix))
> >>     {
> >>       need_modrm = twobyte_has_modrm[*insn];
> >>       details->opcode_len = 2;
> >>     }
> >>   else if (prefix != nullptr && vex3_prefix_p (*prefix))
> >>     {
> >>       need_modrm = twobyte_has_modrm[*insn];
> >>       ...
> >> }
> >> ...
> >> 
> >> It will skip the VEX prefix and use twobyte_has_modrm table instead of
> >> onebyte_has_modrm[0xc4/c5] in disassembler. The table usage are totally
> >> different although the table itself is the same. It will cause the need_modrm
> >> value different eventually. For example, opcode for VPBLENDW under 128 bit
> >> is "VEX.128.66.0F3A.WIG 0E /r ib". The need_modrm would be false in gdb
> >> since twobyte_has_modrm[0x0e] is false.
> >> 
> >> Does anyone know the reason on that? It is weird to me.
> >
> > Same here; see https://sourceware.org/pipermail/gdb-patches/2019-February/155347.html.
> > That patch might require re-basing and some work to be up-to-date again, but
> > fundamentally it still looks applicable. I don't really understand why stuff
> > like this isn't allowed in. Pedro's desire for a testcase is understandable,
> > but shouldn't block such a patch (there was a 2nd one s well) for this many
> > years.
> 
> I didn't realise a patch was rotting for this. There's Alexander's
> PR28999 (and a few other either dupes or very-related bugs) too.
> 
> While I can understand wanting a testcase, tdep is really in a sorry
> state for x86 anyway, and this clearly makes it better. Perhaps with
> Haochen's interest, we can finally get it in. But I don't see any
> specific x86 maintainers for gdb.

I hope the bug is fixed by a more comprehensive patchset from Tom, which has
already landed:
https://inbox.sourceware.org/gdb-patches/e5282a4b-5d9f-4891-b9b8-45ded54ec6ee@suse.de/

Haochen's question still stands, I guess.

Alexander