Re: [PATCH] i386: Check invalid (%dx) usage

[Jan Beulich <jbeulich@suse.com>]

On 07.11.2022 10:55, Jan Beulich via Binutils wrote:
> On 04.11.2022 21:55, H.J. Lu via Binutils wrote:
>> (%dx) isn't a valid memory address in any modes.  It is used as a special
>> memory operand for input/output port address in AT&T syntax and should
>> only be used with input/output instructions.  Update i386_att_operand to
>> set i.input_output_operand to true for (%dx) and issue an error if (%dx)
>> is used with non-input/output instructions.
> 
> Hmm, this shouldn't require a new flag I would hope. We did properly reject
> bad uses up to 2.31 ("operand type mismatch"). Whatever was broken there
> would need correcting instead, imo. A possible candidate looks to be
> 2fb5be8dac9d ("x86: drop {,reg16_}inoutportreg variables"), albeit perhaps
> combined with later changes - in 2.33 behavior changed again.

What about the change below, perhaps combined with your testsuite adjustments
(albeit I'd like to point out that "incl" isn't the best choice, as %dx is
invalid with that anyway; "incw" would be better)? That way we'll uniformly
get "`(%dx)' is not a valid base/index expression" for bad uses of (%dx),
matching any other uses of wrong addressing forms.

Jan

x86: restrict use of (%dx)

PR gas/29751
The AT&T mode special case operand (%dx) is valid to use only with
instructions nominally expecting %dx to specify an I/O port address.
Prefix the respective checking with an opcode check. Keep that as
simple as possible by recognizing that opcodes 0x64 and 0x66 (which
wrongly also match the check) encode prefixes, which hence - even if
used standalone - don't take any operands, so match_template() will
fail there for other reasons.

While there also complete the transformation from memory to register
operand: The lack thereof was responsible for SEGV when (%dx) was
(wrongly) used with certain insns.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -11884,7 +11884,9 @@ i386_att_operand (char *operand_string)
 	}
 
       /* Special case for (%dx) while doing input/output op.  */
-      if (i.base_reg
+      if ((current_templates->start->base_opcode | 0x8a) == 0xee
+	  && current_templates->start->opcode_modifier.opcodespace == SPACE_BASE
+	  && i.base_reg
 	  && i.base_reg->reg_type.bitfield.instance == RegD
 	  && i.base_reg->reg_type.bitfield.word
 	  && i.index_reg == 0
@@ -11893,6 +11895,8 @@ i386_att_operand (char *operand_string)
 	  && !operand_type_check (i.types[this_operand], disp))
 	{
 	  i.types[this_operand] = i.base_reg->reg_type;
+ 	  i.op[this_operand].regs = i.base_reg;
+	  i.reg_operands++;
 	  return 1;
 	}