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

[Jan Beulich <jbeulich@suse.com>]

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