Re: [PATCH 2/6] Disable pseudo prefix {rex2} for illegal instructions.

[Jan Beulich <jbeulich@suse.com>]

On 21.09.2023 12:11, Cui, Lili wrote:
> All opcodes listed map0 0x4*/0x7*/0xa* and map0 0x3*/0x8*
> are reserved under REX2 and triggers #UD when prefixed with REX2

For the latter I guess you mean map1, and then yes, this matches what the
doc says, but I still wonder: There are more ModR/M-less insns, for all
of which imo REX2 would likely want to be invalid. IOW - is this really
(going to remain) a strictly opcode-row-based constraint?

In any event, considering that you already made i386-gen generate a fair
number of these attributes, can't it take care of the opcode rows here as
well?

> --- a/gas/testsuite/gas/i386/x86-64-pseudos-bad.s
> +++ b/gas/testsuite/gas/i386/x86-64-pseudos-bad.s
> @@ -5,3 +5,45 @@ pseudos:
>  	{rex} vmovaps %xmm7,%xmm2
>  	{rex} vmovaps %xmm17,%xmm2
>  	{rex} rorx $7,%eax,%ebx
> +
> +	#All opcodes in the row 0xa* prefixed REX2 are illegal.
> +	{rex2} mov    0x90909090,%al
> +	{rex2} movabs 0x1,%al
> +	{rex2} cmpsb  %es:(%edi),%ds:(%esi)
> +	{rex2} lodsb
> +	{rex2} lods   %ds:(%esi),%al
> +	{rex2} lodsb   (%esi)
> +	{rex2} movsb
> +	{rex2} movsb   (%esi), (%edi)
> +	{rex2} scasl
> +	{rex2} scas   %es:(%edi),%eax
> +	{rex2} scasb   (%edi)
> +	{rex2} stosb
> +	{rex2} stosb   (%edi)
> +	{rex2} stos   %eax,%es:(%edi)
> +
> +	#All opcodes in the row 0x7* prefixed REX2 are illegal.
> +	{rex2} jo     .+2-0x70
> +	{rex2} jno    .+2-0x70
> +	{rex2} jb     .+2-0x70
> +	{rex2} jae    .+2-0x70
> +	{rex2} je     .+2-0x70
> +	{rex2} jne    .+2-0x70
> +	{rex2} jbe    .+2-0x70
> +	{rex2} ja     .+2-0x70
> +	{rex2} js     .+2-0x70
> +	{rex2} jns    .+2-0x70
> +	{rex2} jp     .+2-0x70
> +	{rex2} jnp    .+2-0x70
> +	{rex2} jl     .+2-0x70
> +	{rex2} jge    .+2-0x70
> +	{rex2} jle    .+2-0x70
> +	{rex2} jg     .+2-0x70
> +
> +	#All opcodes in the row 0xf3* prefixed REX2 are illegal.

0xf3 is a REP prefix. Please avoid the addition os misleading comments.

Also mention of map1 row 0x8 is missing (these being the 32-disp Jcc,
they won't need separate testing, but they want mentioning).

> --- a/gas/testsuite/gas/i386/x86-64-pseudos.d
> +++ b/gas/testsuite/gas/i386/x86-64-pseudos.d
> @@ -448,7 +448,6 @@ Disassembly of section .text:
>   +[a-f0-9]+:	41 0f 38 01 00       	phaddw \(%r8\),%mm0
>   +[a-f0-9]+:	88 c4                	mov    %al,%ah
>   +[a-f0-9]+:	d5 00 d3 e0          	{rex2} shl %cl,%eax
> - +[a-f0-9]+:	d5 00 a0 01 00 00 00 00 00 00 00 	{rex2} movabs 0x1,%al
>   +[a-f0-9]+:	d5 00 38 ca          	{rex2} cmp %cl,%dl
>   +[a-f0-9]+:	d5 00 b3 01          	{rex2} mov \$(0x)?1,%bl
>   +[a-f0-9]+:	d5 00 89 c3          	{rex2} mov %eax,%ebx
> --- a/gas/testsuite/gas/i386/x86-64-pseudos.s
> +++ b/gas/testsuite/gas/i386/x86-64-pseudos.s
> @@ -405,7 +405,6 @@ _start:
>  	{rex} phaddw (%r8),%mm0
>  	{rex2} mov %al,%ah
>  	{rex2} shl %cl, %eax
> -	{rex2} movabs 1, %al
>  	{rex2} cmp %cl, %dl
>  	{rex2} mov $1, %bl
>  	{rex2} movl %eax,%ebx

If this one insn is wrong to have here, why was it put here in the first place?

Jan