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

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

[H.J. Lu]

(%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.

	PR gas/29751
	* config/tc-i386.c (_i386_insn): Add input_output_operand.
	(md_assemble): Issue an error if input/output memory operand is
	used with non-input/output instructions.
	(i386_att_operand): Set i.input_output_operand to true for
	(%dx).
	* testsuite/gas/i386/inval.l: Updated.
	* testsuite/gas/i386/x86-64-inval.l: Likewise.
	* testsuite/gas/i386/inval.s: Add tests for invalid (%dx) usage.
	* testsuite/gas/i386/x86-64-inval.s: Likewise.
---
 gas/config/tc-i386.c                  | 14 ++++++++++++++
 gas/testsuite/gas/i386/inval.l        |  7 +++++++
 gas/testsuite/gas/i386/inval.s        |  4 ++++
 gas/testsuite/gas/i386/x86-64-inval.l |  7 +++++++
 gas/testsuite/gas/i386/x86-64-inval.s |  4 ++++
 5 files changed, 36 insertions(+)

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index a846b9e8653..42c12bb3d55 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -311,6 +311,10 @@ struct _i386_insn
     /* The operand to a branch insn indicates an absolute branch.  */
     bool jumpabsolute;
 
+    /* There is a memory operand of (%dx) which should be only used
+       with input/output instructions.  */
+    bool input_output_operand;
+
     /* Extended states.  */
     enum
       {
@@ -5043,6 +5047,15 @@ md_assemble (char *line)
       i.disp_operands = 0;
     }
 
+  /* The memory operand of (%dx) should be only used with input/output
+     instructions (base opcodes: 0x6c, 0x6e, 0xec, 0xee).  */
+  if (i.input_output_operand && (i.tm.base_opcode | 0x82) != 0xee)
+    {
+      as_bad (_("input/output port address isn't allowed with `%s'"),
+	      i.tm.name);
+      return;
+    }
+
   if (optimize && !i.no_optimize && i.tm.opcode_modifier.optimize)
     optimize_encoding ();
 
@@ -11750,6 +11763,7 @@ i386_att_operand (char *operand_string)
 	  && !operand_type_check (i.types[this_operand], disp))
 	{
 	  i.types[this_operand] = i.base_reg->reg_type;
+	  i.input_output_operand = true;
 	  return 1;
 	}
 
diff --git a/gas/testsuite/gas/i386/inval.l b/gas/testsuite/gas/i386/inval.l
index abe220620b7..bd1017c7bee 100644
--- a/gas/testsuite/gas/i386/inval.l
+++ b/gas/testsuite/gas/i386/inval.l
@@ -97,6 +97,9 @@
 .*:112: Error: .*
 .*:113: Error: .*
 .*:114: Error: .*
+.*:116: Error: .*
+.*:117: Error: .*
+.*:118: Error: .*
 GAS LISTING .*
 
 
@@ -220,3 +223,7 @@ GAS LISTING .*
 \fGAS LISTING .*
 
 
+[ 	]*[1-9][0-9]*[ 	]+
+[ 	]*[1-9][0-9]*[ 	]+incl	\(%dx\)
+[ 	]*[1-9][0-9]*[ 	]+mov	\(%dx\), %ax
+[ 	]*[1-9][0-9]*[ 	]+mov	%ax, \(%dx\)
diff --git a/gas/testsuite/gas/i386/inval.s b/gas/testsuite/gas/i386/inval.s
index 7adfec64600..7e9a778f74f 100644
--- a/gas/testsuite/gas/i386/inval.s
+++ b/gas/testsuite/gas/i386/inval.s
@@ -112,3 +112,7 @@ movnti word ptr [eax], ax
 	inb	%dx, %ax
 	outb	%ax, %dx
 	movb	%ax, %bx
+
+	incl	(%dx)
+	mov	(%dx), %ax
+	mov	%ax, (%dx)
diff --git a/gas/testsuite/gas/i386/x86-64-inval.l b/gas/testsuite/gas/i386/x86-64-inval.l
index bbb8ba295cb..688bcd90894 100644
--- a/gas/testsuite/gas/i386/x86-64-inval.l
+++ b/gas/testsuite/gas/i386/x86-64-inval.l
@@ -111,6 +111,9 @@
 .*:117: Error: .*
 .*:118: Error: .*
 .*:121: Error: .*
+.*:123: Error: .*
+.*:124: Error: .*
+.*:125: Error: .*
 GAS LISTING .*
 
 
@@ -241,3 +244,7 @@ GAS LISTING .*
 [ 	]*[1-9][0-9]*[ 	]+
 [ 	]*[1-9][0-9]*[ 	]+\.att_syntax prefix
 [ 	]*[1-9][0-9]*[ 	]+movsd \(%rsi\), %ss:\(%rdi\), %ss:\(%rax\)
+[ 	]*[1-9][0-9]*[ 	]+
+[ 	]*[1-9][0-9]*[ 	]+incl	\(%dx\)
+[ 	]*[1-9][0-9]*[ 	]+mov	\(%dx\), %ax
+[ 	]*[1-9][0-9]*[ 	]+mov	%ax, \(%dx\)
diff --git a/gas/testsuite/gas/i386/x86-64-inval.s b/gas/testsuite/gas/i386/x86-64-inval.s
index 85c3582d4b2..16622a41c95 100644
--- a/gas/testsuite/gas/i386/x86-64-inval.s
+++ b/gas/testsuite/gas/i386/x86-64-inval.s
@@ -119,3 +119,7 @@ movnti word ptr [rax], ax
 
 	.att_syntax prefix
 	movsd (%rsi), %ss:(%rdi), %ss:(%rax)
+
+	incl	(%dx)
+	mov	(%dx), %ax
+	mov	%ax, (%dx)
-- 
2.37.3

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

[Jan Beulich]

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.

Jan

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

[Jan Beulich]

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;
 	}
 

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

[H.J. Lu]

On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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

Since incl is misassembled, it is a good test.

> 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

Since current_templates doesn't point to the matched instruction,
checking current_templates looks like abuse.  I don't think error
messages should be a concern here.

> 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

I prefer to keep it ASIS since the lack of the transformation helped
catch this error.

> 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;
>         }
>
>
>


-- 
H.J.

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

[Jan Beulich]

On 07.11.2022 20:58, H.J. Lu wrote:
> On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> 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
> 
> Since incl is misassembled, it is a good test.

Well, perhaps both incl and incw then?

>> 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
> 
> Since current_templates doesn't point to the matched instruction,
> checking current_templates looks like abuse.  I don't think error
> messages should be a concern here.

We use current_templates in similar ways in quite a number of places,
when match_templates() hasn't run yet.

>> 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
> 
> I prefer to keep it ASIS since the lack of the transformation helped
> catch this error.

Interesting view. I think with just this code added you'd still see
mis-assembly; you merely wouldn't see ...

>> operand: The lack thereof was responsible for SEGV when (%dx) was
>> (wrongly) used with certain insns.

... SEGV anymore.

Jan

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

[H.J. Lu]

On Mon, Nov 7, 2022 at 11:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 07.11.2022 20:58, H.J. Lu wrote:
> > On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> 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
> >
> > Since incl is misassembled, it is a good test.
>
> Well, perhaps both incl and incw then?

Sure.

> >> 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
> >
> > Since current_templates doesn't point to the matched instruction,
> > checking current_templates looks like abuse.  I don't think error
> > messages should be a concern here.
>
> We use current_templates in similar ways in quite a number of places,
> when match_templates() hasn't run yet.

Since the first template isn't the selected one, your check allows
the invalid opcodes.

> >> 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
> >
> > I prefer to keep it ASIS since the lack of the transformation helped
> > catch this error.
>
> Interesting view. I think with just this code added you'd still see
> mis-assembly; you merely wouldn't see ...

True since it uses the invalid template.

> >> operand: The lack thereof was responsible for SEGV when (%dx) was
> >> (wrongly) used with certain insns.
>
> ... SEGV anymore.
>
> Jan



-- 
H.J.

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

[Jan Beulich]

On 08.11.2022 22:06, H.J. Lu wrote:
> On Mon, Nov 7, 2022 at 11:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 07.11.2022 20:58, H.J. Lu wrote:
>>> On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> 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
>>>
>>> Since current_templates doesn't point to the matched instruction,
>>> checking current_templates looks like abuse.  I don't think error
>>> messages should be a concern here.
>>
>> We use current_templates in similar ways in quite a number of places,
>> when match_templates() hasn't run yet.
> 
> Since the first template isn't the selected one, your check allows
> the invalid opcodes.

I guess I don't understand, but I guess I'll also give up. Which
template the check is done against doesn't really matter here, as
long as it's one with the correct mnemonic. We could of course
also re-order templates to have ones allowing for %dx first, but
I view any such ordering dependencies as fragile.

Jan

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

[H.J. Lu]

On Tue, Nov 8, 2022 at 11:21 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.11.2022 22:06, H.J. Lu wrote:
> > On Mon, Nov 7, 2022 at 11:34 PM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 07.11.2022 20:58, H.J. Lu wrote:
> >>> On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> 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
> >>>
> >>> Since current_templates doesn't point to the matched instruction,
> >>> checking current_templates looks like abuse.  I don't think error
> >>> messages should be a concern here.
> >>
> >> We use current_templates in similar ways in quite a number of places,
> >> when match_templates() hasn't run yet.
> >
> > Since the first template isn't the selected one, your check allows
> > the invalid opcodes.
>
> I guess I don't understand, but I guess I'll also give up. Which

Your proposed change does

current_templates->start->base_opcode | 0x8a) == 0xee

to allow opcode 0xe4 and (%dx) is allowed for non-I/O opcodes.

> template the check is done against doesn't really matter here, as
> long as it's one with the correct mnemonic. We could of course
> also re-order templates to have ones allowing for %dx first, but
> I view any such ordering dependencies as fragile.
>

That is true.

I posted the v2 patch to add inw tests.

-- 
H.J.

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

[Jan Beulich]

On 09.11.2022 21:24, H.J. Lu wrote:
> On Tue, Nov 8, 2022 at 11:21 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 08.11.2022 22:06, H.J. Lu wrote:
>>> On Mon, Nov 7, 2022 at 11:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 07.11.2022 20:58, H.J. Lu wrote:
>>>>> On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> 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
>>>>>
>>>>> Since current_templates doesn't point to the matched instruction,
>>>>> checking current_templates looks like abuse.  I don't think error
>>>>> messages should be a concern here.
>>>>
>>>> We use current_templates in similar ways in quite a number of places,
>>>> when match_templates() hasn't run yet.
>>>
>>> Since the first template isn't the selected one, your check allows
>>> the invalid opcodes.
>>
>> I guess I don't understand, but I guess I'll also give up. Which
> 
> Your proposed change does
> 
> current_templates->start->base_opcode | 0x8a) == 0xee
> 
> to allow opcode 0xe4 and (%dx) is allowed for non-I/O opcodes.

0xe4 is very much an I/O opcode, merely one not allowing for (%dx).
This solely is to ...

>> template the check is done against doesn't really matter here, as
>> long as it's one with the correct mnemonic. We could of course
>> also re-order templates to have ones allowing for %dx first, but
>> I view any such ordering dependencies as fragile.
>>
> 
> That is true.

... avoid introducing yet another ordering dependency.

Jan

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

[H.J. Lu]

On Wed, Nov 9, 2022 at 11:21 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.11.2022 21:24, H.J. Lu wrote:
> > On Tue, Nov 8, 2022 at 11:21 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 08.11.2022 22:06, H.J. Lu wrote:
> >>> On Mon, Nov 7, 2022 at 11:34 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 07.11.2022 20:58, H.J. Lu wrote:
> >>>>> On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>> 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
> >>>>>
> >>>>> Since current_templates doesn't point to the matched instruction,
> >>>>> checking current_templates looks like abuse.  I don't think error
> >>>>> messages should be a concern here.
> >>>>
> >>>> We use current_templates in similar ways in quite a number of places,
> >>>> when match_templates() hasn't run yet.
> >>>
> >>> Since the first template isn't the selected one, your check allows
> >>> the invalid opcodes.
> >>
> >> I guess I don't understand, but I guess I'll also give up. Which
> >
> > Your proposed change does
> >
> > current_templates->start->base_opcode | 0x8a) == 0xee
> >
> > to allow opcode 0xe4 and (%dx) is allowed for non-I/O opcodes.
>
> 0xe4 is very much an I/O opcode, merely one not allowing for (%dx).

But it also matches other opcodes.

> This solely is to ...
>
> >> template the check is done against doesn't really matter here, as
> >> long as it's one with the correct mnemonic. We could of course
> >> also re-order templates to have ones allowing for %dx first, but
> >> I view any such ordering dependencies as fragile.
> >>
> >
> > That is true.
>
> ... avoid introducing yet another ordering dependency.
>
> Jan



-- 
H.J.

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

[Jan Beulich]

On 10.11.2022 18:22, H.J. Lu wrote:
> On Wed, Nov 9, 2022 at 11:21 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 09.11.2022 21:24, H.J. Lu wrote:
>>> On Tue, Nov 8, 2022 at 11:21 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 08.11.2022 22:06, H.J. Lu wrote:
>>>>> On Mon, Nov 7, 2022 at 11:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 07.11.2022 20:58, H.J. Lu wrote:
>>>>>>> On Mon, Nov 7, 2022 at 3:44 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>> 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
>>>>>>>
>>>>>>> Since current_templates doesn't point to the matched instruction,
>>>>>>> checking current_templates looks like abuse.  I don't think error
>>>>>>> messages should be a concern here.
>>>>>>
>>>>>> We use current_templates in similar ways in quite a number of places,
>>>>>> when match_templates() hasn't run yet.
>>>>>
>>>>> Since the first template isn't the selected one, your check allows
>>>>> the invalid opcodes.
>>>>
>>>> I guess I don't understand, but I guess I'll also give up. Which
>>>
>>> Your proposed change does
>>>
>>> current_templates->start->base_opcode | 0x8a) == 0xee
>>>
>>> to allow opcode 0xe4 and (%dx) is allowed for non-I/O opcodes.
>>
>> 0xe4 is very much an I/O opcode, merely one not allowing for (%dx).
> 
> But it also matches other opcodes.

Let's enumerate them all:

0x64	gs: prefix
0x66	data prefix
0x6c	ins
0x6e	outs
0xe4	in
0xe6	out
0xec	in
0xee	out

As said in the description: "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."

Jan