From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 51994 invoked by alias); 8 Aug 2018 12:50:59 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 51970 invoked by uid 89); 8 Aug 2018 12:50:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.5 required=5.0 tests=AWL,BAYES_00,DRUGS_ERECTILE,DRUGS_ERECTILE_OBFU,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wm0-f67.google.com Received: from mail-wm0-f67.google.com (HELO mail-wm0-f67.google.com) (74.125.82.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 08 Aug 2018 12:50:56 +0000 Received: by mail-wm0-f67.google.com with SMTP id y2-v6so2629938wma.1 for ; Wed, 08 Aug 2018 05:50:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=fr4P0dWWO/zP24NarrZFsvSh/318YlTNXsxPIPNf+Pc=; b=bv+kWaBZoKsBrET4a29Z0wYTiklL5gdwocgIlUKGSTjF+VxwyvKYAKFdVw8QBz8wVC +3bMW11xVDuawTBoD7+41c/xuJC6heRNkhm+tBIwxTXbawyjBlptgHc6GQZPE7yie8AB xbTK41aNs95OUEY8t0Iqaom73fo4MgCyj3CahIWN3/LwsmL1zmB/+sZDTUJ4IVptwE1Y Raaou+QaPHzYCOOCsHIipiIJQunqmLZLQkDlIptfbZuweYlwpAZF0UiKwzAxL/MkjYzM TSKspdV3I26A7j2w8mNEwRyCvDcT75RiLEcqucXQim7WJ/swu3sWCxNyOkRgb/eMJtVv 0TPA== Return-Path: Received: from localhost (host81-140-215-41.range81-140.btcentralplus.com. [81.140.215.41]) by smtp.gmail.com with ESMTPSA id e12-v6sm3867338wru.89.2018.08.08.05.50.53 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 08 Aug 2018 05:50:53 -0700 (PDT) Date: Wed, 08 Aug 2018 12:50:00 -0000 From: Andrew Burgess To: Jim Wilson Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/5] RISC-V: Add software single step support. Message-ID: <20180808125052.GM3155@embecosm.com> References: <20180808021607.7652-1-jimw@sifive.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180808021607.7652-1-jimw@sifive.com> X-Fortune: There's no such thing as pure pleasure User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00153.txt.bz2 * Jim Wilson [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 > +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 > +riscv_software_single_step (struct regcache *regcache); > + > #endif /* RISCV_TDEP_H */ > -- > 2.17.1 >