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

[Sam James <sam@gentoo.org>]

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.

sam