Re: [RFC, RFA] multi-arch PREPARE_TO_PROCEED()

[Andrew Cagney <ac131313 at cygnus dot com>]

David Smith wrote:

David two thoughts:

	o	is the parameter ``select_it'' needed?

	o	In your opinion, is there any reason
		to have anything other than the
		generic_prepare_to_proceed()?

I'm thinking that we should dump the HP/UX and Linux implementations and
instead, always use your code.  So ... can someone test David's change
under linux threads?

Anyway, I'm ok with the multi-arch part (both with and without the
``select_it'' parameter) and the change to infrun.c.

With respect to the possibility of dumping the HP/UX and linux
implementations - MichaelS or MarkK?

	Andrew

> This patch multi-arch's the PREPARE_TO_PROCEED macro.  PREPARE_TO_PROCEED
> gets called in proceed() (infrun.c:1028).  Here's where it gets called (which
> also explains its use):
> 
> #ifdef PREPARE_TO_PROCEED
>    /* In a multi-threaded task we may select another thread
>       and then continue or step.
> 
>       But if the old thread was stopped at a breakpoint, it
>       will immediately cause another breakpoint stop without
>       any execution (i.e. it will report a breakpoint hit
>       incorrectly).  So we must step over it first.
> 
>       PREPARE_TO_PROCEED checks the current thread against the thread
>       that reported the most recent event.  If a step-over is required
>       it returns TRUE and sets the current thread to the old thread. */
>    if (PREPARE_TO_PROCEED (1) && breakpoint_here_p (read_pc ()))
>      {
>        oneproc = 1;
>        thread_step_needed = 1;
>      }
> 
> #endif /* PREPARE_TO_PROCEED */
> 
> The real problem with this change is that to know whether or not the user has
> switched threads since the breakpoint was hit, we have to cache the return
> values of target_wait()/target_wait_hook().  So, I modified
> handle_inferior_event() to cache the needed information (and I added a
> function to returned the cached information).  The existing implementations
> (hppa-tdep.c, lin-lwp.c, linux-thread.c, m3-nat.c) cheat a bit by defining
> their own wait functions.
> 
> Here's the ChangeLog entry:
> 
> 2001-03-29  David Smith  <dsmith@redhat.com>
> 
>         * arch-utils.c (default_prepare_to_proceed)
>         (generic_prepare_to_proceed): Added new functions.
>         * arch-utils.h: New function declarations for
>         default_prepare_to_proceed() and generic_prepare_to_proceed().
>         * gdbarch.sh: Added PREPARE_TO_PROCEED.
>         * gdbarch.c: Regenerated.
>         * gdbarch.h: Regenerated.
>         * inferior.h: Added get_last_target_status() declaration.
>         * infrun.c (get_last_target_status): Added new function.
>         (handle_inferior_event): Saves last pid and waitstatus, which will
>         get returned by get_last_target_status().
> --
> David Smith
> dsmith@redhat.com
> Red Hat, Inc.
> http://www.redhat.com
> 256.704.9222 (direct)
> 256.837.3839 (fax)
> 
>   ------------------------------------------------------------------------
> Index: gdb/arch-utils.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arch-utils.c,v
> retrieving revision 1.22
> diff -c -p -u -p -r1.22 arch-utils.c
> --- gdb/arch-utils.c    2001/03/07 02:57:08     1.22
> +++ gdb/arch-utils.c    2001/03/29 20:06:46
> @@ -248,6 +248,61 @@ 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
> +generic_prepare_to_proceed (int select_it)
> +{
> +  int wait_pid;
> +  struct target_waitstatus wait_status;
> +
> +  /* Get the last target status returned by target_wait().  */
> +  get_last_target_status (&wait_pid, &wait_status);
> +
> +  /* Make sure we were stopped at a breakpoint.  */
> +  if (wait_status.kind != TARGET_WAITKIND_STOPPED
> +      || wait_status.value.sig != TARGET_SIGNAL_TRAP)
> +    {
> +      return 0;
> +    }
> +
> +  if (wait_pid != -1 && inferior_pid != wait_pid)
> +    {
> +      /* Switched over from WAIT_PID.  */
> +      CORE_ADDR wait_pc = read_pc_pid (wait_pid);
> +
> +      /* Avoid switching where it wouldn't do any good, i.e. if both
> +         threads are at the same breakpoint.  */
> +      if (wait_pc != read_pc () && breakpoint_here_p (wait_pc))
> +       {
> +         if (select_it)
> +           {
> +             /* User hasn't deleted the breakpoint.  Switch back to
> +                WAIT_PID and return non-zero.  */
> +             inferior_pid = wait_pid;
> +
> +             /* FIXME: This stuff came from switch_to_thread() in
> +                thread.c (which should probably be a public function).  */
> +             flush_cached_frames ();
> +             registers_changed ();
> +             stop_pc = wait_pc;
> +             select_frame (get_current_frame (), 0);
> +           }
> +
> +         return 1;
> +       }
> +    }
> +  return 0;
> +
> +}
> +
>  /* Functions to manipulate the endianness of the target.  */
> 
>  #ifdef TARGET_BYTE_ORDER_SELECTABLE
> Index: gdb/arch-utils.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/arch-utils.h,v
> retrieving revision 1.13
> diff -c -p -u -p -r1.13 arch-utils.h
> --- gdb/arch-utils.h    2001/03/06 08:21:05     1.13
> +++ gdb/arch-utils.h    2001/03/29 20:06:46
> @@ -105,5 +105,10 @@ extern int no_op_reg_to_regnum (int reg)
> 
>  extern CORE_ADDR default_frame_address (struct frame_info *);
> 
> +/* Default prepare_to_procced. */
> +
> +extern int default_prepare_to_proceed (int select_it);
> +
> +extern int generic_prepare_to_proceed (int select_it);
> 
>  #endif
> Index: gdb/gdbarch.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbarch.c,v
> retrieving revision 1.56
> diff -c -p -u -p -r1.56 gdbarch.c
> --- gdb/gdbarch.c       2001/03/24 02:07:48     1.56
> +++ gdb/gdbarch.c       2001/03/29 20:07:00
> @@ -226,6 +226,7 @@ struct gdbarch
>    gdbarch_memory_insert_breakpoint_ftype *memory_insert_breakpoint;
>    gdbarch_memory_remove_breakpoint_ftype *memory_remove_breakpoint;
>    CORE_ADDR decr_pc_after_break;
> +  gdbarch_prepare_to_proceed_ftype *prepare_to_proceed;
>    CORE_ADDR function_start_offset;
>    gdbarch_remote_translate_xfer_address_ftype *remote_translate_xfer_address;
>    CORE_ADDR frame_args_skip;
> @@ -380,6 +381,7 @@ struct gdbarch startup_gdbarch =
>    0,
>    0,
>    0,
> +  0,
>    /* startup_gdbarch() */
>  };
> 
> @@ -457,6 +459,7 @@ gdbarch_alloc (const struct gdbarch_info
>    gdbarch->memory_insert_breakpoint = default_memory_insert_breakpoint;
>    gdbarch->memory_remove_breakpoint = default_memory_remove_breakpoint;
>    gdbarch->decr_pc_after_break = -1;
> +  gdbarch->prepare_to_proceed = default_prepare_to_proceed;
>    gdbarch->function_start_offset = -1;
>    gdbarch->remote_translate_xfer_address = generic_remote_translate_xfer_address;
>    gdbarch->frame_args_skip = -1;
> @@ -710,6 +713,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
>        && (gdbarch->decr_pc_after_break == -1))
>      internal_error (__FILE__, __LINE__,
>                      "gdbarch: verify_gdbarch: decr_pc_after_break invalid");
> +  /* Skip verify of prepare_to_proceed, invalid_p == 0 */
>    if ((GDB_MULTI_ARCH >= 2)
>        && (gdbarch->function_start_offset == -1))
>      internal_error (__FILE__, __LINE__,
> @@ -1320,6 +1324,12 @@ gdbarch_dump (struct gdbarch *gdbarch, s
>                        "gdbarch_dump: DECR_PC_AFTER_BREAK # %s\n",
>                        XSTRING (DECR_PC_AFTER_BREAK));
>  #endif
> +#ifdef PREPARE_TO_PROCEED
> +  fprintf_unfiltered (file,
> +                      "gdbarch_dump: %s # %s\n",
> +                      "PREPARE_TO_PROCEED(select_it)",
> +                      XSTRING (PREPARE_TO_PROCEED (select_it)));
> +#endif
>  #ifdef FUNCTION_START_OFFSET
>    fprintf_unfiltered (file,
>                        "gdbarch_dump: FUNCTION_START_OFFSET # %s\n",
> @@ -2034,6 +2044,13 @@ gdbarch_dump (struct gdbarch *gdbarch, s
>                        "gdbarch_dump: DECR_PC_AFTER_BREAK = %ld\n",
>                        (long) DECR_PC_AFTER_BREAK);
>  #endif
> +#ifdef PREPARE_TO_PROCEED
> +  if (GDB_MULTI_ARCH)
> +    fprintf_unfiltered (file,
> +                        "gdbarch_dump: PREPARE_TO_PROCEED = 0x%08lx\n",
> +                        (long) current_gdbarch->prepare_to_proceed
> +                        /*PREPARE_TO_PROCEED ()*/);
> +#endif
>  #ifdef FUNCTION_START_OFFSET
>    fprintf_unfiltered (file,
>                        "gdbarch_dump: FUNCTION_START_OFFSET = %ld\n",
> @@ -3866,6 +3883,24 @@ set_gdbarch_decr_pc_after_break (struct
>                                   CORE_ADDR decr_pc_after_break)
>  {
>    gdbarch->decr_pc_after_break = decr_pc_after_break;
> +}
> +
> +int
> +gdbarch_prepare_to_proceed (struct gdbarch *gdbarch, int select_it)
> +{
> +  if (gdbarch->prepare_to_proceed == 0)
> +    internal_error (__FILE__, __LINE__,
> +                    "gdbarch: gdbarch_prepare_to_proceed invalid");
> +  if (gdbarch_debug >= 2)
> +    fprintf_unfiltered (gdb_stdlog, "gdbarch_prepare_to_proceed called\n");
> +  return gdbarch->prepare_to_proceed (select_it);
> +}
> +
> +void
> +set_gdbarch_prepare_to_proceed (struct gdbarch *gdbarch,
> +                                gdbarch_prepare_to_proceed_ftype prepare_to_proceed)
> +{
> +  gdbarch->prepare_to_proceed = prepare_to_proceed;
>  }
> 
>  CORE_ADDR
> Index: gdb/gdbarch.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbarch.h,v
> retrieving revision 1.46
> diff -c -p -u -p -r1.46 gdbarch.h
> --- gdb/gdbarch.h       2001/03/24 02:07:48     1.46
> +++ gdb/gdbarch.h       2001/03/29 20:07:00
> @@ -1215,6 +1215,20 @@ extern void set_gdbarch_decr_pc_after_br
>  #endif
>  #endif
> 
> +/* 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))
> +#endif
> +
> +typedef int (gdbarch_prepare_to_proceed_ftype) (int select_it);
> +extern int gdbarch_prepare_to_proceed (struct gdbarch *gdbarch, int select_it);
> +extern void set_gdbarch_prepare_to_proceed (struct gdbarch *gdbarch, gdbarch_prepare_to_proceed_ftype *prepare_to_proceed);
> +#if GDB_MULTI_ARCH
> +#if (GDB_MULTI_ARCH > GDB_MULTI_ARCH_PARTIAL) || !defined (PREPARE_TO_PROCEED)
> +#define PREPARE_TO_PROCEED(select_it) (gdbarch_prepare_to_proceed (current_gdbarch, select_it))
> +#endif
> +#endif
> +
>  extern CORE_ADDR gdbarch_function_start_offset (struct gdbarch *gdbarch);
>  extern void set_gdbarch_function_start_offset (struct gdbarch *gdbarch, CORE_ADDR function_start_offset);
>  #if GDB_MULTI_ARCH
> Index: gdb/gdbarch.sh
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbarch.sh,v
> retrieving revision 1.59
> diff -c -p -u -p -r1.59 gdbarch.sh
> --- gdb/gdbarch.sh      2001/03/24 02:07:49     1.59
> +++ gdb/gdbarch.sh      2001/03/29 20:07:01
> @@ -489,6 +489,7 @@ f:2:BREAKPOINT_FROM_PC:unsigned char *:b
>  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
>  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: gdb/inferior.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/inferior.h,v
> retrieving revision 1.18
> diff -c -p -u -p -r1.18 inferior.h
> --- gdb/inferior.h      2001/03/13 23:31:13     1.18
> +++ gdb/inferior.h      2001/03/29 20:07:07
> @@ -261,6 +261,8 @@ extern int signal_print_update (int, int
> 
>  extern int signal_pass_update (int, int);
> 
> +extern void get_last_target_status(int *pid, struct target_waitstatus *status);
> +
>  /* From infcmd.c */
> 
>  extern void tty_command (char *, int);
> Index: gdb/infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.29
> diff -c -p -u -p -r1.29 infrun.c
> --- gdb/infrun.c        2001/03/22 23:58:37     1.29
> +++ gdb/infrun.c        2001/03/29 20:07:07
> @@ -406,6 +406,12 @@ static struct breakpoint *through_sigtra
>     currently be running in a syscall. */
>  static int number_of_threads_in_syscalls;
> 
> +/* This is a cached copy of the pid/waitstatus of the last event
> +   returned by target_wait()/target_wait_hook().  This information is
> +   returned by get_last_target_status(). */
> +static int target_last_wait_pid = -1;
> +static struct target_waitstatus target_last_waitstatus;
> +
>  /* This is used to remember when a fork, vfork or exec event
>     was caught by a catchpoint, and thus the event is to be
>     followed at the next resume of the inferior, and not
> @@ -1407,6 +1413,18 @@ check_for_old_step_resume_breakpoint (vo
>      warning ("GDB bug: infrun.c (wait_for_inferior): dropping old step_resume breakpoint");
>  }
> 
> +/* Return the cached copy of the last pid/waitstatus returned by
> +   target_wait()/target_wait_hook().  The data is actually cached by
> +   handle_inferior_event(), which gets called immediately after
> +   target_wait()/target_wait_hook().  */
> +
> +void
> +get_last_target_status(int *pid, struct target_waitstatus *status)
> +{
> +  *pid = target_last_wait_pid;
> +  *status = target_last_waitstatus;
> +}
> +
>  /* Given an execution control state that has been freshly filled in
>     by an event from the inferior, figure out what it means and take
>     appropriate action.  */
> @@ -1416,6 +1434,10 @@ handle_inferior_event (struct execution_
>  {
>    CORE_ADDR tmp;
>    int stepped_after_stopped_by_watchpoint;
> +
> +  /* Cache the last pid/waitstatus. */
> +  target_last_wait_pid = ecs->pid;
> +  target_last_waitstatus = *ecs->wp;
> 
>    /* Keep this extra brace for now, minimizes diffs.  */
>    {