From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cagney To: David Smith Cc: GDB Patches Subject: Re: [RFC, RFA] multi-arch PREPARE_TO_PROCEED() Date: Fri, 30 Mar 2001 12:16:00 -0000 Message-id: <3AC4E993.7B23D144@cygnus.com> References: <3AC39D32.8090709@redhat.com> X-SW-Source: 2001-03/msg00559.html 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 > > * 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. */ > {