From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10980 invoked by alias); 7 Jul 2014 14:56:42 -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 10961 invoked by uid 89); 7 Jul 2014 14:56:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 07 Jul 2014 14:56:38 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id C32F21161C8; Mon, 7 Jul 2014 10:56:36 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id kCKGcIkdyPqG; Mon, 7 Jul 2014 10:56:36 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 59C711161AD; Mon, 7 Jul 2014 10:56:36 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id F30B440F62; Mon, 7 Jul 2014 07:56:34 -0700 (PDT) Date: Mon, 07 Jul 2014 14:56:00 -0000 From: Joel Brobecker To: Ajit Kumar Agarwal Cc: "gdb-patches@sourceware.org" , Michael Eager , Pedro Alves , Vinod Kathail , Vidhumouli Hunsigida , Nagaraju Mekala Subject: Re: [Patch, microblaze]: Add support of microblaze software single stepping Message-ID: <20140707145634.GF6038@adacore.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-07/txt/msg00115.txt.bz2 > PING! A quick look at the patch shows that the code does not conform to the GNU Coding style: - '{' brace at start of function should be on next line, - Sentences in comments should start with a capital letter and end with a period - etc. Also, GDB Coding Style, which is a superset of the GCS, require that every single new function have proper documentation a the start of them. You can refer to: https://sourceware.org/gdb/wiki/Internals%20Coding-Standards Last but not least, it looks like some lines got joined together, putting multiple statements on the same line. > > -----Original Message----- > From: Ajit Kumar Agarwal > Sent: Friday, June 20, 2014 11:40 AM > To: 'gdb-patches@sourceware.org' > Cc: 'Michael Eager'; 'Pedro Alves'; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala > Subject: RE: [Patch, microblaze]: Add support of microblaze software single stepping > > Please find the patch that supports the microblaze software single stepping. This patch handles the cases of branch and return with delay slot > and the imm instruction in microblaze. Could you please review and let me know if its okay. > > [Patch, microblaze]: Add support of microblaze software single stepping > > This patch adds the support of microblaze software single stepping. It > handles the cases of branch and return with delay slot and imm instruction > in microblaze. > > ChangeLog: > 2014-06-19 Ajit Agarwal > > * microblaze-tdep.c (microblaze_software_single_step): New. > (microblaze_gdbarch_init): Use of set_gdbarch_software_single_step. > > Signed-off-by:Ajit Agarwal ajitkum@xilinx.com > > --- > gdb/microblaze-tdep.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 79 insertions(+), 0 deletions(-) > > diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c index 14c1b52..3de2f70 100644 > --- a/gdb/microblaze-tdep.c > +++ b/gdb/microblaze-tdep.c > @@ -628,6 +628,83 @@ microblaze_stabs_argument_has_addr (struct gdbarch *gdbarch, struct type *type) > return (TYPE_LENGTH (type) == 16); > } > > +static int > +microblaze_software_single_step (struct frame_info *frame) { > + struct gdbarch *arch = get_frame_arch (frame); > + struct address_space *aspace = get_frame_address_space (frame); > + struct gdbarch_tdep *tdep = gdbarch_tdep (arch); > + enum bfd_endian byte_order = gdbarch_byte_order (arch); > + int ret = 0; > + int ii; > + CORE_ADDR pc; > + long insn; > + enum microblaze_instr minstr; > + bfd_boolean isunsignednum; > + enum microblaze_instr_type insn_type; > + short delay_slots; > + int imm; > + bfd_boolean immfound = FALSE; > + CORE_ADDR breaks[2] = {-1,-1}; > + CORE_ADDR address; > + int targetvalid; > + > + /* Set a breakpoint at the next instruction */ > + /* If the current instruction is an imm, set it at the inst after */ > + /* If the instruction has a delay slot, skip the delay slot */ pc = > + get_frame_pc (frame); insn = microblaze_fetch_instruction (pc); > + minstr = get_insn_microblaze (insn, &isunsignednum, &insn_type, > + &delay_slots); if (insn_type == immediate_inst) > + { > + int rd, ra, rb; > + immfound = TRUE; > + minstr = microblaze_decode_insn (insn, &rd, &ra, &rb, &imm); > + pc = pc + INST_WORD_SIZE; > + insn = microblaze_fetch_instruction (pc); > + minstr = get_insn_microblaze (insn, &isunsignednum, &insn_type, &delay_slots); > + } > + if (insn_type != return_inst) > + breaks[0] = pc + delay_slots * INST_WORD_SIZE + INST_WORD_SIZE; > + > + /* Now check for branch or return instructions */ if (insn_type == > + branch_inst || insn_type == return_inst) > + { > + int limm; > + int lrd, lra, lrb; > + int ra, rb; > + bfd_boolean targetvalid; > + bfd_boolean unconditionalbranch; > + microblaze_decode_insn (insn, &lrd, &lra, &lrb, &limm); > + if (lra >= 0 && lra < MICROBLAZE_NUM_REGS) > + ra = get_frame_register_unsigned (frame, lra); > + else > + ra = 0; > + if (lrb >= 0 && lrb < MICROBLAZE_NUM_REGS) > + rb = get_frame_register_unsigned (frame, lrb); > + else > + rb = 0; > + address = microblaze_get_target_address (insn, immfound, imm, pc, > + ra, rb, &targetvalid, &unconditionalbranch); > + > + if (!unconditionalbranch) > + breaks[1] = address; > + } > + > + /* Insert the breakpoints */ > + if (breaks[0] != -1) > + { > + insert_single_step_breakpoint (arch, aspace, breaks[0]); > + ret = 1; > + } > + if (breaks[1] != -1) > + { > + insert_single_step_breakpoint (arch, aspace, breaks[1]); > + ret = 1; > + } > + > + return ret; > +} > + > static void > microblaze_write_pc (struct regcache *regcache, CORE_ADDR pc) { @@ -708,6 +785,8 @@ microblaze_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > > set_gdbarch_breakpoint_from_pc (gdbarch, microblaze_breakpoint_from_pc); > > + set_gdbarch_software_single_step (gdbarch, > + microblaze_software_single_step); > + > set_gdbarch_frame_args_skip (gdbarch, 8); > > set_gdbarch_print_insn (gdbarch, print_insn_microblaze); > -- > 1.7.1 > > Thanks & Regards > Ajit -- Joel