Re: [RFA] Resubmit process record and replay, 4/10

[Pedro Alves <pedro@codesourcery.com>]

A Thursday 20 November 2008 01:46:05, Michael Snyder escreveu:
> Seems like a couple of people had some doubts about this
> particular part of the implementation.
> 
> Does anyone have any alternative suggestions?
> 

Sounds like a deficiency of the target interface, as most target_ops
functions don't take a "this" pointer and do the "beneath"
lookup themselves.

Would doing something like this work here? :

  for (t = current_target.beneath; t != NULL; t = t->beneath)
    if (t->to_xfer_partial != NULL)
	{
	  t->to_xfer_partial (...);
	  return;
	}

  etc., that is don't stop looking at the immediate
  target beneath, but look until you find one that suits instead.

I'd prefer that the core side isn't special cased for
the record target.  If need be, we can add, change or adapt
the target interfaces.

-- 
Pedro Alves


> teawater wrote:
> > The process record and replay target has six function pointers
> > record_beneath_to_resume, record_beneath_to_wait,
> > record_beneath_to_prepare_to_store, record_beneath_to_xfer_partial,
> > record_beneath_to_insert_breakpoint and
> > record_beneath_to_remove_breakpoint.
> > This patch add code to function "update_current_target" make these
> > pointers always point to the function of lower strata target than
> > process record and replay target.
> > 
> > When GDB in record mode, we need to call the really debug function in
> > low strata target because process record and replay target need call
> > this function to control the inferior.
> > 
> > Struct target_ops already has a pointer "beneath" point to low strata
> > target, but process record and replay target doesn't use it.  Because
> > if low strata target doesn't set some function pointers, process
> > record and replay target will need to call the function pointers of
> > the target that is low strata target of this target.  If this target
> > doesn't set them too, it will need to call anothers.  So use "beneath"
> > is not a good choice and "multi-thread" target that need function
> > pointers of low strata target doesn't use "beneath" too.
> > 
> > 2008-11-16  Hui Zhu  <teawater@gmail.com>
> > 
> > 	* target.c (update_current_target): Set function points
> > 	record_beneath_to_resume,
> > 	record_beneath_to_wait,
> > 	record_beneath_to_prepare_to_store,
> > 	record_beneath_to_xfer_partial,
> > 	record_beneath_to_insert_breakpoint
> > 	and record_beneath_to_remove_breakpoint point to the function
> > 	of target that lower than process record and replay target.
> > 
> >  target.c |   36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > Following is the diff with the previous patch:
> > @@ -8,7 +8,7 @@
> > 
> >   static void target_info (char *, int);
> > 
> > -@@ -387,6 +388,12 @@ update_current_target (void)
> > +@@ -405,6 +406,12 @@
> >         if (!current_target.FIELD) \
> >   	current_target.FIELD = (TARGET)->FIELD
> > 
> > @@ -21,12 +21,12 @@
> >     for (t = target_stack; t; t = t->beneath)
> >       {
> >         INHERIT (to_shortname, t);
> > -@@ -476,6 +483,35 @@ update_current_target (void)
> > +@@ -494,6 +501,35 @@
> >         /* Do not inherit to_memory_map.  */
> >         /* Do not inherit to_flash_erase.  */
> >         /* Do not inherit to_flash_done.  */
> >  +
> > -+      /* Set the real beneath function pointers. */
> > ++      /* Set pointers to functions in the target beneath us.  */
> >  +      if (t != &record_ops)
> >  +        {
> >  +           if (!record_beneath_to_resume)
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > --- a/target.c
> > +++ b/target.c
> > @@ -41,6 +41,7 @@
> >  #include "target-descriptions.h"
> >  #include "gdbthread.h"
> >  #include "solib.h"
> > +#include "record.h"
> >  
> >  static void target_info (char *, int);
> >  
> > @@ -405,6 +406,12 @@
> >        if (!current_target.FIELD) \
> >  	current_target.FIELD = (TARGET)->FIELD
> >  
> > +  record_beneath_to_resume = NULL;
> > +  record_beneath_to_store_registers = NULL;
> > +  record_beneath_to_xfer_partial = NULL;
> > +  record_beneath_to_insert_breakpoint = NULL;
> > +  record_beneath_to_remove_breakpoint = NULL;
> > +
> >    for (t = target_stack; t; t = t->beneath)
> >      {
> >        INHERIT (to_shortname, t);
> > @@ -494,6 +501,35 @@
> >        /* Do not inherit to_memory_map.  */
> >        /* Do not inherit to_flash_erase.  */
> >        /* Do not inherit to_flash_done.  */
> > +
> > +      /* Set pointers to functions in the target beneath us.  */
> > +      if (t != &record_ops)
> > +        {
> > +           if (!record_beneath_to_resume)
> > +             {
> > +               record_beneath_to_resume = t->to_resume;
> > +             }
> > +           if (!record_beneath_to_wait)
> > +             {
> > +               record_beneath_to_wait = t->to_wait;
> > +             }
> > +           if (!record_beneath_to_store_registers)
> > +             {
> > +               record_beneath_to_store_registers = t->to_store_registers;
> > +             }
> > +           if (!record_beneath_to_xfer_partial)
> > +             {
> > +               record_beneath_to_xfer_partial = t->to_xfer_partial;
> > +             }
> > +           if (!record_beneath_to_insert_breakpoint)
> > +             {
> > +               record_beneath_to_insert_breakpoint = t->to_insert_breakpoint;
> > +             }
> > +           if (!record_beneath_to_remove_breakpoint)
> > +             {
> > +               record_beneath_to_remove_breakpoint = t->to_remove_breakpoint;
> > +             }
> > +        }
> >      }
> >  #undef INHERIT
> >  
> 
> 



-- 
Pedro Alves