This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] gas/x86-64: properly distinguish low and high register ranges
- From: "Jan Beulich" <JBeulich at suse dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: <binutils at sourceware dot org>
- Date: Wed, 25 Jul 2012 07:56:36 +0100
- Subject: Re: [PATCH] gas/x86-64: properly distinguish low and high register ranges
- References: <500EC9B00200007800090382@nat28.tlf.novell.com> <CAMe9rOrY+ECKpfYM23heJB4Ma1BT9aFd2+-z1oVdhq3EiXwdRQ@mail.gmail.com>
>>> On 24.07.12 at 16:16, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> Can you add some testcases?
I knew you would ask this, but sorry, this makes no sense - if test
cases would are desirable here, they shouldn't be testing just the
things that this patch fixes, but also any other invalid operand
combinations. As an example - why would testing that "xlat [r11]"
isn't accepted be needed, but not e.g. "xlat [ecx]"?
Furthermore, this fixes actually broken behavior, so accepting
the change shouldn't be dependent upon test case availability.
Jan
> On Jul 24, 2012 6:14 AM, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>> There were several cases where the registers in the REX encoded range
>> got treated identically to the ones in the base range, due to not
>> paying attention to the fact that reg_entry's reg_num field doesn't
>> fully specify the register number (reg_flags also needs to be checked
>> for RegRex). This patch introduces and uses a new (inline) function to
>> obtain the full register number, and uses it to fix all those cases.
>>
>> 2012-07-24 Jan Beulich <jbeulich@suse.com>
>>
>> * config/tc-i386.c (register_number): New function.
>> (build_vex_prefix, process_immext, process_operands,
>> build_modrm_byte, i386_index_check): Use it.
>>
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -1758,6 +1758,17 @@ operand_type_register_match (i386_operan
>> }
>>
>> static INLINE unsigned int
>> +register_number (const reg_entry *r)
>> +{
>> + unsigned int nr = r->reg_num;
>> +
>> + if (r->reg_flags & RegRex)
>> + nr += 8;
>> +
>> + return nr;
>> +}
>> +
>> +static INLINE unsigned int
>> mode_from_disp_size (i386_operand_type t)
>> {
>> if (t.bitfield.disp8)
>> @@ -2830,12 +2841,7 @@ build_vex_prefix (const insn_template *t
>>
>> /* Check register specifier. */
>> if (i.vex.register_specifier)
>> - {
>> - register_specifier = i.vex.register_specifier->reg_num;
>> - if ((i.vex.register_specifier->reg_flags & RegRex))
>> - register_specifier += 8;
>> - register_specifier = ~register_specifier & 0xf;
>> - }
>> + register_specifier = ~register_number (i.vex.register_specifier) &
>> 0xf;
>> else
>> register_specifier = 0xf;
>>
>> @@ -2976,14 +2982,13 @@ process_immext (void)
>>
>> if (i.tm.cpu_flags.bitfield.cpusse3 && i.operands > 0)
>> {
>> - /* SSE3 Instructions have the fixed operands with an opcode
>> - suffix which is coded in the same place as an 8-bit immediate
>> - field would be. Here we check those operands and remove them
>> - afterwards. */
>> + /* MONITOR/MWAIT have fixed operands with an opcode suffix which
>> + is coded in the same place as an 8-bit immediate field would be.
>> + Here we check those operands and remove them afterwards. */
>> unsigned int x;
>>
>> for (x = 0; x < i.operands; x++)
>> - if (i.op[x].regs->reg_num != x)
>> + if (register_number (i.op[x].regs) != x)
>> as_bad (_("can't use register '%s%s' as operand %d in '%s'."),
>> register_prefix, i.op[x].regs->reg_name, x + 1,
>> i.tm.name);
>> @@ -5075,7 +5080,7 @@ process_operands (void)
>> {
>> /* The first operand is implicit and must be xmm0. */
>> gas_assert (operand_type_equal (&i.types[0], ®xmm));
>> - if (i.op[0].regs->reg_num != 0)
>> + if (register_number (i.op[0].regs) != 0)
>> return bad_implicit_operand (1);
>>
>> if (i.tm.opcode_modifier.vexsources == VEX3SOURCES)
>> @@ -5149,7 +5154,7 @@ duplicate:
>> gas_assert (i.reg_operands
>> && (operand_type_equal (&i.types[0], ®xmm)
>> || operand_type_equal (&i.types[0], ®ymm)));
>> - if (i.op[0].regs->reg_num != 0)
>> + if (register_number (i.op[0].regs) != 0)
>> return bad_implicit_operand (i.types[0].bitfield.regxmm);
>>
>> for (j = 1; j < i.operands; j++)
>> @@ -5352,10 +5357,7 @@ build_modrm_byte (void)
>> || operand_type_equal
>> (&i.tm.operand_types[reg_slot],
>> ®ymm));
>> exp->X_op = O_constant;
>> - exp->X_add_number
>> - = ((i.op[reg_slot].regs->reg_num
>> - + ((i.op[reg_slot].regs->reg_flags & RegRex) ? 8 : 0))
>> - << 4);
>> + exp->X_add_number = register_number (i.op[reg_slot].regs) << 4;
>> }
>> else
>> {
>> @@ -5399,9 +5401,7 @@ build_modrm_byte (void)
>> || operand_type_equal (&i.tm.operand_types[reg_slot],
>> ®ymm));
>> i.op[imm_slot].imms->X_add_number
>> - |= ((i.op[reg_slot].regs->reg_num
>> - + ((i.op[reg_slot].regs->reg_flags & RegRex) ? 8 : 0))
>> - << 4);
>> + |= register_number (i.op[reg_slot].regs) << 4;
>> }
>>
>> gas_assert (operand_type_equal (&i.tm.operand_types[nds], ®xmm)
>> @@ -7314,7 +7314,7 @@ i386_index_check (const char *operand_st
>> ? i.base_reg->reg_type.bitfield.reg32
>> : i.base_reg->reg_type.bitfield.reg16))
>> ok = 0;
>> - else if (i.base_reg->reg_num != expected)
>> + else if (register_number (i.base_reg) != expected)
>> ok = -1;
>>
>> if (ok < 0)
>> @@ -7329,7 +7329,7 @@ i386_index_check (const char *operand_st
>> : (flag_code == CODE_16BIT) ^ !i.prefix[ADDR_PREFIX]
>> ? i386_regtab[j].reg_type.bitfield.reg32
>> : i386_regtab[j].reg_type.bitfield.reg16)
>> - && i386_regtab[j].reg_num == expected)
>> + && register_number(i386_regtab + j) == expected)
>> break;
>> gas_assert (j < i386_regtab_size);
>> as_warn (_("`%s' is not valid here (expected `%c%s%s%c')"),
>>
>>
>>