[v2,1/2] RISC-V: Fallback for instructions longer than 64b
Checks
Commit Message
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 usage. 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 | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Comments
On 06.10.2022 06:40, Tsukasa OI via Binutils 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 usage. 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.
While I agree with the code change, I don't agree with the adjustment
to the comment - you're changing it to match the sole present caller,
but imo such a comment ought to describe the behavior of the function
irrespective of how it's used at any given point in time.
Jan
On 2022/10/06 17:22, Jan Beulich wrote:
> On 06.10.2022 06:40, Tsukasa OI via Binutils 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 usage. 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.
>
> While I agree with the code change, I don't agree with the adjustment
> to the comment - you're changing it to match the sole present caller,
> but imo such a comment ought to describe the behavior of the function
> irrespective of how it's used at any given point in time.
>
> Jan
>
Okay, I revised the comment to reflect the specification itself.
Thanks,
Tsukasa
@@ -1109,7 +1109,7 @@ 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 0, 2 or 4, 0 for auto detection. */
static bool
validate_riscv_insn (const struct riscv_opcode *opc, int length)
@@ -1120,11 +1120,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))
{