* [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) @ 2022-05-23 10:06 Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 1/3] RISC-V: Add xlen to match_func Tsukasa OI ` (4 more replies) 0 siblings, 5 replies; 7+ messages in thread From: Tsukasa OI @ 2022-05-23 10:06 UTC (permalink / raw) To: Tsukasa OI, Palmer Dabbelt, Nelson Chu, Kito Cheng; +Cc: binutils ** Note ** This patch is not compatible with my previous Zfinx fixes. Actual PATCH v1 is going to be submitted after this patchset is reviewed and Zfinx fixes (relatively high priority) are applied. Certain shift instructions have a constraint: shift amount must be less than current XLEN. This constraint is checked on assembler and simulator, but not on disassembler. It causes GDB to print wrong (and invalid) instructions and can be a problem while ... for instance, checking for invalid build configuration. We have several methods to deal with it: - Add `xlen' argument to match_func to test XLEN - Split shift instructions to per-XLEN variants Because shift instructions are so basic and XLEN-checking is so simple, I chose the former. The latter is used on my Zfinx fixes but this is because register pair checking involves relatively complex per-operand constraints. Complex solution for simple problem can be a new problem. But... - Is it okay to add an argument anyway? - Which is better extra argument? 1. unsigned xlen 2. const riscv_parse_subset_t * subset which contains ISA version, XLEN and extensions ... but a part of BFD, not opcodes. I also (slightly) changed the assembler to suppress extra error message just like I did on Zicbop instructions: <https://sourceware.org/pipermail/binutils/2021-December/118910.html> <https://sourceware.org/pipermail/binutils/2021-December/118938.html> Tsukasa OI (3): RISC-V: Add xlen to match_func RISC-V: Check shift amount against XLEN RISC-V: Add disassembler tests for shift amount gas/config/tc-riscv.c | 8 +- gas/testsuite/gas/riscv/shamt-dis-32.d | 34 +++++++ gas/testsuite/gas/riscv/shamt-dis-64.d | 34 +++++++ gas/testsuite/gas/riscv/shamt-dis.s | 45 +++++++++ include/opcode/riscv.h | 3 +- opcodes/riscv-dis.c | 2 +- opcodes/riscv-opc.c | 122 +++++++++++++++---------- sim/riscv/sim-main.c | 2 +- 8 files changed, 195 insertions(+), 55 deletions(-) create mode 100644 gas/testsuite/gas/riscv/shamt-dis-32.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis-64.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis.s base-commit: cb0d58bf4d274cfb1ae11b75bd2b3ba81c8d371d -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH 1/3] RISC-V: Add xlen to match_func 2022-05-23 10:06 [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI @ 2022-05-23 10:06 ` Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 2/3] RISC-V: Check shift amount against XLEN Tsukasa OI ` (3 subsequent siblings) 4 siblings, 0 replies; 7+ messages in thread From: Tsukasa OI @ 2022-05-23 10:06 UTC (permalink / raw) To: Tsukasa OI, Palmer Dabbelt, Nelson Chu, Kito Cheng; +Cc: binutils For shift amount checking against current XLEN, we need to add third argument `xlen' to opcode-matching function. gas/ChangeLog: * config/tc-riscv.c (riscv_ip): Call match_func with XLEN. include/ChangeLog: * opcode/riscv.h (struct riscv_opcode): Modify match_func so that XLEN can be matched. opcodes/ChangeLog: * riscv-dis.c (riscv_disassemble_insn): Call match_func with XLEN. * riscv-opc.c (match_opcode, match_never, match_rs1_eq_rs2, match_rd_nonzero, match_c_add, match_c_add_with_hint, match_c_nop, match_c_addi16sp, match_c_lui, match_c_lui_with_hint, match_c_addi4spn, match_c_slli, match_slli_as_c_slli, match_c_slli64, match_srxi_as_c_srxi, match_vs1_eq_vs2, match_vd_eq_vs1_eq_vs2): Add argument `xlen'. ChangeLog: * sim/riscv/sim-main.c (step_once): Call match_func with XLEN. --- gas/config/tc-riscv.c | 2 +- include/opcode/riscv.h | 3 +- opcodes/riscv-dis.c | 2 +- opcodes/riscv-opc.c | 76 ++++++++++++++++++++++++------------------ sim/riscv/sim-main.c | 2 +- 5 files changed, 49 insertions(+), 36 deletions(-) diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c index 1b730b4be36..b207b2d30c8 100644 --- a/gas/config/tc-riscv.c +++ b/gas/config/tc-riscv.c @@ -2309,7 +2309,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr, case '\0': /* End of args. */ if (insn->pinfo != INSN_MACRO) { - if (!insn->match_func (insn, ip->insn_opcode)) + if (!insn->match_func (insn, ip->insn_opcode, xlen)) break; /* For .insn, insn->match and insn->mask are 0. */ diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h index 0d1fbcf8fc5..6ee2640cac1 100644 --- a/include/opcode/riscv.h +++ b/include/opcode/riscv.h @@ -426,7 +426,8 @@ struct riscv_opcode /* A function to determine if a word corresponds to this instruction. Usually, this computes ((word & mask) == match). */ - int (*match_func) (const struct riscv_opcode *op, insn_t word); + int (*match_func) (const struct riscv_opcode *op, insn_t word, + unsigned xlen); /* For a macro, this is INSN_MACRO. Otherwise, it is a collection of bits describing the instruction, notably any relevant hazard diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c index 9ff31167775..4738c6a98c7 100644 --- a/opcodes/riscv-dis.c +++ b/opcodes/riscv-dis.c @@ -644,7 +644,7 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info) for (; op->name; op++) { /* Does the opcode match? */ - if (! (op->match_func) (op, word)) + if (! (op->match_func) (op, word, xlen)) continue; /* Is this a pseudo-instruction and may we print it as such? */ if (no_aliases && (op->pinfo & INSN_ALIAS)) diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c index bbd4a3718f6..636e67a3eb0 100644 --- a/opcodes/riscv-opc.c +++ b/opcodes/riscv-opc.c @@ -132,65 +132,71 @@ const char * const riscv_vma[2] = #define MASK_VMASK (OP_MASK_VMASK << OP_SH_VMASK) static int -match_opcode (const struct riscv_opcode *op, insn_t insn) +match_opcode (const struct riscv_opcode *op, insn_t insn, + unsigned xlen ATTRIBUTE_UNUSED) { return ((insn ^ op->match) & op->mask) == 0; } static int match_never (const struct riscv_opcode *op ATTRIBUTE_UNUSED, - insn_t insn ATTRIBUTE_UNUSED) + insn_t insn ATTRIBUTE_UNUSED, + unsigned xlen ATTRIBUTE_UNUSED) { return 0; } static int -match_rs1_eq_rs2 (const struct riscv_opcode *op, insn_t insn) +match_rs1_eq_rs2 (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { int rs1 = (insn & MASK_RS1) >> OP_SH_RS1; int rs2 = (insn & MASK_RS2) >> OP_SH_RS2; - return match_opcode (op, insn) && rs1 == rs2; + return match_opcode (op, insn, xlen) && rs1 == rs2; } static int -match_rd_nonzero (const struct riscv_opcode *op, insn_t insn) +match_rd_nonzero (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { - return match_opcode (op, insn) && ((insn & MASK_RD) != 0); + return match_opcode (op, insn, xlen) && ((insn & MASK_RD) != 0); } static int -match_c_add (const struct riscv_opcode *op, insn_t insn) +match_c_add (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { - return match_rd_nonzero (op, insn) && ((insn & MASK_CRS2) != 0); + return match_rd_nonzero (op, insn, xlen) && ((insn & MASK_CRS2) != 0); } /* We don't allow mv zero,X to become a c.mv hint, so we need a separate matching function for this. */ static int -match_c_add_with_hint (const struct riscv_opcode *op, insn_t insn) +match_c_add_with_hint (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { - return match_opcode (op, insn) && ((insn & MASK_CRS2) != 0); + return match_opcode (op, insn, xlen) && ((insn & MASK_CRS2) != 0); } static int -match_c_nop (const struct riscv_opcode *op, insn_t insn) +match_c_nop (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { - return (match_opcode (op, insn) + return (match_opcode (op, insn, xlen) && (((insn & MASK_RD) >> OP_SH_RD) == 0)); } static int -match_c_addi16sp (const struct riscv_opcode *op, insn_t insn) +match_c_addi16sp (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { - return (match_opcode (op, insn) + return (match_opcode (op, insn, xlen) && (((insn & MASK_RD) >> OP_SH_RD) == 2)); } static int -match_c_lui (const struct riscv_opcode *op, insn_t insn) +match_c_lui (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { - return (match_rd_nonzero (op, insn) + return (match_rd_nonzero (op, insn, xlen) && (((insn & MASK_RD) >> OP_SH_RD) != 2) && EXTRACT_CITYPE_LUI_IMM (insn) != 0); } @@ -199,71 +205,77 @@ match_c_lui (const struct riscv_opcode *op, insn_t insn) matching function for this. */ static int -match_c_lui_with_hint (const struct riscv_opcode *op, insn_t insn) +match_c_lui_with_hint (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { - return (match_opcode (op, insn) + return (match_opcode (op, insn, xlen) && (((insn & MASK_RD) >> OP_SH_RD) != 2) && EXTRACT_CITYPE_LUI_IMM (insn) != 0); } static int -match_c_addi4spn (const struct riscv_opcode *op, insn_t insn) +match_c_addi4spn (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { - return match_opcode (op, insn) && EXTRACT_CIWTYPE_ADDI4SPN_IMM (insn) != 0; + return match_opcode (op, insn, xlen) + && EXTRACT_CIWTYPE_ADDI4SPN_IMM (insn) != 0; } /* This requires a non-zero shift. A zero rd is a hint, so is allowed. */ static int -match_c_slli (const struct riscv_opcode *op, insn_t insn) +match_c_slli (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { - return match_opcode (op, insn) && EXTRACT_CITYPE_IMM (insn) != 0; + return match_opcode (op, insn, xlen) && EXTRACT_CITYPE_IMM (insn) != 0; } /* This requires a non-zero rd, and a non-zero shift. */ static int -match_slli_as_c_slli (const struct riscv_opcode *op, insn_t insn) +match_slli_as_c_slli (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { - return match_rd_nonzero (op, insn) && EXTRACT_CITYPE_IMM (insn) != 0; + return match_rd_nonzero (op, insn, xlen) + && EXTRACT_CITYPE_IMM (insn) != 0; } /* This requires a zero shift. A zero rd is a hint, so is allowed. */ static int -match_c_slli64 (const struct riscv_opcode *op, insn_t insn) +match_c_slli64 (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { - return match_opcode (op, insn) && EXTRACT_CITYPE_IMM (insn) == 0; + return match_opcode (op, insn, xlen) && EXTRACT_CITYPE_IMM (insn) == 0; } /* This is used for both srli and srai. This requires a non-zero shift. A zero rd is not possible. */ static int -match_srxi_as_c_srxi (const struct riscv_opcode *op, insn_t insn) +match_srxi_as_c_srxi (const struct riscv_opcode *op, insn_t insn, + unsigned xlen) { - return match_opcode (op, insn) && EXTRACT_CITYPE_IMM (insn) != 0; + return match_opcode (op, insn, xlen) && EXTRACT_CITYPE_IMM (insn) != 0; } static int match_vs1_eq_vs2 (const struct riscv_opcode *op, - insn_t insn) + insn_t insn, unsigned xlen) { int vs1 = (insn & MASK_VS1) >> OP_SH_VS1; int vs2 = (insn & MASK_VS2) >> OP_SH_VS2; - return match_opcode (op, insn) && vs1 == vs2; + return match_opcode (op, insn, xlen) && vs1 == vs2; } static int match_vd_eq_vs1_eq_vs2 (const struct riscv_opcode *op, - insn_t insn) + insn_t insn, unsigned xlen) { int vd = (insn & MASK_VD) >> OP_SH_VD; int vs1 = (insn & MASK_VS1) >> OP_SH_VS1; int vs2 = (insn & MASK_VS2) >> OP_SH_VS2; - return match_opcode (op, insn) && vd == vs1 && vs1 == vs2; + return match_opcode (op, insn, xlen) && vd == vs1 && vs1 == vs2; } const struct riscv_opcode riscv_opcodes[] = diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c index 62f475671c9..0b0a021a1c6 100644 --- a/sim/riscv/sim-main.c +++ b/sim/riscv/sim-main.c @@ -983,7 +983,7 @@ void step_once (SIM_CPU *cpu) for (; op->name; op++) { /* Does the opcode match? */ - if (! op->match_func (op, iw)) + if (! op->match_func (op, iw, xlen)) continue; /* Is this a pseudo-instruction and may we print it as such? */ if (op->pinfo & INSN_ALIAS) -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH 2/3] RISC-V: Check shift amount against XLEN 2022-05-23 10:06 [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 1/3] RISC-V: Add xlen to match_func Tsukasa OI @ 2022-05-23 10:06 ` Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 3/3] RISC-V: Add disassembler tests for shift amount Tsukasa OI ` (2 subsequent siblings) 4 siblings, 0 replies; 7+ messages in thread From: Tsukasa OI @ 2022-05-23 10:06 UTC (permalink / raw) To: Tsukasa OI, Palmer Dabbelt, Nelson Chu, Kito Cheng; +Cc: binutils Although assembler does the equivalent, disassembler did not have shift amount checking. This commit makes such invalid RV32 instruction invalid by checking shift amount against current XLEN. gas/ChangeLog: * config/tc-riscv.c (riscv_ip): Mask shift amount to suppress extra assembler error messages. opcodes/ChangeLog: * riscv-opc.c (match_shamt): New. (match_c_slli, match_slli_as_c_slli, match_srxi_as_c_srxi): Check shift amount against current xlen. --- gas/config/tc-riscv.c | 6 +++-- opcodes/riscv-opc.c | 52 ++++++++++++++++++++++++++----------------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c index b207b2d30c8..66907bdf0f2 100644 --- a/gas/config/tc-riscv.c +++ b/gas/config/tc-riscv.c @@ -2403,7 +2403,8 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr, || imm_expr->X_op != O_constant || (unsigned long) imm_expr->X_add_number >= xlen) break; - ip->insn_opcode |= ENCODE_CITYPE_IMM (imm_expr->X_add_number); + ip->insn_opcode |= + ENCODE_CITYPE_IMM (imm_expr->X_add_number & (xlen - 1)); rvc_imm_done: asarg = expr_end; imm_expr->X_op = O_absent; @@ -2843,7 +2844,8 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr, if ((unsigned long) imm_expr->X_add_number >= xlen) as_bad (_("improper shift amount (%lu)"), (unsigned long) imm_expr->X_add_number); - INSERT_OPERAND (SHAMT, *ip, imm_expr->X_add_number); + INSERT_OPERAND (SHAMT, *ip, + imm_expr->X_add_number & (xlen - 1)); imm_expr->X_op = O_absent; asarg = expr_end; continue; diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c index 636e67a3eb0..b71a9f4711a 100644 --- a/opcodes/riscv-opc.c +++ b/opcodes/riscv-opc.c @@ -162,6 +162,13 @@ match_rd_nonzero (const struct riscv_opcode *op, insn_t insn, return match_opcode (op, insn, xlen) && ((insn & MASK_RD) != 0); } +static int +match_shamt (const struct riscv_opcode *op, insn_t insn, unsigned xlen) +{ + unsigned shamt = (insn & MASK_SHAMT) >> OP_SH_SHAMT; + return match_opcode (op, insn, xlen) && shamt < xlen; +} + static int match_c_add (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { @@ -226,7 +233,9 @@ match_c_addi4spn (const struct riscv_opcode *op, insn_t insn, static int match_c_slli (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { - return match_opcode (op, insn, xlen) && EXTRACT_CITYPE_IMM (insn) != 0; + return match_opcode (op, insn, xlen) + && EXTRACT_CITYPE_IMM (insn) != 0 + && (EXTRACT_CITYPE_IMM (insn) & (RISCV_RVC_IMM_REACH - 1)) < xlen; } /* This requires a non-zero rd, and a non-zero shift. */ @@ -236,7 +245,8 @@ match_slli_as_c_slli (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { return match_rd_nonzero (op, insn, xlen) - && EXTRACT_CITYPE_IMM (insn) != 0; + && EXTRACT_CITYPE_IMM (insn) != 0 + && (EXTRACT_CITYPE_IMM (insn) & (RISCV_RVC_IMM_REACH - 1)) < xlen; } /* This requires a zero shift. A zero rd is a hint, so is allowed. */ @@ -254,7 +264,9 @@ static int match_srxi_as_c_srxi (const struct riscv_opcode *op, insn_t insn, unsigned xlen) { - return match_opcode (op, insn, xlen) && EXTRACT_CITYPE_IMM (insn) != 0; + return match_opcode (op, insn, xlen) + && EXTRACT_CITYPE_IMM (insn) != 0 + && (EXTRACT_CITYPE_IMM (insn) & (RISCV_RVC_IMM_REACH - 1)) < xlen; } static int @@ -371,20 +383,20 @@ const struct riscv_opcode riscv_opcodes[] = {"la.tls.ie", 0, INSN_CLASS_I, "d,A", 0, (int) M_LA_TLS_IE, match_never, INSN_MACRO }, {"neg", 0, INSN_CLASS_I, "d,t", MATCH_SUB, MASK_SUB|MASK_RS1, match_opcode, INSN_ALIAS }, /* sub 0 */ {"slli", 0, INSN_CLASS_C, "d,CU,C>", MATCH_C_SLLI, MASK_C_SLLI, match_slli_as_c_slli, INSN_ALIAS }, -{"slli", 0, INSN_CLASS_I, "d,s,>", MATCH_SLLI, MASK_SLLI, match_opcode, 0 }, +{"slli", 0, INSN_CLASS_I, "d,s,>", MATCH_SLLI, MASK_SLLI, match_shamt, 0 }, {"sll", 0, INSN_CLASS_C, "d,CU,C>", MATCH_C_SLLI, MASK_C_SLLI, match_slli_as_c_slli, INSN_ALIAS }, {"sll", 0, INSN_CLASS_I, "d,s,t", MATCH_SLL, MASK_SLL, match_opcode, 0 }, -{"sll", 0, INSN_CLASS_I, "d,s,>", MATCH_SLLI, MASK_SLLI, match_opcode, INSN_ALIAS }, +{"sll", 0, INSN_CLASS_I, "d,s,>", MATCH_SLLI, MASK_SLLI, match_shamt, INSN_ALIAS }, {"srli", 0, INSN_CLASS_C, "Cs,Cw,C>", MATCH_C_SRLI, MASK_C_SRLI, match_srxi_as_c_srxi, INSN_ALIAS }, -{"srli", 0, INSN_CLASS_I, "d,s,>", MATCH_SRLI, MASK_SRLI, match_opcode, 0 }, +{"srli", 0, INSN_CLASS_I, "d,s,>", MATCH_SRLI, MASK_SRLI, match_shamt, 0 }, {"srl", 0, INSN_CLASS_C, "Cs,Cw,C>", MATCH_C_SRLI, MASK_C_SRLI, match_srxi_as_c_srxi, INSN_ALIAS }, {"srl", 0, INSN_CLASS_I, "d,s,t", MATCH_SRL, MASK_SRL, match_opcode, 0 }, -{"srl", 0, INSN_CLASS_I, "d,s,>", MATCH_SRLI, MASK_SRLI, match_opcode, INSN_ALIAS }, +{"srl", 0, INSN_CLASS_I, "d,s,>", MATCH_SRLI, MASK_SRLI, match_shamt, INSN_ALIAS }, {"srai", 0, INSN_CLASS_C, "Cs,Cw,C>", MATCH_C_SRAI, MASK_C_SRAI, match_srxi_as_c_srxi, INSN_ALIAS }, -{"srai", 0, INSN_CLASS_I, "d,s,>", MATCH_SRAI, MASK_SRAI, match_opcode, 0 }, +{"srai", 0, INSN_CLASS_I, "d,s,>", MATCH_SRAI, MASK_SRAI, match_shamt, 0 }, {"sra", 0, INSN_CLASS_C, "Cs,Cw,C>", MATCH_C_SRAI, MASK_C_SRAI, match_srxi_as_c_srxi, INSN_ALIAS }, {"sra", 0, INSN_CLASS_I, "d,s,t", MATCH_SRA, MASK_SRA, match_opcode, 0 }, -{"sra", 0, INSN_CLASS_I, "d,s,>", MATCH_SRAI, MASK_SRAI, match_opcode, INSN_ALIAS }, +{"sra", 0, INSN_CLASS_I, "d,s,>", MATCH_SRAI, MASK_SRAI, match_shamt, INSN_ALIAS }, {"sub", 0, INSN_CLASS_C, "Cs,Cw,Ct", MATCH_C_SUB, MASK_C_SUB, match_opcode, INSN_ALIAS }, {"sub", 0, INSN_CLASS_I, "d,s,t", MATCH_SUB, MASK_SUB, match_opcode, 0 }, {"lb", 0, INSN_CLASS_I, "d,o(s)", MATCH_LB, MASK_LB, match_opcode, INSN_DREF|INSN_1_BYTE }, @@ -963,9 +975,9 @@ const struct riscv_opcode riscv_opcodes[] = {"orn", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,t", MATCH_ORN, MASK_ORN, match_opcode, 0 }, {"xnor", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,t", MATCH_XNOR, MASK_XNOR, match_opcode, 0 }, {"rol", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,t", MATCH_ROL, MASK_ROL, match_opcode, 0 }, -{"rori", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,>", MATCH_RORI, MASK_RORI, match_opcode, 0 }, +{"rori", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,>", MATCH_RORI, MASK_RORI, match_shamt, 0 }, {"ror", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,t", MATCH_ROR, MASK_ROR, match_opcode, 0 }, -{"ror", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,>", MATCH_RORI, MASK_RORI, match_opcode, INSN_ALIAS }, +{"ror", 0, INSN_CLASS_ZBB_OR_ZBKB, "d,s,>", MATCH_RORI, MASK_RORI, match_shamt, INSN_ALIAS }, {"rev8", 32, INSN_CLASS_ZBB_OR_ZBKB, "d,s", MATCH_GREVI | MATCH_SHAMT_REV8_32, MASK_GREVI | MASK_SHAMT, match_opcode, 0 }, {"rev8", 64, INSN_CLASS_ZBB_OR_ZBKB, "d,s", MATCH_GREVI | MATCH_SHAMT_REV8_64, MASK_GREVI | MASK_SHAMT, match_opcode, 0 }, {"rolw", 64, INSN_CLASS_ZBB_OR_ZBKB, "d,s,t", MATCH_ROLW, MASK_ROLW, match_opcode, 0 }, @@ -983,7 +995,7 @@ const struct riscv_opcode riscv_opcodes[] = {"zext.w", 64, INSN_CLASS_ZBA, "d,s", MATCH_ADD_UW, MASK_ADD_UW | MASK_RS2, match_opcode, INSN_ALIAS }, {"zext.w", 64, INSN_CLASS_I, "d,s", 0, (int) M_ZEXTW, match_never, INSN_MACRO }, {"add.uw", 64, INSN_CLASS_ZBA, "d,s,t", MATCH_ADD_UW, MASK_ADD_UW, match_opcode, 0 }, -{"slli.uw", 64, INSN_CLASS_ZBA, "d,s,>", MATCH_SLLI_UW, MASK_SLLI_UW, match_opcode, 0 }, +{"slli.uw", 64, INSN_CLASS_ZBA, "d,s,>", MATCH_SLLI_UW, MASK_SLLI_UW, match_shamt, 0 }, /* Zbc or zbkc instructions. */ {"clmul", 0, INSN_CLASS_ZBC_OR_ZBKC, "d,s,t", MATCH_CLMUL, MASK_CLMUL, match_opcode, 0 }, @@ -991,18 +1003,18 @@ const struct riscv_opcode riscv_opcodes[] = {"clmulr", 0, INSN_CLASS_ZBC, "d,s,t", MATCH_CLMULR, MASK_CLMULR, match_opcode, 0 }, /* Zbs instructions. */ -{"bclri", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BCLRI, MASK_BCLRI, match_opcode, 0 }, -{"bseti", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BSETI, MASK_BSETI, match_opcode, 0 }, -{"binvi", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BINVI, MASK_BINVI, match_opcode, 0 }, -{"bexti", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BEXTI, MASK_BEXTI, match_opcode, 0 }, +{"bclri", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BCLRI, MASK_BCLRI, match_shamt, 0 }, +{"bseti", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BSETI, MASK_BSETI, match_shamt, 0 }, +{"binvi", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BINVI, MASK_BINVI, match_shamt, 0 }, +{"bexti", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BEXTI, MASK_BEXTI, match_shamt, 0 }, {"bclr", 0, INSN_CLASS_ZBS, "d,s,t", MATCH_BCLR, MASK_BCLR, match_opcode, 0 }, -{"bclr", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BCLRI, MASK_BCLRI, match_opcode, INSN_ALIAS }, +{"bclr", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BCLRI, MASK_BCLRI, match_shamt, INSN_ALIAS }, {"bset", 0, INSN_CLASS_ZBS, "d,s,t", MATCH_BSET, MASK_BSET, match_opcode, 0 }, -{"bset", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BSETI, MASK_BSETI, match_opcode, INSN_ALIAS }, +{"bset", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BSETI, MASK_BSETI, match_shamt, INSN_ALIAS }, {"binv", 0, INSN_CLASS_ZBS, "d,s,t", MATCH_BINV, MASK_BINV, match_opcode, 0 }, -{"binv", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BINVI, MASK_BINVI, match_opcode, INSN_ALIAS }, +{"binv", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BINVI, MASK_BINVI, match_shamt, INSN_ALIAS }, {"bext", 0, INSN_CLASS_ZBS, "d,s,t", MATCH_BEXT, MASK_BEXT, match_opcode, 0 }, -{"bext", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BEXTI, MASK_BEXTI, match_opcode, INSN_ALIAS }, +{"bext", 0, INSN_CLASS_ZBS, "d,s,>", MATCH_BEXTI, MASK_BEXTI, match_shamt, INSN_ALIAS }, /* Zbkx instructions. */ {"xperm4", 0, INSN_CLASS_ZBKX, "d,s,t", MATCH_XPERM4, MASK_XPERM4, match_opcode, 0 }, -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH 3/3] RISC-V: Add disassembler tests for shift amount 2022-05-23 10:06 [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 1/3] RISC-V: Add xlen to match_func Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 2/3] RISC-V: Check shift amount against XLEN Tsukasa OI @ 2022-05-23 10:06 ` Tsukasa OI 2022-07-30 3:47 ` [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI 2022-07-30 3:51 ` [PATCH 0/1] " Tsukasa OI 4 siblings, 0 replies; 7+ messages in thread From: Tsukasa OI @ 2022-05-23 10:06 UTC (permalink / raw) To: Tsukasa OI, Palmer Dabbelt, Nelson Chu, Kito Cheng; +Cc: binutils This commit adds tests for shift instruction to make sure that the disassembler detects invalid shift amount. gas/ChangeLog: * testsuite/gas/riscv/shamt-dis.s: New disassembler test. * testsuite/gas/riscv/shamt-dis-32.d: Likewise. * testsuite/gas/riscv/shamt-dis-64.d: Likewise. --- gas/testsuite/gas/riscv/shamt-dis-32.d | 34 +++++++++++++++++++ gas/testsuite/gas/riscv/shamt-dis-64.d | 34 +++++++++++++++++++ gas/testsuite/gas/riscv/shamt-dis.s | 45 ++++++++++++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 gas/testsuite/gas/riscv/shamt-dis-32.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis-64.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis.s diff --git a/gas/testsuite/gas/riscv/shamt-dis-32.d b/gas/testsuite/gas/riscv/shamt-dis-32.d new file mode 100644 index 00000000000..b160353f297 --- /dev/null +++ b/gas/testsuite/gas/riscv/shamt-dis-32.d @@ -0,0 +1,34 @@ +#as: -march=rv32ic_zba_zbb_zbs +#source: shamt-dis.s +#objdump: -d -M no-aliases + +.*:[ ]+file format .* + + +Disassembly of section .text: + +0+000 <target>: +[ ]+[0-9a-f]+:[ ]+01f59513[ ]+slli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+02059513[ ]+\.4byte[ ]+0x2059513 +[ ]+[0-9a-f]+:[ ]+01f5d513[ ]+srli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+0205d513[ ]+\.4byte[ ]+0x205d513 +[ ]+[0-9a-f]+:[ ]+41f5d513[ ]+srai[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4205d513[ ]+\.4byte[ ]+0x4205d513 +[ ]+[0-9a-f]+:[ ]+057e[ ]+c\.slli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+1502[ ]+\.2byte[ ]+0x1502 +[ ]+[0-9a-f]+:[ ]+817d[ ]+c\.srli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9101[ ]+\.2byte[ ]+0x9101 +[ ]+[0-9a-f]+:[ ]+857d[ ]+c\.srai[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9501[ ]+\.2byte[ ]+0x9501 +[ ]+[0-9a-f]+:[ ]+61f5d513[ ]+rori[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6205d513[ ]+\.4byte[ ]+0x6205d513 +[ ]+[0-9a-f]+:[ ]+09f5951b[ ]+\.4byte[ ]+0x9f5951b +[ ]+[0-9a-f]+:[ ]+0a05951b[ ]+\.4byte[ ]+0xa05951b +[ ]+[0-9a-f]+:[ ]+49f59513[ ]+bclri[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a059513[ ]+\.4byte[ ]+0x4a059513 +[ ]+[0-9a-f]+:[ ]+29f59513[ ]+bseti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+2a059513[ ]+\.4byte[ ]+0x2a059513 +[ ]+[0-9a-f]+:[ ]+69f59513[ ]+binvi[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6a059513[ ]+\.4byte[ ]+0x6a059513 +[ ]+[0-9a-f]+:[ ]+49f5d513[ ]+bexti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a05d513[ ]+\.4byte[ ]+0x4a05d513 diff --git a/gas/testsuite/gas/riscv/shamt-dis-64.d b/gas/testsuite/gas/riscv/shamt-dis-64.d new file mode 100644 index 00000000000..d10e90d9d30 --- /dev/null +++ b/gas/testsuite/gas/riscv/shamt-dis-64.d @@ -0,0 +1,34 @@ +#as: -march=rv64ic_zba_zbb_zbs +#source: shamt-dis.s +#objdump: -d -M no-aliases + +.*:[ ]+file format .* + + +Disassembly of section .text: + +0+000 <target>: +[ ]+[0-9a-f]+:[ ]+01f59513[ ]+slli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+02059513[ ]+slli[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+01f5d513[ ]+srli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+0205d513[ ]+srli[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+41f5d513[ ]+srai[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4205d513[ ]+srai[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+057e[ ]+c\.slli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+1502[ ]+c\.slli[ ]+a0,0x20 +[ ]+[0-9a-f]+:[ ]+817d[ ]+c\.srli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9101[ ]+c\.srli[ ]+a0,0x20 +[ ]+[0-9a-f]+:[ ]+857d[ ]+c\.srai[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9501[ ]+c\.srai[ ]+a0,0x20 +[ ]+[0-9a-f]+:[ ]+61f5d513[ ]+rori[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6205d513[ ]+rori[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+09f5951b[ ]+slli\.uw[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+0a05951b[ ]+slli\.uw[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+49f59513[ ]+bclri[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a059513[ ]+bclri[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+29f59513[ ]+bseti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+2a059513[ ]+bseti[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+69f59513[ ]+binvi[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6a059513[ ]+binvi[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+49f5d513[ ]+bexti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a05d513[ ]+bexti[ ]+a0,a1,0x20 diff --git a/gas/testsuite/gas/riscv/shamt-dis.s b/gas/testsuite/gas/riscv/shamt-dis.s new file mode 100644 index 00000000000..edade302562 --- /dev/null +++ b/gas/testsuite/gas/riscv/shamt-dis.s @@ -0,0 +1,45 @@ +target: + # slli a0,a1,SHAMT [31/32] + .insn 0x01f59513 + .insn 0x02059513 + # srli a0,a1,SHAMT [31/32] + .insn 0x01f5d513 + .insn 0x0205d513 + # srai a0,a1,SHAMT [31/32] + .insn 0x41f5d513 + .insn 0x4205d513 + + # RVC + # c.slli a0,SHAMT [31/32] + .insn 0x057e + .insn 0x1502 + # c.srli a0,SHAMT [31/32] + .insn 0x817d + .insn 0x9101 + # c.srai a0,SHAMT [31/32] + .insn 0x857d + .insn 0x9501 + + # Zbb extension (or Zbkb) + # rori a0,a1,SHAMT [31/32] + .insn 0x61f5d513 + .insn 0x6205d513 + + # Zba extension + # slli.uw a0,a1,SHAMT [31/32] (invalid on RV32) + .insn 0x09f5951b + .insn 0x0a05951b + + # Zbs extension + # bclri a0,a1,SHAMT [31/32] + .insn 0x49f59513 + .insn 0x4a059513 + # bseti a0,a1,SHAMT [31/32] + .insn 0x29f59513 + .insn 0x2a059513 + # binvi a0,a1,SHAMT [31/32] + .insn 0x69f59513 + .insn 0x6a059513 + # bexti a0,a1,SHAMT [31/32] + .insn 0x49f5d513 + .insn 0x4a05d513 -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) 2022-05-23 10:06 [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI ` (2 preceding siblings ...) 2022-05-23 10:06 ` [RFC PATCH 3/3] RISC-V: Add disassembler tests for shift amount Tsukasa OI @ 2022-07-30 3:47 ` Tsukasa OI 2022-07-30 3:51 ` [PATCH 0/1] " Tsukasa OI 4 siblings, 0 replies; 7+ messages in thread From: Tsukasa OI @ 2022-07-30 3:47 UTC (permalink / raw) To: Palmer Dabbelt, Nelson Chu, Kito Cheng; +Cc: binutils This patchset is officially withdrawn as I will submit new simpler patchset for the exact same issue. Thanks, Tsukasa On 2022/05/23 19:06, Tsukasa OI wrote: > ** Note ** > This patch is not compatible with my previous Zfinx fixes. Actual PATCH > v1 is going to be submitted after this patchset is reviewed and Zfinx > fixes (relatively high priority) are applied. > > Certain shift instructions have a constraint: shift amount must be less > than current XLEN. This constraint is checked on assembler and > simulator, but not on disassembler. It causes GDB to print wrong (and > invalid) instructions and can be a problem while ... for instance, > checking for invalid build configuration. > > We have several methods to deal with it: > > - Add `xlen' argument to match_func to test XLEN > - Split shift instructions to per-XLEN variants > > Because shift instructions are so basic and XLEN-checking is so simple, > I chose the former. The latter is used on my Zfinx fixes but this is > because register pair checking involves relatively complex per-operand > constraints. Complex solution for simple problem can be a new problem. > > But... > > - Is it okay to add an argument anyway? > - Which is better extra argument? > 1. unsigned xlen > 2. const riscv_parse_subset_t * subset > which contains ISA version, XLEN and extensions > ... but a part of BFD, not opcodes. > > I also (slightly) changed the assembler to suppress extra error message > just like I did on Zicbop instructions: > <https://sourceware.org/pipermail/binutils/2021-December/118910.html> > <https://sourceware.org/pipermail/binutils/2021-December/118938.html> > > > > > Tsukasa OI (3): > RISC-V: Add xlen to match_func > RISC-V: Check shift amount against XLEN > RISC-V: Add disassembler tests for shift amount > > gas/config/tc-riscv.c | 8 +- > gas/testsuite/gas/riscv/shamt-dis-32.d | 34 +++++++ > gas/testsuite/gas/riscv/shamt-dis-64.d | 34 +++++++ > gas/testsuite/gas/riscv/shamt-dis.s | 45 +++++++++ > include/opcode/riscv.h | 3 +- > opcodes/riscv-dis.c | 2 +- > opcodes/riscv-opc.c | 122 +++++++++++++++---------- > sim/riscv/sim-main.c | 2 +- > 8 files changed, 195 insertions(+), 55 deletions(-) > create mode 100644 gas/testsuite/gas/riscv/shamt-dis-32.d > create mode 100644 gas/testsuite/gas/riscv/shamt-dis-64.d > create mode 100644 gas/testsuite/gas/riscv/shamt-dis.s > > > base-commit: cb0d58bf4d274cfb1ae11b75bd2b3ba81c8d371d ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/1] RISC-V: Check shift amount against XLEN (disassembler) 2022-05-23 10:06 [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI ` (3 preceding siblings ...) 2022-07-30 3:47 ` [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI @ 2022-07-30 3:51 ` Tsukasa OI 2022-07-30 3:51 ` [PATCH 1/1] RISC-V: Check shift amount against XLEN (disasm) Tsukasa OI 4 siblings, 1 reply; 7+ messages in thread From: Tsukasa OI @ 2022-07-30 3:51 UTC (permalink / raw) To: Tsukasa OI, Nelson Chu, Palmer Dabbelt, Kito Cheng; +Cc: binutils Hello, This patchset is going to support shift amount checking on the disassembler. Previous Patchset: <https://sourceware.org/pipermail/binutils/2022-May/120956.html> <https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_shift_xlen> Tracker on GitHub: <https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_shift_xlen_2> In the disassembler, there's no validation whether shift amount is valid on the specific architecture (RV32/64). My previous Idea 1 patchset tried to fix that by adding xlen argument to every match_func (in opcode entries). However, it requires bigger core disassembler changes and maintaining is relatively hard. Idea 2 patchset (this patchset) fixes that by printing invalid0x[SHAMT] instead of actual shift width (SHAMT in hexadecimal like regular shift amounts). If we don't have to print "unrecognized instruction" (such as .4byte) on instrucitons with invalid shift amounts, this patch would be the simplest solution for the invalid shift amount checking support on the disassembler. This patchset supports styling. invalid shift amounts are printed as text, not immediate. It makes invalid shift amounts default white opposed to blue on valid ones. See an example with the GitHub Wiki page linked above. Thanks, Tsukasa Tsukasa OI (1): RISC-V: Check shift amount against XLEN (disasm) gas/testsuite/gas/riscv/shamt-dis-32.d | 34 +++++++++++++++++++ gas/testsuite/gas/riscv/shamt-dis-64.d | 34 +++++++++++++++++++ gas/testsuite/gas/riscv/shamt-dis.s | 47 ++++++++++++++++++++++++++ opcodes/riscv-dis.c | 16 ++++++--- 4 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 gas/testsuite/gas/riscv/shamt-dis-32.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis-64.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis.s base-commit: b245c595aaa59812f8f3a0e8b70ea5f52e045627 -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/1] RISC-V: Check shift amount against XLEN (disasm) 2022-07-30 3:51 ` [PATCH 0/1] " Tsukasa OI @ 2022-07-30 3:51 ` Tsukasa OI 0 siblings, 0 replies; 7+ messages in thread From: Tsukasa OI @ 2022-07-30 3:51 UTC (permalink / raw) To: Tsukasa OI, Nelson Chu, Palmer Dabbelt, Kito Cheng; +Cc: binutils Although the assembler does the equivalent, the disassembler did not have shift amount checking. Instead of actually rejecting invalid shift amount, this commit prints invalid shift amount as "invalid0x[SHAMT]". It also changes the style so that style-aware disassembler can distinguish regular shift amount (immediate) and invalid one. gas/ChangeLog: * testsuite/gas/riscv/shamt-dis.s: New disassembler test. * testsuite/gas/riscv/shamt-dis-32.d: Likewise. * testsuite/gas/riscv/shamt-dis-64.d: Likewise. opcodes/ChangeLog: * riscv-dis.c (print_insn_args): Print invalid shift amount as "invalid0x[SHAMT]" where SHAMT is represented in hexadecimal. --- gas/testsuite/gas/riscv/shamt-dis-32.d | 34 +++++++++++++++++++ gas/testsuite/gas/riscv/shamt-dis-64.d | 34 +++++++++++++++++++ gas/testsuite/gas/riscv/shamt-dis.s | 47 ++++++++++++++++++++++++++ opcodes/riscv-dis.c | 16 ++++++--- 4 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 gas/testsuite/gas/riscv/shamt-dis-32.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis-64.d create mode 100644 gas/testsuite/gas/riscv/shamt-dis.s diff --git a/gas/testsuite/gas/riscv/shamt-dis-32.d b/gas/testsuite/gas/riscv/shamt-dis-32.d new file mode 100644 index 00000000000..85a247f7e25 --- /dev/null +++ b/gas/testsuite/gas/riscv/shamt-dis-32.d @@ -0,0 +1,34 @@ +#as: -march=rv32ic_zba_zbb_zbs +#source: shamt-dis.s +#objdump: -d -M no-aliases + +.*:[ ]+file format .* + + +Disassembly of section .text: + +0+000 <target>: +[ ]+[0-9a-f]+:[ ]+01f59513[ ]+slli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+02059513[ ]+slli[ ]+a0,a1,invalid0x20 +[ ]+[0-9a-f]+:[ ]+01f5d513[ ]+srli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+0205d513[ ]+srli[ ]+a0,a1,invalid0x20 +[ ]+[0-9a-f]+:[ ]+41f5d513[ ]+srai[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4205d513[ ]+srai[ ]+a0,a1,invalid0x20 +[ ]+[0-9a-f]+:[ ]+057e[ ]+c\.slli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+1502[ ]+c\.slli[ ]+a0,invalid0x20 +[ ]+[0-9a-f]+:[ ]+817d[ ]+c\.srli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9101[ ]+c\.srli[ ]+a0,invalid0x20 +[ ]+[0-9a-f]+:[ ]+857d[ ]+c\.srai[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9501[ ]+c\.srai[ ]+a0,invalid0x20 +[ ]+[0-9a-f]+:[ ]+61f5d513[ ]+rori[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6205d513[ ]+rori[ ]+a0,a1,invalid0x20 +[ ]+[0-9a-f]+:[ ]+09f5951b[ ]+\.4byte[ ]+0x9f5951b +[ ]+[0-9a-f]+:[ ]+0a05951b[ ]+\.4byte[ ]+0xa05951b +[ ]+[0-9a-f]+:[ ]+49f59513[ ]+bclri[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a059513[ ]+bclri[ ]+a0,a1,invalid0x20 +[ ]+[0-9a-f]+:[ ]+29f59513[ ]+bseti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+2a059513[ ]+bseti[ ]+a0,a1,invalid0x20 +[ ]+[0-9a-f]+:[ ]+69f59513[ ]+binvi[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6a059513[ ]+binvi[ ]+a0,a1,invalid0x20 +[ ]+[0-9a-f]+:[ ]+49f5d513[ ]+bexti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a05d513[ ]+bexti[ ]+a0,a1,invalid0x20 diff --git a/gas/testsuite/gas/riscv/shamt-dis-64.d b/gas/testsuite/gas/riscv/shamt-dis-64.d new file mode 100644 index 00000000000..d10e90d9d30 --- /dev/null +++ b/gas/testsuite/gas/riscv/shamt-dis-64.d @@ -0,0 +1,34 @@ +#as: -march=rv64ic_zba_zbb_zbs +#source: shamt-dis.s +#objdump: -d -M no-aliases + +.*:[ ]+file format .* + + +Disassembly of section .text: + +0+000 <target>: +[ ]+[0-9a-f]+:[ ]+01f59513[ ]+slli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+02059513[ ]+slli[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+01f5d513[ ]+srli[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+0205d513[ ]+srli[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+41f5d513[ ]+srai[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4205d513[ ]+srai[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+057e[ ]+c\.slli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+1502[ ]+c\.slli[ ]+a0,0x20 +[ ]+[0-9a-f]+:[ ]+817d[ ]+c\.srli[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9101[ ]+c\.srli[ ]+a0,0x20 +[ ]+[0-9a-f]+:[ ]+857d[ ]+c\.srai[ ]+a0,0x1f +[ ]+[0-9a-f]+:[ ]+9501[ ]+c\.srai[ ]+a0,0x20 +[ ]+[0-9a-f]+:[ ]+61f5d513[ ]+rori[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6205d513[ ]+rori[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+09f5951b[ ]+slli\.uw[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+0a05951b[ ]+slli\.uw[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+49f59513[ ]+bclri[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a059513[ ]+bclri[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+29f59513[ ]+bseti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+2a059513[ ]+bseti[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+69f59513[ ]+binvi[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+6a059513[ ]+binvi[ ]+a0,a1,0x20 +[ ]+[0-9a-f]+:[ ]+49f5d513[ ]+bexti[ ]+a0,a1,0x1f +[ ]+[0-9a-f]+:[ ]+4a05d513[ ]+bexti[ ]+a0,a1,0x20 diff --git a/gas/testsuite/gas/riscv/shamt-dis.s b/gas/testsuite/gas/riscv/shamt-dis.s new file mode 100644 index 00000000000..a6e292242d6 --- /dev/null +++ b/gas/testsuite/gas/riscv/shamt-dis.s @@ -0,0 +1,47 @@ +target: + # slli a0,a1,SHAMT [31/32] + .insn i OP_IMM, 1, a0, a1, 0x000 | 31 + .insn i OP_IMM, 1, a0, a1, 0x000 | 32 + # srli a0,a1,SHAMT [31/32] + .insn i OP_IMM, 5, a0, a1, 0x000 | 31 + .insn i OP_IMM, 5, a0, a1, 0x000 | 32 + # srai a0,a1,SHAMT [31/32] + .insn i OP_IMM, 5, a0, a1, 0x400 | 31 + .insn i OP_IMM, 5, a0, a1, 0x400 | 32 + + # RVC + # c.slli a0,SHAMT [31/32] + .insn ci 2, 0, a0, 31 + .insn ci 2, 0, a0, 32 - 0x40 + # Although c.sr[la]i are CB-format instructions, + # encode them as CI-type for immediate consistency. + # c.srli a0,SHAMT [31/32] + .insn ci 1, 4, x2, 31 + .insn ci 1, 4, x2, 32 - 0x40 + # c.srai a0,SHAMT [31/32] + .insn ci 1, 4, x10, 31 + .insn ci 1, 4, x10, 32 - 0x40 + + # Zbb extension (or Zbkb) + # rori a0,a1,SHAMT [31/32] + .insn i OP_IMM, 5, a0, a1, 0x600 | 31 + .insn i OP_IMM, 5, a0, a1, 0x600 | 32 + + # Zba extension + # slli.uw a0,a1,SHAMT [31/32] (invalid on RV32) + .insn i OP_IMM_32, 1, a0, a1, 0x080 | 31 + .insn i OP_IMM_32, 1, a0, a1, 0x080 | 32 + + # Zbs extension + # bclri a0,a1,SHAMT [31/32] + .insn i OP_IMM, 1, a0, a1, 0x480 | 31 + .insn i OP_IMM, 1, a0, a1, 0x480 | 32 + # bseti a0,a1,SHAMT [31/32] + .insn i OP_IMM, 1, a0, a1, 0x280 | 31 + .insn i OP_IMM, 1, a0, a1, 0x280 | 32 + # binvi a0,a1,SHAMT [31/32] + .insn i OP_IMM, 1, a0, a1, 0x680 | 31 + .insn i OP_IMM, 1, a0, a1, 0x680 | 32 + # bexti a0,a1,SHAMT [31/32] + .insn i OP_IMM, 5, a0, a1, 0x480 | 31 + .insn i OP_IMM, 5, a0, a1, 0x480 | 32 diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c index 164fd209dbd..3e9519525d4 100644 --- a/opcodes/riscv-dis.c +++ b/opcodes/riscv-dis.c @@ -286,8 +286,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info (int)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1))); break; case '>': - print (info->stream, dis_style_immediate, "0x%x", - (int)EXTRACT_CITYPE_IMM (l) & 0x3f); + if (((unsigned)EXTRACT_CITYPE_IMM (l) & 0x3fU) >= xlen) + print (info->stream, dis_style_text, "invalid0x%x", + (unsigned)EXTRACT_CITYPE_IMM (l) & 0x3fU); + else + print (info->stream, dis_style_immediate, "0x%x", + (unsigned)EXTRACT_CITYPE_IMM (l) & 0x3fU); break; case '<': print (info->stream, dis_style_immediate, "0x%x", @@ -481,8 +485,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info break; case '>': - print (info->stream, dis_style_immediate, "0x%x", - (int)EXTRACT_OPERAND (SHAMT, l)); + if ((unsigned)EXTRACT_OPERAND (SHAMT, l) >= xlen) + print (info->stream, dis_style_text, "invalid0x%x", + (unsigned)EXTRACT_OPERAND (SHAMT, l)); + else + print (info->stream, dis_style_immediate, "0x%x", + (unsigned)EXTRACT_OPERAND (SHAMT, l)); break; case '<': -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-07-30 3:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-23 10:06 [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 1/3] RISC-V: Add xlen to match_func Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 2/3] RISC-V: Check shift amount against XLEN Tsukasa OI 2022-05-23 10:06 ` [RFC PATCH 3/3] RISC-V: Add disassembler tests for shift amount Tsukasa OI 2022-07-30 3:47 ` [RFC PATCH 0/3] RISC-V: Check shift amount against XLEN (disassembler) Tsukasa OI 2022-07-30 3:51 ` [PATCH 0/1] " Tsukasa OI 2022-07-30 3:51 ` [PATCH 1/1] RISC-V: Check shift amount against XLEN (disasm) Tsukasa OI
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).