Re: gdbserver

[Daniel Jacobowitz <drow@mvista.com>]

On Fri, Aug 30, 2002 at 04:34:39PM -0700, Michael Snyder wrote:
> Daniel Jacobowitz wrote:
> > 
> > [Bart, please try this patch.]
> > 
> > List folks,
> > 
> > I think the time has come for generic_prepare_to_proceed to actually be
> > used.  The problem addressed by this patch is that PREPARE_TO_PROCEED
> > is not a native-only macro (and not part of the target stack).  So
> > lin_lwp_prepare_to_proceed would be called when using gdbserver, and of
> > course trap_ptid would be null_ptid or stale.
> > generic_prepare_to_proceed works correctly for lin-lwp native
> > debugging, and for remote debugging.  This patch fixes an incorrect
> > breakpoint hit after manually switching threads; i.e. the same
> > breakpoint would be hit a second time.  I'll try to write an
> > independent test case.
> > 
> > Patch look OK?
> 
> Of course, a simpler and less intrusive fix would be to simply
> define PREPARE_TO_PROCEED as generic_prepare_to_proceed, and 
> remove lin_lwp_prepare_to_proceed.  
> 
> I'm not necessarily objecting to this patch -- just pointing
> out an alternative.  If people think we're ready for this step, 
> it's fine with me.

I think we're ready, but let's wait and see.  For any non-threaded
target generic_prepare_to_proceed won't do any harm, since it checks
inferior_ptid != resume_ptid; for threaded targets, some version of
this function must be better than none.

> 
> 
> > On Fri, Aug 23, 2002 at 08:35:17AM +0200, Van Assche, Bart wrote:
> > > Hello Dan,
> > >
> > >    Thanks for your response. I tried with the latest snapshot, and debugging
> > > threads has indeed improved. However, I still get a SIGSEGV reported when I
> > > use remote debugging (that I do not get with native debugging -- see also
> > > below).
> > 
> > --
> > Daniel Jacobowitz
> > MontaVista Software                         Debian GNU/Linux Developer
> > 
> > 2002-08-30  Daniel Jacobowitz  <drow@mvista.com>
> > 
> >         * arch-utils.c (default_prepare_to_proceed): Remove.
> >         * arch-utils.h (default_prepare_to_proceed): Remove prototype.
> >         * gdbarch.sh (PREPARE_TO_PROCEED): Change default to
> >         generic_prepare_to_proceed.
> >         * gdbarch.c: Regenerated.
> >         * gdbarch.h: Regenerated.
> >         * lin-lwp.c (lin_lwp_prepare_to_proceed): Remove.
> >         * config/nm-linux.h (PREPARE_TO_PROCEED): Don't define.
> >         (lin_lwp_prepare_to_proceed): Remove prototype.
> > 
> > Index: arch-utils.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/arch-utils.c,v
> > retrieving revision 1.67
> > diff -u -p -r1.67 arch-utils.c
> > --- arch-utils.c        24 Aug 2002 00:21:34 -0000      1.67
> > +++ arch-utils.c        30 Aug 2002 22:13:11 -0000
> > @@ -320,13 +320,6 @@ default_frame_address (struct frame_info
> >    return fi->frame;
> >  }
> > 
> > -/* Default prepare_to_procced().  */
> > -int
> > -default_prepare_to_proceed (int select_it)
> > -{
> > -  return 0;
> > -}
> > -
> >  /* Generic prepare_to_proceed().  This one should be suitable for most
> >     targets that support threads. */
> >  int
> > Index: gdbarch.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbarch.c,v
> > retrieving revision 1.146
> > diff -u -p -r1.146 gdbarch.c
> > --- gdbarch.c   24 Aug 2002 00:21:34 -0000      1.146
> > +++ gdbarch.c   30 Aug 2002 22:13:12 -0000
> > @@ -530,7 +530,7 @@ gdbarch_alloc (const struct gdbarch_info
> >    current_gdbarch->memory_insert_breakpoint = default_memory_insert_breakpoint;
> >    current_gdbarch->memory_remove_breakpoint = default_memory_remove_breakpoint;
> >    current_gdbarch->decr_pc_after_break = -1;
> > -  current_gdbarch->prepare_to_proceed = default_prepare_to_proceed;
> > +  current_gdbarch->prepare_to_proceed = generic_prepare_to_proceed;
> >    current_gdbarch->function_start_offset = -1;
> >    current_gdbarch->remote_translate_xfer_address = generic_remote_translate_xfer_address;
> >    current_gdbarch->frame_args_skip = -1;
> > Index: gdbarch.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbarch.h,v
> > retrieving revision 1.114
> > diff -u -p -r1.114 gdbarch.h
> > --- gdbarch.h   24 Aug 2002 00:21:34 -0000      1.114
> > +++ gdbarch.h   30 Aug 2002 22:13:13 -0000
> > @@ -1809,7 +1809,7 @@ extern void set_gdbarch_decr_pc_after_br
> > 
> >  /* Default (function) for non- multi-arch platforms. */
> >  #if (!GDB_MULTI_ARCH) && !defined (PREPARE_TO_PROCEED)
> > -#define PREPARE_TO_PROCEED(select_it) (default_prepare_to_proceed (select_it))
> > +#define PREPARE_TO_PROCEED(select_it) (generic_prepare_to_proceed (select_it))
> >  #endif
> > 
> >  typedef int (gdbarch_prepare_to_proceed_ftype) (int select_it);
> > Index: gdbarch.sh
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbarch.sh,v
> > retrieving revision 1.158
> > diff -u -p -r1.158 gdbarch.sh
> > --- gdbarch.sh  24 Aug 2002 00:21:34 -0000      1.158
> > +++ gdbarch.sh  30 Aug 2002 22:13:14 -0000
> > @@ -549,7 +549,7 @@ f:2:BREAKPOINT_FROM_PC:const unsigned ch
> >  f:2:MEMORY_INSERT_BREAKPOINT:int:memory_insert_breakpoint:CORE_ADDR addr, char *contents_cache:addr, contents_cache::0:default_memory_insert_breakpoint::0
> >  f:2:MEMORY_REMOVE_BREAKPOINT:int:memory_remove_breakpoint:CORE_ADDR addr, char *contents_cache:addr, contents_cache::0:default_memory_remove_breakpoint::0
> >  v:2:DECR_PC_AFTER_BREAK:CORE_ADDR:decr_pc_after_break::::0:-1
> > -f::PREPARE_TO_PROCEED:int:prepare_to_proceed:int select_it:select_it::0:default_prepare_to_proceed::0
> > +f::PREPARE_TO_PROCEED:int:prepare_to_proceed:int select_it:select_it::0:generic_prepare_to_proceed::0
> >  v:2:FUNCTION_START_OFFSET:CORE_ADDR:function_start_offset::::0:-1
> >  #
> >  f:2:REMOTE_TRANSLATE_XFER_ADDRESS:void:remote_translate_xfer_address:CORE_ADDR gdb_addr, int gdb_len, CORE_ADDR *rem_addr, int *rem_len:gdb_addr, gdb_len, rem_addr, rem_len:::generic_remote_translate_xfer_address::0
> > Index: lin-lwp.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/lin-lwp.c,v
> > retrieving revision 1.35
> > diff -u -p -r1.35 lin-lwp.c
> > --- lin-lwp.c   27 Aug 2002 22:37:06 -0000      1.35
> > +++ lin-lwp.c   30 Aug 2002 22:13:14 -0000
> > @@ -294,46 +294,6 @@ iterate_over_lwps (int (*callback) (stru
> >  }
> > 
> > 
> > -/* Implementation of the PREPARE_TO_PROCEED hook for the GNU/Linux LWP
> > -   layer.
> > -
> > -   Note that this implementation is potentially redundant now that
> > -   default_prepare_to_proceed() has been added.
> > -
> > -   FIXME This may not support switching threads after Ctrl-C
> > -   correctly. The default implementation does support this. */
> > -
> > -int
> > -lin_lwp_prepare_to_proceed (void)
> > -{
> > -  if (! ptid_equal (trap_ptid, null_ptid)
> > -      && ! ptid_equal (inferior_ptid, trap_ptid))
> > -    {
> > -      /* Switched over from TRAP_PID.  */
> > -      CORE_ADDR stop_pc = read_pc ();
> > -      CORE_ADDR trap_pc;
> > -
> > -      /* Avoid switching where it wouldn't do any good, i.e. if both
> > -         threads are at the same breakpoint.  */
> > -      trap_pc = read_pc_pid (trap_ptid);
> > -      if (trap_pc != stop_pc && breakpoint_here_p (trap_pc))
> > -       {
> > -         /* User hasn't deleted the breakpoint.  Return non-zero, and
> > -             switch back to TRAP_PID.  */
> > -         inferior_ptid = trap_ptid;
> > -
> > -         /* FIXME: Is this stuff really necessary?  */
> > -         flush_cached_frames ();
> > -         registers_changed ();
> > -
> > -         return 1;
> > -       }
> > -    }
> > -
> > -  return 0;
> > -}
> > -
> > -
> >  #if 0
> >  static void
> >  lin_lwp_open (char *args, int from_tty)
> > Index: config/nm-linux.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/config/nm-linux.h,v
> > retrieving revision 1.12
> > diff -u -p -r1.12 nm-linux.h
> > --- config/nm-linux.h   24 Feb 2002 22:56:04 -0000      1.12
> > +++ config/nm-linux.h   30 Aug 2002 22:13:14 -0000
> > @@ -49,9 +49,6 @@ struct target_waitstatus;
> >  extern ptid_t child_wait (ptid_t ptid, struct target_waitstatus *ourstatus);
> >  #define CHILD_WAIT
> > 
> > -extern int lin_lwp_prepare_to_proceed (void);
> > -#define PREPARE_TO_PROCEED(select_it) lin_lwp_prepare_to_proceed ()
> > -
> >  extern void lin_lwp_attach_lwp (ptid_t ptid, int verbose);
> >  #define ATTACH_LWP(ptid, verbose) lin_lwp_attach_lwp ((ptid), (verbose))
> > 
> > Index: arch-utils.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/arch-utils.h,v
> > retrieving revision 1.41
> > diff -u -p -r1.41 arch-utils.h
> > --- arch-utils.h        24 Aug 2002 00:21:34 -0000      1.41
> > +++ arch-utils.h        30 Aug 2002 22:15:25 -0000
> > @@ -115,8 +115,6 @@ extern CORE_ADDR default_frame_address (
> > 
> >  /* Default prepare_to_procced. */
> > 
> > -extern int default_prepare_to_proceed (int select_it);
> > -
> >  extern int generic_prepare_to_proceed (int select_it);
> > 
> >  /* Versions of init_frame_pc().  Do nothing; do the default. */
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer