[PATCH v4 9/9] Support APX JMPABS for disassembler

[Jan Beulich]

On 07.12.2023 10:01, Cui, Lili wrote:
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-jmpabs-intel.d
> @@ -0,0 +1,11 @@
> +#as:
> +#objdump: -dw -Mintel
> +#name: x86_64 APX_F JMPABS insns (Intel disassembly)
> +#source: x86-64-apx-jmpabs.s
> +
> +.*: +file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <_start>:
> +\s*[a-f0-9]+:\s*d5 00 a1 02 00 00 00 00 00 00 00[	 ]+jmpabs 0x2

Missing #pass ?

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-jmpabs-inval.d
> @@ -0,0 +1,40 @@
> +#as: --64
> +#objdump: -dw
> +#name: illegal decoding of APX_F jmpabs insns
> +#source: x86-64-apx-jmpabs-inval.s
> +
> +.*: +file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <.text>:
> +\s*[a-f0-9]+:	66 d5 00 a1[  	]+\(bad\)
> +\s*[a-f0-9]+:	01 00[  	]+add    %eax,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	67 d5 00 a1[  	]+\(bad\)
> +\s*[a-f0-9]+:	01 00[  	]+add    %eax,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	f2 d5 00 a1[  	]+\(bad\)
> +\s*[a-f0-9]+:	01 00[  	]+add    %eax,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	f3 d5 00 a1[  	]+\(bad\)
> +\s*[a-f0-9]+:	01 00[  	]+add    %eax,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	f0 d5 00 a1[  	]+\(bad\)
> +\s*[a-f0-9]+:	01 00[  	]+add    %eax,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	d5 08 a1[  	]+\(bad\)
> +\s*[a-f0-9]+:	01 00[  	]+add    %eax,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*[a-f0-9]+:	00 00[  	]+add    %al,\(%rax\)
> +\s*...

The more "canonical" way of expressing this is #pass.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-jmpabs-inval.s
> @@ -0,0 +1,15 @@
> +# Check bytecode of APX_F jmpabs instructions with illegal encode.
> +
> +	.text
> +# With 66 prefix
> +	.byte 0x66,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00

At the example of this, can't this be

	.insn {rex2} data16 0xa1, $1{:u64}

I notice though that

	.insn {rex} 0xa1, $1{:u64}

presently ignores {rex}, which looks like a bug I ought to fix.

	.insn rex 0xa1, $1{:u64}

does work though (and so should {rex2}).

> +# With 67 prefix
> +	.byte 0x67,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +# With F2 prefix
> +	.byte 0xf2,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +# With F3 prefix
> +	.byte 0xf3,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +# With LOCK prefix
> +	.byte 0xf0,0xd5,0x00,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00
> +# REX2.M0 = 0 REX2.W = 1
> +	.byte 0xd5,0x08,0xa1,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00

Here {rex2} wouldn't be sufficient, so at least partly using .byte is
going to be (largely) unavoidable for the time being.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-jmpabs.d
> @@ -0,0 +1,11 @@
> +#as:
> +#objdump: -dw
> +#name: x86_64 APX_F JMPABS insns
> +#source: x86-64-apx-jmpabs.s
> +
> +.*: +file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <_start>:
> +\s*[a-f0-9]+:\s*d5 00 a1 02 00 00 00 00 00 00 00[	 ]+jmpabs \$0x2

Missing #pass again?

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-jmpabs.s
> @@ -0,0 +1,5 @@
> +# Check 64bit APX_F JMPABS instructions
> +
> +	.text
> + _start:
> +	.byte 0xd5,0x00,0xa1,0x02,0x00,0x00,0x00,0x00,0x00,0x00,0x00

As per above, please use .insn here as well it at all possible.

> @@ -9777,7 +9778,7 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
>      ins.all_prefixes[ins.last_rex_prefix] = 0;
>  
>    /* Check if the REX2 prefix is used.  */
> -  if (ins.last_rex2_prefix >= 0 && (ins.rex2 & 7))
> +  if (ins.last_rex2_prefix >= 0 && (ins.rex2 & (7 | REX2_SPECIAL)))
>      ins.all_prefixes[ins.last_rex2_prefix] = 0;

Seeing that REX2_SPECIAL isn't introduced here, doesn't this belong in the
PUSHP/POPP patch then?

> @@ -13933,3 +13934,37 @@ PUSH2_POP2_Fixup (instr_info *ins, int bytemode, int sizeflag)
>  
>    return OP_VEX (ins, bytemode, sizeflag);
>  }
> +
> +static bool
> +JMPABS_Fixup (instr_info *ins, int bytemode, int sizeflag)
> +{
> +  if (ins->address_mode == mode_64bit

I think I had asked before already: Is this really needed when ...

> +      && ins->last_rex2_prefix >= 0

... this can be non-negative only in 64-bit mode anyway?

> +      && (ins->rex2 & 0x80) == 0x0)

What is 0x80? DYM the respective equivalent #define for the opcode
map bit (which ought to be 0x8 aiui)?

Further on what basis was the split determined between the conditions
here and ...

> +    {
> +      uint64_t op;
> +
> +      if (bytemode == eAX_reg)
> +	return true;
> +
> +      if (!get64 (ins, &op))
> +	return false;
> +
> +      if ((ins->prefixes & (PREFIX_OPCODE | PREFIX_ADDR | PREFIX_LOCK)) != 0x0
> +	  || (ins->rex & REX_W) != 0x0)

... these further ones?

Overall it's a bit sad of course to have only the disassembler side of this,
but well ...

Jan