[RFC, RFA] multi-arch PREPARE_TO_PROCEED()

[RFC, RFA] multi-arch PREPARE_TO_PROCEED()

[David Smith]

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)
From deephan@erols.com Thu Mar 29 21:47:00 2001
From: David Deephanphongs <deephan@erols.com>
To: gdb-patches@sources.redhat.com
Subject: [PATCH] specify arguments to debugee from commandline
Date: Thu, 29 Mar 2001 21:47:00 -0000
Message-id: <20010330005457.A21793@llamedos.org>
X-SW-Source: 2001-03/msg00542.html
Content-length: 4638

This patch will add the option --args=<args> to gdb.
It assumes that <args> is one argument (i.e., is in quotes, or has the
spaces escaped).
--args acts just like calling 'set args <args>' does from the gdb prompt:
gdb --args '--color=never -l gdb' /bin/ls
is equivalent to:
gdb /bin/ls
gdb> set args --color=never -l gdb

It also adds a description of --args when 'gdb -h' is run,
and adds a description of --args to the manpage.

The patch is against the 20010327 snapshot.

======= Changelog =========
2001-03-30  David Deephanphongs  <david@llamedos.org>

	* main.c:
	(set_inferior_args): Added extern prototype of set_inferior_args.
	(captured_main): Add --args option to gdb.
	* infcmd.c:
	(inferior_args): initialize to zero.
	(_initialize_infcmd): only call set_inferior_args (xtrdup ("")) 
	if inferior_args is set to zero.
	* gdb.1, main.c (print_gdb_help):
	Document --args option

====== Patch =======
diff -c3p gdb-orig/gdb.1 gdb/gdb.1
*** gdb-orig/gdb.1	Tue Mar  6 03:21:07 2001
--- gdb/gdb.1	Fri Mar 30 00:22:00 2001
*************** gdb \- The GNU Debugger
*** 38,43 ****
--- 38,46 ----
  .RB "[\|" "\-d "\c
  .I dir\c
  \&\|]
+ .RB "[\|" "\-args "\c
+ .I args\c
+ \&\|]
  .RB "[\|" \c
  .I prog\c
  .RB "[\|" \c
*************** Execute GDB commands from file \c
*** 264,269 ****
--- 267,280 ----
  Add \c
  .I directory\c
  \& to the path to search for source files.
+ 
+ 
+ .TP
+ .BI "\-args=" "args"\c
+ \&
+ Set \c
+ .I args\c
+ \& as the default arguments for the program to be debugged.
  .PP
  
  .TP
diff -c3p gdb-orig/infcmd.c gdb/infcmd.c
*** gdb-orig/infcmd.c	Wed Mar 21 11:42:38 2001
--- gdb/infcmd.c	Thu Mar 29 23:27:07 2001
*************** static void breakpoint_auto_delete_conte
*** 121,127 ****
  /* String containing arguments to give to the program, separated by spaces.
     Empty string (pointer to '\0') means no args.  */
  
! static char *inferior_args;
  
  /* File name for default use for standard in/out in the inferior.  */
  
--- 121,127 ----
  /* String containing arguments to give to the program, separated by spaces.
     Empty string (pointer to '\0') means no args.  */
  
! static char *inferior_args = 0;
  
  /* File name for default use for standard in/out in the inferior.  */
  
*************** Register name as argument means describe
*** 1967,1973 ****
    add_info ("float", float_info,
  	    "Print the status of the floating point unit\n");
  
!   set_inferior_args (xstrdup (""));	/* Initially no args */
    inferior_environ = make_environ ();
    init_environ (inferior_environ);
  }
--- 1967,1974 ----
    add_info ("float", float_info,
  	    "Print the status of the floating point unit\n");
  
!   if (inferior_args == 0)
! 	set_inferior_args (xstrdup (""));	/* Initially no args */
    inferior_environ = make_environ ();
    init_environ (inferior_environ);
  }
diff -c3p gdb-orig/main.c gdb/main.c
*** gdb-orig/main.c	Tue Mar  6 03:21:10 2001
--- gdb/main.c	Fri Mar 30 00:14:39 2001
*************** static void print_gdb_help (struct ui_fi
*** 89,94 ****
--- 89,98 ----
  extern int enable_external_editor;
  extern char *external_editor_command;
  
+ /* Used to set the arguments to the inferior program (i.e., the program that
+    is being debugged.) */
+ extern char *set_inferior_args (char *newargs);
+ 
  #ifdef __CYGWIN__
  #include <windows.h>		/* for MAX_PATH */
  #include <sys/cygwin.h>		/* for cygwin32_conv_to_posix_path */
*************** captured_main (void *data)
*** 284,289 ****
--- 288,294 ----
        {"windows", no_argument, &use_windows, 1},
        {"statistics", no_argument, 0, 13},
        {"write", no_argument, &write_files, 1},
+ 	  {"args", required_argument, 0, 14},
  /* Allow machine descriptions to add more options... */
  #ifdef ADDITIONAL_OPTIONS
        ADDITIONAL_OPTIONS
*************** captured_main (void *data)
*** 325,330 ****
--- 330,339 ----
  	    display_time = 1;
  	    display_space = 1;
  	    break;
+ 	  case 14:
+ 	    /* Set the arguments for the inferior program. */
+ 	    set_inferior_args ( xstrdup (optarg) );
+ 	    break;
  	  case 'f':
  	    annotation_level = 1;
  /* We have probably been invoked from emacs.  Disable window interface.  */
*************** Options:\n\n\
*** 824,829 ****
--- 833,841 ----
    -w                 Use a window interface.\n\
    --write            Set writing into executable and core files.\n\
    --xdb              XDB compatibility mode.\n\
+ ", stream);
+   fputs_unfiltered ("\
+   --args=args        Set arguments to give debugged program when it is run.\n\
  ", stream);
  #ifdef ADDITIONAL_OPTION_HELP
    fputs_unfiltered (ADDITIONAL_OPTION_HELP, stream);

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

[Andrew Cagney]

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.  */
>    {

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

[David Smith]

Andrew,

Thanks for taking the time to look at this patch.  See comments below.

Andrew Cagney wrote:

> David Smith wrote:
> 
> David two thoughts:
> 
> 	o	is the parameter ``select_it'' needed?

Not really, since the only call to PREPARE_TO_PROCEED (in infrun.c) always 
passes a 1.  However:

- This macro is already documented in gdbint.texinfo as taking the 
"select_it" parameter.

- If I change the call interface, the possibilities are much higher that I'll 
screw up the 4 existing implementations of PREPARE_TO_PROCEED.  Three of them 
(hppa-tdep.c, lin-lwp.c, linux-thread.c) should be easy enough to compile, 
but the last one, m3-nat.c, will not be easy.  I've searched and can't find a 
Mach3 system anywhere (and that port doesn't cross-compile).

I was trying not to affect the existing implementations.

> 
> 	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?

There is no logical reason why the generic_prepare_to_proceed() couldn't 
suffice for the 4 existing ports that define PREPARE_TO_PROCEED().  However, 
the actual implementation gets a bit tricky.

None of the 4 existing implementations (hppa-tdep.c, lin-lwp.c, 
linux-thread.c, m3-nat.c) are multi-arched.  The 4 existing implementations 
also are a bit odd.  The Mach3 (m3-nat.c) and HP/UX (hppa-tdep.c) 
implementations are quite odd in the way they figure out if the threads have 
been switched.  The two linux implementations (linux-thread.c and lin-lwp.c) 
are fairly normal, with the newest Linux implementation (lin-lwp.c) being 
pretty straight-forward, except for its integration with thread-db.c.

All 4 implementations switch threads in different ways.  Here's a bad ascii 
chart of what they do (and don't do).  The list of things to do I got from 
switch_to_thread() (in thread.c).

                         hppa-tdep.c   linux-thread.c   lin-lwp.c   m3-nat.c
Switch "inferior_pid"?      X                              X
Flush cached frames?        X                              X
Flush register data?        X                              X
Update "stop_pc"?
Select a new frame?

linux-thread.c just sets an internal variable (linuxthreads_step_pid) and 
then does some magic with that variable in linuxthreads_resume().  m3-nat.c 
calls a Mach3 specific switch_to_thread() function, which doesn't change 
inferior_pid at all (?).

> 
> 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
> 

I think I've read that the HP/UX port compiles, but can't actually debug 
programs, so testing that change would be quite difficult...

-- 
David Smith
dsmith@redhat.com
Red Hat, Inc.
http://www.redhat.com
256.704.9222 (direct)
256.837.3839 (fax)

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

[Andrew Cagney]

David Smith wrote:

>                          hppa-tdep.c   linux-thread.c   lin-lwp.c   m3-nat.c
> Switch "inferior_pid"?      X                              X
> Flush cached frames?        X                              X
> Flush register data?        X                              X
> Update "stop_pc"?
> Select a new frame?
> 
> linux-thread.c just sets an internal variable (linuxthreads_step_pid) and
> then does some magic with that variable in linuxthreads_resume().  m3-nat.c
> calls a Mach3 specific switch_to_thread() function, which doesn't change
> inferior_pid at all (?).

Yes, ok.  Can of worms... Ulgh.  I'd check things in as they are.
Perhaphs just add a comment to each of the existing implementations
suggesting that they are potentially redundant.

	Andrew

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

[David Smith]

Andrew,

Thanks for the response.  I've checked my patch in and added a comment to the 
4 existing implementations as you suggested.

Andrew Cagney wrote:

> David Smith wrote:
> 
> 
>>                          hppa-tdep.c   linux-thread.c   lin-lwp.c   m3-nat.c
>> Switch "inferior_pid"?      X                              X
>> Flush cached frames?        X                              X
>> Flush register data?        X                              X
>> Update "stop_pc"?
>> Select a new frame?
>> 
>> linux-thread.c just sets an internal variable (linuxthreads_step_pid) and
>> then does some magic with that variable in linuxthreads_resume().  m3-nat.c
>> calls a Mach3 specific switch_to_thread() function, which doesn't change
>> inferior_pid at all (?).
> 
> 
> Yes, ok.  Can of worms... Ulgh.  I'd check things in as they are.
> Perhaphs just add a comment to each of the existing implementations
> suggesting that they are potentially redundant.
> 
> 	Andrew



-- 
David Smith
dsmith@redhat.com
Red Hat, Inc.
http://www.redhat.com
256.704.9222 (direct)
256.837.3839 (fax)