From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Jim Wilson <jimw@sifive.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 2/5] RISC-V: Add software single step support.
Date: Wed, 08 Aug 2018 12:50:00 -0000 [thread overview]
Message-ID: <20180808125052.GM3155@embecosm.com> (raw)
In-Reply-To: <20180808021607.7652-1-jimw@sifive.com>
* Jim Wilson <jimw@sifive.com> [2018-08-07 19:16:07 -0700]:
> This adds software single step support that is needed by the linux native port.
> This is modeled after equivalent code in the MIPS port.
>
> This also fixes a few bugs in the compressed instruction decode support. Some
> instructions are RV32/RV64 specific, and this wasn't being checked. Also, a
> few instructions were accidentally using the non-compressed is_* function.
>
> This has been tested on a HiFive Unleashed running Fedora, by putting a
> breakpoint on start, typing stepi, and then holding down the return key until
> it finishes, and observing that I see everything I expect to see along the way.
> There is a problem in _dl_addr where I get into an infinite loop, but it seems
> to be some synchronization code that doesn't agree with single step, so I have
> to find the end of the loop, put a breakpoint there, continue, and then single
> step again until the end.
>
> gdb/
> * riscv-tdep.c (enum opcode): Add jump, branch, lr, and sc opcodes.
> (decode_register_index_short): New.
> (decode_j_type_insn, decode_cj_type_insn): New.
> (decode_b_type_insn, decode_cb_type_insn): New.
> (riscv_insn::decode): Add support for jumps, branches, lr, and sc. New
> local xlen. Check xlen when decoding ambiguous compressed insns. In
> compressed decode, use is_c_lui_insn instead of is_lui_insn, and
> is_c_sw_insn instead of is_sw_insn.
> (riscv_next_pc, riscv_next_pc_atomic_sequence): New.
> (riscv_software_single_step): New.
> * riscv-tdep.h (riscv_software_single_step): Declare.
Looks good to me.
Thanks,
Andrew
> ---
> gdb/riscv-tdep.c | 250 +++++++++++++++++++++++++++++++++++++++++++++--
> gdb/riscv-tdep.h | 4 +
> 2 files changed, 248 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 20181896c5..1df1300100 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -880,6 +880,18 @@ public:
> LUI,
> SD,
> SW,
> + /* These are needed for software breakopint support. */
> + JAL,
> + JALR,
> + BEQ,
> + BNE,
> + BLT,
> + BGE,
> + BLTU,
> + BGEU,
> + /* These are needed for stepping over atomic sequences. */
> + LR,
> + SC,
>
> /* Other instructions are not interesting during the prologue scan, and
> are ignored. */
> @@ -936,6 +948,12 @@ private:
> return (opcode >> offset) & 0x1F;
> }
>
> + /* Extract 5 bit register field at OFFSET from instruction OPCODE. */
> + int decode_register_index_short (unsigned long opcode, int offset)
> + {
> + return ((opcode >> offset) & 0x7) + 8;
> + }
> +
> /* Helper for DECODE, decode 32-bit R-type instruction. */
> void decode_r_type_insn (enum opcode opcode, ULONGEST ival)
> {
> @@ -987,6 +1005,36 @@ private:
> m_imm.s = EXTRACT_UTYPE_IMM (ival);
> }
>
> + /* Helper for DECODE, decode 32-bit J-type instruction. */
> + void decode_j_type_insn (enum opcode opcode, ULONGEST ival)
> + {
> + m_opcode = opcode;
> + m_rd = decode_register_index (ival, OP_SH_RD);
> + m_imm.s = EXTRACT_UJTYPE_IMM (ival);
> + }
> +
> + /* Helper for DECODE, decode 32-bit J-type instruction. */
> + void decode_cj_type_insn (enum opcode opcode, ULONGEST ival)
> + {
> + m_opcode = opcode;
> + m_imm.s = EXTRACT_RVC_J_IMM (ival);
> + }
> +
> + void decode_b_type_insn (enum opcode opcode, ULONGEST ival)
> + {
> + m_opcode = opcode;
> + m_rs1 = decode_register_index (ival, OP_SH_RS1);
> + m_rs2 = decode_register_index (ival, OP_SH_RS2);
> + m_imm.s = EXTRACT_SBTYPE_IMM (ival);
> + }
> +
> + void decode_cb_type_insn (enum opcode opcode, ULONGEST ival)
> + {
> + m_opcode = opcode;
> + m_rs1 = decode_register_index_short (ival, OP_SH_CRS1S);
> + m_imm.s = EXTRACT_RVC_B_IMM (ival);
> + }
> +
> /* Fetch instruction from target memory at ADDR, return the content of
> the instruction, and update LEN with the instruction length. */
> static ULONGEST fetch_instruction (struct gdbarch *gdbarch,
> @@ -1083,32 +1131,83 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
> decode_s_type_insn (SD, ival);
> else if (is_sw_insn (ival))
> decode_s_type_insn (SW, ival);
> + else if (is_jal_insn (ival))
> + decode_j_type_insn (JAL, ival);
> + else if (is_jalr_insn (ival))
> + decode_i_type_insn (JALR, ival);
> + else if (is_beq_insn (ival))
> + decode_b_type_insn (BEQ, ival);
> + else if (is_bne_insn (ival))
> + decode_b_type_insn (BNE, ival);
> + else if (is_blt_insn (ival))
> + decode_b_type_insn (BLT, ival);
> + else if (is_bge_insn (ival))
> + decode_b_type_insn (BGE, ival);
> + else if (is_bltu_insn (ival))
> + decode_b_type_insn (BLTU, ival);
> + else if (is_bgeu_insn (ival))
> + decode_b_type_insn (BGEU, ival);
> + else if (is_lr_w_insn (ival))
> + decode_r_type_insn (LR, ival);
> + else if (is_lr_d_insn (ival))
> + decode_r_type_insn (LR, ival);
> + else if (is_sc_w_insn (ival))
> + decode_r_type_insn (SC, ival);
> + else if (is_sc_d_insn (ival))
> + decode_r_type_insn (SC, ival);
> else
> /* None of the other fields are valid in this case. */
> m_opcode = OTHER;
> }
> else if (m_length == 2)
> {
> - if (is_c_add_insn (ival))
> + int xlen = riscv_isa_xlen (gdbarch);
> +
> + /* C_ADD and C_JALR have the same opcode. If RS2 is 0, then this is a
> + C_JALR. So must try to match C_JALR first as it has more bits in
> + mask. */
> + if (is_c_jalr_insn (ival))
> + decode_cr_type_insn (JALR, ival);
> + else if (is_c_add_insn (ival))
> decode_cr_type_insn (ADD, ival);
> - else if (is_c_addw_insn (ival))
> + /* C_ADDW is RV64 and RV128 only. */
> + else if (xlen != 4 && is_c_addw_insn (ival))
> decode_cr_type_insn (ADDW, ival);
> else if (is_c_addi_insn (ival))
> decode_ci_type_insn (ADDI, ival);
> - else if (is_c_addiw_insn (ival))
> + /* C_ADDIW and C_JAL have the same opcode. C_ADDIW is RV64 and RV128
> + only and C_JAL is RV32 only. */
> + else if (xlen != 4 && is_c_addiw_insn (ival))
> decode_ci_type_insn (ADDIW, ival);
> + else if (xlen == 4 && is_c_jal_insn (ival))
> + decode_cj_type_insn (JAL, ival);
> + /* C_ADDI16SP and C_LUI have the same opcode. If RD is 2, then this is a
> + C_ADDI16SP. So must try to match C_ADDI16SP first as it has more bits
> + in mask. */
> else if (is_c_addi16sp_insn (ival))
> {
> m_opcode = ADDI;
> m_rd = m_rs1 = decode_register_index (ival, OP_SH_RD);
> m_imm.s = EXTRACT_RVC_ADDI16SP_IMM (ival);
> }
> - else if (is_lui_insn (ival))
> + else if (is_c_lui_insn (ival))
> m_opcode = OTHER;
> - else if (is_c_sd_insn (ival))
> + /* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only,
> + and C_FSW is RV32 only. */
> + else if (xlen != 4 && is_c_sd_insn (ival))
> m_opcode = OTHER;
> - else if (is_sw_insn (ival))
> + else if (is_c_sw_insn (ival))
> m_opcode = OTHER;
> + /* C_JR and C_MV have the same opcode. If RS2 is 0, then this is a C_JR.
> + So must try to match C_JR first as it ahs more bits in mask. */
> + else if (is_c_jr_insn (ival))
> + decode_cr_type_insn (JALR, ival);
> + else if (is_c_j_insn (ival))
> + decode_cj_type_insn (JAL, ival);
> + else if (is_c_beqz_insn (ival))
> + decode_cb_type_insn (BEQ, ival);
> + else if (is_c_bnez_insn (ival))
> + decode_cb_type_insn (BNE, ival);
> else
> /* None of the other fields of INSN are valid in this case. */
> m_opcode = OTHER;
> @@ -2610,6 +2709,145 @@ riscv_invalidate_inferior_data (struct inferior *inf)
> }
> }
>
> +/* This decodes the current instruction and determines the address of the
> + next instruction. */
> +
> +static CORE_ADDR
> +riscv_next_pc (struct regcache *regcache, CORE_ADDR pc)
> +{
> + struct gdbarch *gdbarch = regcache->arch ();
> + struct riscv_insn insn;
> + CORE_ADDR next_pc;
> +
> + insn.decode (gdbarch, pc);
> + next_pc = pc + insn.length ();
> +
> + if (insn.opcode () == riscv_insn::JAL)
> + next_pc = pc + insn.imm_signed ();
> + else if (insn.opcode () == riscv_insn::JALR)
> + {
> + LONGEST source;
> + regcache->cooked_read (insn.rs1 (), &source);
> + next_pc = (source + insn.imm_signed ()) & ~(CORE_ADDR) 0x1;
> + }
> + else if (insn.opcode () == riscv_insn::BEQ)
> + {
> + LONGEST src1, src2;
> + regcache->cooked_read (insn.rs1 (), &src1);
> + regcache->cooked_read (insn.rs2 (), &src2);
> + if (src1 == src2)
> + next_pc = pc + insn.imm_signed ();
> + }
> + else if (insn.opcode () == riscv_insn::BNE)
> + {
> + LONGEST src1, src2;
> + regcache->cooked_read (insn.rs1 (), &src1);
> + regcache->cooked_read (insn.rs2 (), &src2);
> + if (src1 != src2)
> + next_pc = pc + insn.imm_signed ();
> + }
> + else if (insn.opcode () == riscv_insn::BLT)
> + {
> + LONGEST src1, src2;
> + regcache->cooked_read (insn.rs1 (), &src1);
> + regcache->cooked_read (insn.rs2 (), &src2);
> + if (src1 < src2)
> + next_pc = pc + insn.imm_signed ();
> + }
> + else if (insn.opcode () == riscv_insn::BGE)
> + {
> + LONGEST src1, src2;
> + regcache->cooked_read (insn.rs1 (), &src1);
> + regcache->cooked_read (insn.rs2 (), &src2);
> + if (src1 >= src2)
> + next_pc = pc + insn.imm_signed ();
> + }
> + else if (insn.opcode () == riscv_insn::BLTU)
> + {
> + ULONGEST src1, src2;
> + regcache->cooked_read (insn.rs1 (), &src1);
> + regcache->cooked_read (insn.rs2 (), &src2);
> + if (src1 < src2)
> + next_pc = pc + insn.imm_signed ();
> + }
> + else if (insn.opcode () == riscv_insn::BGEU)
> + {
> + ULONGEST src1, src2;
> + regcache->cooked_read (insn.rs1 (), &src1);
> + regcache->cooked_read (insn.rs2 (), &src2);
> + if (src1 >= src2)
> + next_pc = pc + insn.imm_signed ();
> + }
> +
> + return next_pc;
> +}
> +
> +/* We can't put a breakpoint in the middle of a lr/sc atomic sequence, so look
> + for the end of the sequence and put the breakpoint there. */
> +
> +static bool
> +riscv_next_pc_atomic_sequence (struct regcache *regcache, CORE_ADDR pc,
> + CORE_ADDR *next_pc)
> +{
> + struct gdbarch *gdbarch = regcache->arch ();
> + struct riscv_insn insn;
> + CORE_ADDR cur_step_pc = pc;
> + CORE_ADDR last_addr = 0;
> +
> + /* First instruction has to be a load reserved. */
> + insn.decode (gdbarch, cur_step_pc);
> + if (insn.opcode () != riscv_insn::LR)
> + return false;
> + cur_step_pc = cur_step_pc + insn.length ();
> +
> + /* Next instruction should be branch to exit. */
> + insn.decode (gdbarch, cur_step_pc);
> + if (insn.opcode () != riscv_insn::BNE)
> + return false;
> + last_addr = cur_step_pc + insn.imm_signed ();
> + cur_step_pc = cur_step_pc + insn.length ();
> +
> + /* Next instruction should be store conditional. */
> + insn.decode (gdbarch, cur_step_pc);
> + if (insn.opcode () != riscv_insn::SC)
> + return false;
> + cur_step_pc = cur_step_pc + insn.length ();
> +
> + /* Next instruction should be branch to start. */
> + insn.decode (gdbarch, cur_step_pc);
> + if (insn.opcode () != riscv_insn::BNE)
> + return false;
> + if (pc != (cur_step_pc + insn.imm_signed ()))
> + return false;
> + cur_step_pc = cur_step_pc + insn.length ();
> +
> + /* We should now be at the end of the sequence. */
> + if (cur_step_pc != last_addr)
> + return false;
> +
> + *next_pc = cur_step_pc;
> + return true;
> +}
> +
> +/* This is called just before we want to resume the inferior, if we want to
> + single-step it but there is no hardware or kernel single-step support. We
> + find the target of the coming instruction and breakpoint it. */
> +
> +std::vector<CORE_ADDR>
> +riscv_software_single_step (struct regcache *regcache)
> +{
> + CORE_ADDR pc, next_pc;
> +
> + pc = regcache_read_pc (regcache);
> +
> + if (riscv_next_pc_atomic_sequence (regcache, pc, &next_pc))
> + return {next_pc};
> +
> + next_pc = riscv_next_pc (regcache, pc);
> +
> + return {next_pc};
> +}
> +
> void
> _initialize_riscv_tdep (void)
> {
> diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
> index b35266daf7..8591116922 100644
> --- a/gdb/riscv-tdep.h
> +++ b/gdb/riscv-tdep.h
> @@ -79,4 +79,8 @@ struct gdbarch_tdep
> /* Return the width in bytes of the general purpose registers for GDBARCH. */
> extern int riscv_isa_xlen (struct gdbarch *gdbarch);
>
> +/* Single step based on where the current instruction will take us. */
> +extern std::vector<CORE_ADDR>
> +riscv_software_single_step (struct regcache *regcache);
> +
> #endif /* RISCV_TDEP_H */
> --
> 2.17.1
>
next prev parent reply other threads:[~2018-08-08 12:50 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-08 2:12 [PATCH 0/5] RISC-V Linux native port Jim Wilson
2018-08-08 2:15 ` [PATCH 1/5] RISC-V: Make riscv_isa_xlen a global function Jim Wilson
2018-08-08 12:42 ` Andrew Burgess
2018-08-08 17:55 ` Jim Wilson
2018-08-08 19:18 ` Simon Marchi
2018-08-08 2:16 ` [PATCH 2/5] RISC-V: Add software single step support Jim Wilson
2018-08-08 12:50 ` Andrew Burgess [this message]
2018-08-08 17:55 ` Jim Wilson
2018-08-08 2:16 ` [PATCH 3/5] RISC-V: Add linux target support Jim Wilson
2018-08-08 14:41 ` Andrew Burgess
2018-08-08 18:19 ` Jim Wilson
2018-08-08 18:35 ` Jim Wilson
2018-08-09 20:40 ` Jim Wilson
2018-08-08 2:17 ` [PATCH 5/5] RISC-V: Add configure support riscv*-linux* Jim Wilson
2018-08-08 16:00 ` Andrew Burgess
2018-08-08 17:30 ` Tom Tromey
2018-08-08 18:22 ` Eli Zaretskii
2018-08-08 20:49 ` Palmer Dabbelt
2018-08-08 23:26 ` Tom Tromey
2018-08-08 23:29 ` Tom Tromey
2018-08-09 2:36 ` Eli Zaretskii
2018-08-09 3:43 ` Jim Wilson
2018-08-09 4:55 ` Tom Tromey
2018-08-09 7:05 ` Andreas Schwab
2018-08-09 12:55 ` Eli Zaretskii
2018-08-09 17:25 ` Jim Wilson
2018-08-09 0:25 ` Jim Wilson
2018-08-09 0:29 ` [PATCH 5/5] RISC-V: Add configure support for riscv*-linux* Jim Wilson
2018-08-09 2:39 ` Eli Zaretskii
2018-08-09 15:57 ` Tom Tromey
2018-08-09 20:42 ` Jim Wilson
2018-08-08 2:17 ` [PATCH 4/5] RISC-V: Add native linux support Jim Wilson
2018-08-08 15:58 ` Andrew Burgess
2018-08-08 23:36 ` Jim Wilson
2018-08-08 23:39 ` Jim Wilson
2018-08-09 8:42 ` Andrew Burgess
2018-08-09 20:41 ` Jim Wilson
2018-10-25 10:49 ` Andreas Schwab
2018-10-25 11:09 ` Andrew Burgess
2018-10-25 12:06 ` Pedro Alves
2018-10-28 11:23 ` Andrew Burgess
2018-10-25 17:55 ` John Baldwin
2018-10-25 18:17 ` Jim Wilson
2018-10-25 19:19 ` John Baldwin
2018-10-27 6:07 ` Palmer Dabbelt
2018-10-29 8:50 ` Andreas Schwab
2018-10-25 16:40 ` Jim Wilson
2018-08-08 12:41 ` [PATCH 0/5] RISC-V Linux native port Andrew Burgess
2018-08-08 17:41 ` Jim Wilson
2018-08-08 18:16 ` Andrew Burgess
2018-08-08 18:42 ` Jim Wilson
2018-08-09 3:18 ` Palmer Dabbelt
2018-08-10 18:04 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180808125052.GM3155@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
--cc=jimw@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).