[RFA] Add mips_software_single_step

[RFA] Add mips_software_single_step

[Daniel Jacobowitz]

This function's pretty straightforward; the Linux port uses it.  Ok to
commit?

I've been tempted to redo this more like the Sparc's version, in which we
set breakpoints at all possible destinations rather than at the calculated
next destination.  But I think that with the previously posted patch, I
trust mips_next_pc enough to leave it this way.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-07-04  Daniel Jacobowitz  <drow@mvista.com>
	* mips-tdep.c (mips_software_single_step): New function.

Index: gdb/mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.54
diff -u -r1.54 mips-tdep.c
--- mips-tdep.c	2001/06/16 20:00:24	1.54
+++ mips-tdep.c	2001/07/04 18:15:03
@@ -1379,6 +1392,35 @@
   return addr;
 }
 
+/* mips_software_single_step() 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 (MIPS on Linux for example).  We find
+   the target of the coming instruction and breakpoint it.
+
+   single_step is also called just after the inferior stops.  If we had
+   set up a simulated single-step, we undo our damage.  */
+
+void
+mips_software_single_step (signal, insert_breakpoints_p)
+     unsigned int signal;
+     int insert_breakpoints_p;
+{
+  static CORE_ADDR next_pc;
+  typedef char binsn_quantum[BREAKPOINT_MAX];
+  static binsn_quantum break_mem;
+  CORE_ADDR pc;
+
+  if (insert_breakpoints_p)
+    {
+      pc = read_register (PC_REGNUM);
+      next_pc = mips_next_pc (pc);
+
+      target_insert_breakpoint (next_pc, break_mem);
+    }
+  else
+      target_remove_breakpoint (next_pc, break_mem);
+}
+
 static void
 mips_init_frame_pc_first (int fromleaf, struct frame_info *prev)
 {

Re: [RFA] Add mips_software_single_step

[Andrew Cagney]

> This function's pretty straightforward; the Linux port uses it.  Ok to
> commit?


How does the linux port use it?  As a tweek to the multi-arch vector or 
as a macro/function?  If the former then it should be static, if the 
latter than a declaration in mips-tdep.h or tm-mips.h (?) is needed.


> I've been tempted to redo this more like the Sparc's version, in which we
> set breakpoints at all possible destinations rather than at the calculated
> next destination.  But I think that with the previously posted patch, I
> trust mips_next_pc enough to leave it this way.


The function is almost ok.  It just needs to be updated to meet GDB's 
current coding conventions (ISO-C, indentation).

With regard to the linux code, you should probably look over 
http://sources.redhat.com/gdb/ari/ and check any new code for potential 
regressions or errors.  Just remember to ignore any of the warnings :-)

	Andrew

Re: [RFA] Add mips_software_single_step

[Daniel Jacobowitz]

On Thu, Jul 05, 2001 at 05:50:14PM -0400, Andrew Cagney wrote:
> > This function's pretty straightforward; the Linux port uses it.  Ok to
> > commit?
> 
> 
> How does the linux port use it?  As a tweek to the multi-arch vector or 
> as a macro/function?  If the former then it should be static, if the 
> latter than a declaration in mips-tdep.h or tm-mips.h (?) is needed.

As a macro:

#define SOFTWARE_SINGLE_STEP_P() 1
extern void mips_software_single_step (unsigned int, int);
#define SOFTWARE_SINGLE_STEP(sig,bp_p) mips_software_single_step (sig, bp_p)

I'm sensing that this belongs in the multi-arch vector, though.  I'll
change that.

The question this raises is how to do it.  I'd rather do it in
mips-linux-tdep.c than mips-tdep.c, and mips-tdep shouldn't know we're
compiling for a Linux target (it already does, via MIPS_DEFAULT_ABI,
but I'll address that in a bit).  How can I modify the gdbarch vector
in a second place?

> The function is almost ok.  It just needs to be updated to meet GDB's 
> current coding conventions (ISO-C, indentation).

Right.  I'll do that too.

> With regard to the linux code, you should probably look over 
> http://sources.redhat.com/gdb/ari/ and check any new code for potential 
> regressions or errors.  Just remember to ignore any of the warnings :-)

I think I'm OK on most of those; I'll go over it again later.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Add mips_software_single_step

[Andrew Cagney]

> On Thu, Jul 05, 2001 at 05:50:14PM -0400, Andrew Cagney wrote:
> 
>> > This function's pretty straightforward; the Linux port uses it.  Ok to
>> > commit?
> 
>> 
>> 
>> How does the linux port use it?  As a tweek to the multi-arch vector or 
>> as a macro/function?  If the former then it should be static, if the 
>> latter than a declaration in mips-tdep.h or tm-mips.h (?) is needed.
> 
> 
> As a macro:
> 
> #define SOFTWARE_SINGLE_STEP_P() 1
> extern void mips_software_single_step (unsigned int, int);
> #define SOFTWARE_SINGLE_STEP(sig,bp_p) mips_software_single_step (sig, bp_p)


The macro definition appearing in config/mips/tm-linux.h but the 
function declaration appearing in config/mips/tm-mips.h?  The function 
declaration is in tm-mips.h so that mips-tdep.c always sees it.


> I'm sensing that this belongs in the multi-arch vector, though.  I'll
> change that.


For the moment the above is ok.


> The question this raises is how to do it.  I'd rather do it in
> mips-linux-tdep.c than mips-tdep.c, and mips-tdep shouldn't know we're
> compiling for a Linux target (it already does, via MIPS_DEFAULT_ABI,
> but I'll address that in a bit).  How can I modify the gdbarch vector
> in a second place?


You're on the right track but (as I wrote):

> The wart?  So far multi-arch has concentrated on problems like:
> 
>     MIPS2 embedded
>     MIPS3 embedded
> 
> but has avoided cases such as:
> 
>     Linux/MIPS native and remote
>     MIPS embedded
>     *BSD/MIPS remote
> 
> i.e. OS as well as ISA/ABI as a variant.  For nm.h to be reduced to

> just _native_ interface support, multi-arch will need to be extended

> to include this.


We're getting into the relm of theory :-)  No one has so far tried to do 
this.  (Well, ok I think Eli is setting himself up for the challenge on 
IRIX.)

The multi-arch notes suggest:

	o	call generic code to create
		a multi-arch vector as per
		ABI/ISA:

		mips-linux-tdep.c calls
		mips-tdep.c:create_gdbarch_object()

	o	refine the created object
		to be OS specific.

The gap in all this is the bit of logic that decides what (mip-tdep.c or 
mips-linux-tdep.c) should create the multi-arch vector.


>> The function is almost ok.  It just needs to be updated to meet GDB's 
>> current coding conventions (ISO-C, indentation).
> 
> 
> Right.  I'll do that too.
> 
> 
>> With regard to the linux code, you should probably look over 
>> http://sources.redhat.com/gdb/ari/ and check any new code for potential 
>> regressions or errors.  Just remember to ignore any of the warnings :-)
> 
> 
> I think I'm OK on most of those; I'll go over it again later.


If you think you're ok then that is more than sufficient.

	Andrew

Re: [RFA] Add mips_software_single_step

[Daniel Jacobowitz]

On Thu, Jul 05, 2001 at 06:50:26PM -0400, Andrew Cagney wrote:
> > On Thu, Jul 05, 2001 at 05:50:14PM -0400, Andrew Cagney wrote:
> > 
> >> > This function's pretty straightforward; the Linux port uses it.  Ok to
> >> > commit?
> > 
> >> 
> >> 
> >> How does the linux port use it?  As a tweek to the multi-arch vector or 
> >> as a macro/function?  If the former then it should be static, if the 
> >> latter than a declaration in mips-tdep.h or tm-mips.h (?) is needed.
> > 
> > 
> > As a macro:
> > 
> > #define SOFTWARE_SINGLE_STEP_P() 1
> > extern void mips_software_single_step (unsigned int, int);
> > #define SOFTWARE_SINGLE_STEP(sig,bp_p) mips_software_single_step (sig, bp_p)
> 
> 
> The macro definition appearing in config/mips/tm-linux.h but the 
> function declaration appearing in config/mips/tm-mips.h?  The function 
> declaration is in tm-mips.h so that mips-tdep.c always sees it.


Right.  My source right now has the declaration in tm-linux.h, but
tm-mips.h is more appropriate; I'll move it before I commit.

> > I'm sensing that this belongs in the multi-arch vector, though.  I'll
> > change that.
> 
> 
> For the moment the above is ok.

OK, then.  Is the style-corrected version below OK?

> We're getting into the relm of theory :-)  No one has so far tried to do 
> this.  (Well, ok I think Eli is setting himself up for the challenge on 
> IRIX.)

I guess I'll revisit this along with Eli once I get the port settled in
some basic state, then.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-07-04  Daniel Jacobowitz  <drow@mvista.com>

	* mips-tdep.c (mips_software_single_step): New function.
	* config/mips/tm-mips.h: Add prototype for
	mips_software_single_step.

Index: config/mips/tm-mips.h
===================================================================
RCS file: /cvs/src/src/gdb/config/mips/tm-mips.h,v
retrieving revision 1.20
diff -u -r1.20 tm-mips.h
--- tm-mips.h	2001/06/15 23:10:55	1.20
+++ tm-mips.h	2001/07/05 23:00:08
@@ -502,3 +502,6 @@
 /* MIPS sign extends addresses */
 #define POINTER_TO_ADDRESS(TYPE,BUF) (signed_pointer_to_address (TYPE, BUF))
 #define ADDRESS_TO_POINTER(TYPE,BUF,ADDR) (address_to_signed_pointer (TYPE, BUF, ADDR))
+
+/* Single step based on where the current instruction will take us.  */
+extern void mips_software_single_step (enum target_signal, int);
Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.54
diff -u -r1.54 mips-tdep.c
--- mips-tdep.c	2001/06/16 20:00:24	1.54
+++ mips-tdep.c	2001/07/05 23:00:09
@@ -1379,6 +1392,33 @@
   return addr;
 }
 
+/* mips_software_single_step() 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 (MIPS on Linux for example).  We find
+   the target of the coming instruction and breakpoint it.
+
+   single_step is also called just after the inferior stops.  If we had
+   set up a simulated single-step, we undo our damage.  */
+
+void
+mips_software_single_step (enum target_signal sig, int insert_breakpoints_p)
+{
+  static CORE_ADDR next_pc;
+  typedef char binsn_quantum[BREAKPOINT_MAX];
+  static binsn_quantum break_mem;
+  CORE_ADDR pc;
+
+  if (insert_breakpoints_p)
+    {
+      pc = read_register (PC_REGNUM);
+      next_pc = mips_next_pc (pc);
+
+      target_insert_breakpoint (next_pc, break_mem);
+    }
+  else
+    target_remove_breakpoint (next_pc, break_mem);
+}
+
 static void
 mips_init_frame_pc_first (int fromleaf, struct frame_info *prev)
 {

Re: [RFA] Add mips_software_single_step

[Andrew Cagney]

> OK, then.  Is the style-corrected version below OK?


Yes, thanks.

	Andrew

Re: [RFA] Add mips_software_single_step

[Daniel Jacobowitz]

On Thu, Jul 05, 2001 at 07:24:26PM -0400, Andrew Cagney wrote:
> > OK, then.  Is the style-corrected version below OK?
> 
> 
> Yes, thanks.

Thanks.  Committed, then.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer