Re: [PATCH v3 1/2] RISC-V: Fallback for instructions longer than 64b

[Nelson Chu <nelson@rivosinc.com>]

In fact we don't really need this change, since so far the parameter
length of validate_riscv_insn will only be 2 and 4,
https://github.com/bminor/binutils-gdb/blob/master/gas/config/tc-riscv.c#L1349

Nelson

On Thu, Oct 6, 2022 at 5:56 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> We don't support instructions longer than 64-bits yet.  Still, we can
> modify validate_riscv_insn function to prevent unexpected behavior by
> limiting the "length" of an instruction to 64-bit (or less).
>
> gas/ChangeLog:
>
>         * config/tc-riscv.c (validate_riscv_insn): Fix function
>         description comment based on current spec.  Limit instruction
>         length up to 64-bit for now.  Make sure that required_bits does
>         not corrupt even if unsigned long long is longer than 64-bit.
> ---
>  gas/config/tc-riscv.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 22385d1baa0..41d6dfc6062 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -1109,7 +1109,8 @@ arg_lookup (char **s, const char *const *array, size_t size, unsigned *regnop)
>
>  /* For consistency checking, verify that all bits are specified either
>     by the match/mask part of the instruction definition, or by the
> -   operand list. The `length` could be 0, 4 or 8, 0 for auto detection.  */
> +   operand list. The `length` could be the actual instruction length or
> +   0 for auto-detection.  */
>
>  static bool
>  validate_riscv_insn (const struct riscv_opcode *opc, int length)
> @@ -1120,11 +1121,13 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
>    insn_t required_bits;
>
>    if (length == 0)
> -    insn_width = 8 * riscv_insn_length (opc->match);
> -  else
> -    insn_width = 8 * length;
> +    length = riscv_insn_length (opc->match);
> +  /* We don't support instructions longer than 64-bits yet.  */
> +  if (length > 8)
> +    length = 8;
> +  insn_width = 8 * length;
>
> -  required_bits = ~0ULL >> (64 - insn_width);
> +  required_bits = ((insn_t)~0ULL) >> (64 - insn_width);
>
>    if ((used_bits & opc->match) != (opc->match & required_bits))
>      {
> --
> 2.34.1
>