Re: [PATCH v2] RISC-V: Add string length check for operands in AS

[Nelson Chu <nelson@rivosinc.com>]

Looks reasonable so committed with passing binutils testsuites.

On Wed, Dec 14, 2022 at 3:33 PM Li Xu <xuli1@eswincomputing.com> wrote:
>
> The patch I previously submitted:
> | Date: Tue Dec 13 04:34:28 GMT 2022
> | Subject: [PATCH] RISC-V: Add string length check for operands in AS
> | Message-ID: <xuli1@eswincomputing.com>
>
> The current AS accepts invalid operands due to miss of operands length check.
> For example, "e6" is an invalid operand in (vsetvli a0, a1, e6, mf8, tu, ma),
> but it's still accepted by assembler. In detail, the condition check "strncmp
> (array[i], *s, len) == 0" in arg_lookup function passes with "strncmp ("e64",
> "e6", 2)" in the case above. So the generated encoding is same as that of
> (vsetvli a0, a1, e64, mf8, tu, ma).
> This patch fixes issue above by prompting an error in such case and also adds
> a new testcase.
>
> gas/ChangeLog:
>
>         * config/tc-riscv.c (arg_lookup): Add string length check for operands.
>         * testsuite/gas/riscv/vector-insns-fail-vsew.d: New testcase for an illegal vsew.
>         * testsuite/gas/riscv/vector-insns-fail-vsew.l: Likewise.
>         * testsuite/gas/riscv/vector-insns-fail-vsew.s: Likewise.
> ---
>  gas/config/tc-riscv.c                            | 3 ++-
>  gas/testsuite/gas/riscv/vector-insns-fail-vsew.d | 3 +++
>  gas/testsuite/gas/riscv/vector-insns-fail-vsew.l | 3 +++
>  gas/testsuite/gas/riscv/vector-insns-fail-vsew.s | 1 +
>  4 files changed, 9 insertions(+), 1 deletion(-)
>  create mode 100644 gas/testsuite/gas/riscv/vector-insns-fail-vsew.d
>  create mode 100644 gas/testsuite/gas/riscv/vector-insns-fail-vsew.l
>  create mode 100644 gas/testsuite/gas/riscv/vector-insns-fail-vsew.s
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 0682eb35524..42c041155c5 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -1206,7 +1206,8 @@ arg_lookup (char **s, const char *const *array, size_t size, unsigned *regnop)
>      return false;
>
>    for (i = 0; i < size; i++)
> -    if (array[i] != NULL && strncmp (array[i], *s, len) == 0)
> +    if (array[i] != NULL && strncmp (array[i], *s, len) == 0
> +        && array[i][len] == '\0')

It is minor that 8 spaces should be replaced by a tab.  Otherwise it looks good.

Thanks
Nelson

>        {
>         *regnop = i;
>         *s += len;
> diff --git a/gas/testsuite/gas/riscv/vector-insns-fail-vsew.d b/gas/testsuite/gas/riscv/vector-insns-fail-vsew.d
> new file mode 100644
> index 00000000000..c0c81579741
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/vector-insns-fail-vsew.d
> @@ -0,0 +1,3 @@
> +#as: -march=rv32ifv
> +#source: vector-insns-fail-vsew.s
> +#error_output: vector-insns-fail-vsew.l
> diff --git a/gas/testsuite/gas/riscv/vector-insns-fail-vsew.l b/gas/testsuite/gas/riscv/vector-insns-fail-vsew.l
> new file mode 100644
> index 00000000000..87a2c22a805
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/vector-insns-fail-vsew.l
> @@ -0,0 +1,3 @@
> +.*: Assembler messages:
> +.*: Error: instruction vsetvli requires absolute expression
> +.*: Error: illegal operands `vsetvli a0,a1,e6,mf8,tu,ma'
> diff --git a/gas/testsuite/gas/riscv/vector-insns-fail-vsew.s b/gas/testsuite/gas/riscv/vector-insns-fail-vsew.s
> new file mode 100644
> index 00000000000..b8f3242406f
> --- /dev/null
> +++ b/gas/testsuite/gas/riscv/vector-insns-fail-vsew.s
> @@ -0,0 +1 @@
> +       vsetvli  a0, a1, e6, mf8, tu, ma                # unrecognized vsew
> --
> 2.17.1
>