Re: [PATCH bpf-next 2/7] arm64: insn: Add encoder for ADD/SUB (extended register)

[email protected] writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The encoder for ADD/SUB (extended register) fails to reject `UXTX` and `SXTX` extensions when generating a 32-bit variant, which results in an UNALLOCATED instruction.
> --
>
> --- Patch [2]: [PATCH bpf-next 2/7] arm64: insn: Add encoder for ADD/SUB (extended register) ---
> commit 286e3f9944297bcaa48fe3c90eb0bde9f1e85a33
> Author: Tejun Heo <[email protected]>
>
> arm64: insn: Add encoder for ADD/SUB (extended register)
>
> This commit adds the aarch64_insn_gen_add_sub_extended_reg() function to the
> insn library to support encoding the extended-register form of ADD and SUB
> instructions. It is modeled on the shifted-register generator and uses the
> existing IMM_6 field for the option and imm3 fields.
>
>> diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
>> index 37ce75f7f1f08..e70ac02385153 100644
>> --- a/arch/arm64/lib/insn.c
>> +++ b/arch/arm64/lib/insn.c
>
> [ ... ]
>
>> +u32 aarch64_insn_gen_add_sub_extended_reg(enum aarch64_insn_register dst,
>> +					  enum aarch64_insn_register src,
>> +					  enum aarch64_insn_register reg,
>> +					  enum aarch64_insn_extend_type extend,
>> +					  int shift,
>> +					  enum aarch64_insn_variant variant,
>> +					  enum aarch64_insn_adsb_type type)
>> +{
>
> [ ... ]
>
>> +	switch (variant) {
>> +	case AARCH64_INSN_VARIANT_32BIT:
>> +		break;
>
> [Severity: Medium]
> Should this case validate that the extend parameter is not UXTX or SXTX?
>
> The ARM Architecture Reference Manual indicates that for 32-bit extended
> register operations (sf == 0), the option field values 011 (UXTX) and
> 111 (SXTX) are UNALLOCATED and lead to an UNDEFINED exception at execution
> time.
>

The manual does not indicate that, 011 UXTX and 111 SXTX are valid
values.