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

[Jan Beulich]

On 12.12.2023 08:10, Hu, Lin1 wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Monday, December 11, 2023 9:04 PM
>>
>> On 07.12.2023 10:01, Cui, Lili wrote:
>>> --- /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}
>>
> 
> Unfortunately, It's useless, it raised
> 
> Error: bad or irreducible absolute expression.
> Error: junk at end of line, first unrecognized character is `d'

Hmm, but parse_insn() ought to be capable of parsing "{rex2} data16 ..."
with the patch in place which introduces {rex2}. Considering that gas 2.41
processes

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

quite fine, I can't see how you would end up with parsing failing at 'd'
(i.e. "data16").

>> I notice though that
>>
>> 	.insn {rex} 0xa1, $1{:u64}
>>
>> presently ignores {rex}, which looks like a bug I ought to fix.
>>
> 
> Indeed, When I change the line like .insn data16 {rex2} 0xa1, $1{:u64}. {rex2} will be ignored too.
>  
>>
>> 	.insn rex 0xa1, $1{:u64}
>>
>> does work though (and so should {rex2}).
>>
> 
> rex2 raised the same error.
> 
> Error: bad or irreducible absolute expression.
> Error: junk at end of line, first unrecognized character is `d'

That's for

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

? If so, then yes, of course: There's no rex2 prefix so far, you're only
introducing {rex2}.

>>> +      && (ins->rex2 & 0x80) == 0x0)
>>
>> What is 0x80? DYM the respective equivalent #define for the opcode map bit
>> (which ought to be 0x8 aiui)?
> 
> 0x80 is corresponding to rex2.M0. 0x8 is corresponding to rex2.W.

Of course. But consider how the field is set:

	      ins->rex2 = rex2_payload >> 4;

Hence why it would be imperative that (a) a #define be introduced and (b)
such a #define be accompanied by a comment explaining why it's 0x8, not
0x80.

A question however is whether you need this check at all. Decoding has
already taken a different route far earlier when REX2.M is set, so aiui
execution wouldn't even make it here in that case. Hence why the
respective test actually works as expected despite the flaw here. (With
that, no new #define is going to be needed, as the code will just
disappear.)

Jan