[RFC/RFA] HPUX: Remove (non-useful?) definition of PREPARE_TO_PROCEED

[RFC/RFA] HPUX: Remove (non-useful?) definition of PREPARE_TO_PROCEED

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

Re: http://sources.redhat.com/ml/gdb-patches/2003-05/msg00487.html

Based on Daniel's hunch, I made the following change, and it tested ok
on our machine. I get the same set of failures for the threads tests:

    FAIL: gdb.threads/print-threads.exp: Running threads (fast) (timeout)
    ERROR: Delete all breakpoints in delete_breakpoints (timeout)
    FAIL: gdb.threads/print-threads.exp: break thread_function (2) (timeout)
    FAIL: gdb.threads/print-threads.exp: set var slow = 1 (timeout)
    FAIL: gdb.threads/print-threads.exp: Running threads (slow) (timeout)
    ERROR: Delete all breakpoints in delete_breakpoints (timeout)
    FAIL: gdb.threads/print-threads.exp: break thread_function (3) (timeout)
    FAIL: gdb.threads/print-threads.exp: set var slow = 1 (2) (timeout)
    FAIL: gdb.threads/print-threads.exp: break kill (timeout)
    FAIL: gdb.threads/print-threads.exp: Running threads (slow with kill breakpoint) (timeout)

In all honesty, I know very little about this, so it was just a
have-a-look kind of test. But based on the fact that this test confirms
Daniel's initial suspicion, I would think that this patch is safe to
apply. Since it will help doing some cleanup....

2003-05-26  J. Brobecker  <brobecker@gnat.com>

        * config/pa/nm-hppah.h (PREPARE_TO_PROCEED): Remove, no longer needed.
        * hppa-tdep.c (hppa_prepare_to_proceed): Likewise.

Shall I commit this patch?

Thanks, (and Thanks to Daniel for the suggestion)
-- 
Joel

[-- Attachment #2: ptp.diff --]
[-- Type: text/plain, Size: 4013 bytes --]

Index: config/pa/nm-hppah.h
===================================================================
RCS file: /cvs/src/src/gdb/config/pa/nm-hppah.h,v
retrieving revision 1.13
diff -u -p -r1.13 nm-hppah.h
--- config/pa/nm-hppah.h	5 May 2003 17:56:56 -0000	1.13
+++ config/pa/nm-hppah.h	26 May 2003 21:04:20 -0000
@@ -53,11 +53,6 @@
 #define CHILD_XFER_MEMORY
 #define CHILD_FOLLOW_FORK
 
-/* While this is for use by threaded programs, it doesn't appear
- * to hurt non-threaded ones.  This is used in infrun.c: */
-#define PREPARE_TO_PROCEED(select_it) hppa_prepare_to_proceed()
-extern int hppa_prepare_to_proceed (void);
-
 /* In infptrace.c or infttrace.c: */
 #define CHILD_PID_TO_EXEC_FILE
 #define CHILD_POST_STARTUP_INFERIOR
Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.79
diff -u -p -r1.79 hppa-tdep.c
--- hppa-tdep.c	22 May 2003 02:04:05 -0000	1.79
+++ hppa-tdep.c	26 May 2003 21:04:20 -0000
@@ -4673,92 +4673,6 @@ unwind_command (char *exp, int from_tty)
   pin (Total_frame_size);
 }
 
-#ifdef PREPARE_TO_PROCEED
-
-/* If the user has switched threads, and there is a breakpoint
-   at the old thread's pc location, then switch to that thread
-   and return TRUE, else return FALSE and don't do a thread
-   switch (or rather, don't seem to have done a thread switch).
-
-   Ptrace-based gdb will always return FALSE to the thread-switch
-   query, and thus also to PREPARE_TO_PROCEED.
-
-   The important thing is whether there is a BPT instruction,
-   not how many user breakpoints there are.  So we have to worry
-   about things like these:
-
-   o  Non-bp stop -- NO
-
-   o  User hits bp, no switch -- NO
-
-   o  User hits bp, switches threads -- YES
-
-   o  User hits bp, deletes bp, switches threads -- NO
-
-   o  User hits bp, deletes one of two or more bps
-   at that PC, user switches threads -- YES
-
-   o  Plus, since we're buffering events, the user may have hit a
-   breakpoint, deleted the breakpoint and then gotten another
-   hit on that same breakpoint on another thread which
-   actually hit before the delete. (FIXME in breakpoint.c
-   so that "dead" breakpoints are ignored?) -- NO
-
-   For these reasons, we have to violate information hiding and
-   call "breakpoint_here_p".  If core gdb thinks there is a bpt
-   here, that's what counts, as core gdb is the one which is
-   putting the BPT instruction in and taking it out.
-
-   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
-hppa_prepare_to_proceed (void)
-{
-  pid_t old_thread;
-  pid_t current_thread;
-
-  old_thread = hppa_switched_threads (PIDGET (inferior_ptid));
-  if (old_thread != 0)
-    {
-      /* Switched over from "old_thread".  Try to do
-         as little work as possible, 'cause mostly
-         we're going to switch back. */
-      CORE_ADDR new_pc;
-      CORE_ADDR old_pc = read_pc ();
-
-      /* Yuk, shouldn't use global to specify current
-         thread.  But that's how gdb does it. */
-      current_thread = PIDGET (inferior_ptid);
-      inferior_ptid = pid_to_ptid (old_thread);
-
-      new_pc = read_pc ();
-      if (new_pc != old_pc	/* If at same pc, no need */
-	  && breakpoint_here_p (new_pc))
-	{
-	  /* User hasn't deleted the BP.
-	     Return TRUE, finishing switch to "old_thread". */
-	  flush_cached_frames ();
-	  registers_changed ();
-#if 0
-	  printf ("---> PREPARE_TO_PROCEED (was %d, now %d)!\n",
-		  current_thread, PIDGET (inferior_ptid));
-#endif
-
-	  return 1;
-	}
-
-      /* Otherwise switch back to the user-chosen thread. */
-      inferior_ptid = pid_to_ptid (current_thread);
-      new_pc = read_pc ();	/* Re-prime register cache */
-    }
-
-  return 0;
-}
-#endif /* PREPARE_TO_PROCEED */
-
 void
 hppa_skip_permanent_breakpoint (void)
 {

Re: [RFC/RFA] HPUX: Remove (non-useful?) definition of PREPARE_TO_PROCEED

[Daniel Jacobowitz]

On Mon, May 26, 2003 at 02:15:58PM -0700, Joel Brobecker wrote:
> Re: http://sources.redhat.com/ml/gdb-patches/2003-05/msg00487.html
> 
> Based on Daniel's hunch, I made the following change, and it tested ok
> on our machine. I get the same set of failures for the threads tests:
> 
>     FAIL: gdb.threads/print-threads.exp: Running threads (fast) (timeout)
>     ERROR: Delete all breakpoints in delete_breakpoints (timeout)
>     FAIL: gdb.threads/print-threads.exp: break thread_function (2) (timeout)
>     FAIL: gdb.threads/print-threads.exp: set var slow = 1 (timeout)
>     FAIL: gdb.threads/print-threads.exp: Running threads (slow) (timeout)
>     ERROR: Delete all breakpoints in delete_breakpoints (timeout)
>     FAIL: gdb.threads/print-threads.exp: break thread_function (3) (timeout)
>     FAIL: gdb.threads/print-threads.exp: set var slow = 1 (2) (timeout)
>     FAIL: gdb.threads/print-threads.exp: break kill (timeout)
>     FAIL: gdb.threads/print-threads.exp: Running threads (slow with kill breakpoint) (timeout)
> 
> In all honesty, I know very little about this, so it was just a
> have-a-look kind of test. But based on the fact that this test confirms
> Daniel's initial suspicion, I would think that this patch is safe to
> apply. Since it will help doing some cleanup....

It won't actually show up in the testsuite.  I'm trying and trying to
remember which bug report I was working on when I developed the
prepare-to-proceed patch in the first place.  Also, removing the
definition means that you'll use default_prepare_to_proceed (which is
essentially useless) instead of the generic_prepare_to_proceed which
I'm proposing we use instead.

> 2003-05-26  J. Brobecker  <brobecker@gnat.com>
> 
>         * config/pa/nm-hppah.h (PREPARE_TO_PROCEED): Remove, no longer needed.
>         * hppa-tdep.c (hppa_prepare_to_proceed): Likewise.
> 
> Shall I commit this patch?
> 
> Thanks, (and Thanks to Daniel for the suggestion)

Would you do me the great favor of retesting with changing
PREPARE_TO_PROCEED to use generic_prepare_to_proceed instead of
hppa_prepare_to_proceed?  And if no one objects in a few days, commit
that.  Then the rest falls out, since you will have removed one of the
two remaining specializations of this method.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC/RFA] HPUX: Remove (non-useful?) definition of PREPARE_TO_PROCEED

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

> Would you do me the great favor of retesting with changing
> PREPARE_TO_PROCEED to use generic_prepare_to_proceed instead of
> hppa_prepare_to_proceed?  And if no one objects in a few days, commit
> that.  Then the rest falls out, since you will have removed one of the
> two remaining specializations of this method.

Sure. Here is a new patch, no new regressions. As suggested, will commit
in a few days, if noone objects.

2003-05-26  J. Brobecker  <brobecker@gnat.com>

        * config/pa/nm-hppah.h (PREPARE_TO_PROCEED): Use the generic
        prepare_to_proceed procedure instead of the hppa-specific one.
        * hppa-tdep.c (hppa_prepare_to_proceed): Remove, no longer used.

-- 
Joel

[-- Attachment #2: ptp.diff --]
[-- Type: text/plain, Size: 4428 bytes --]

Index: config/pa/nm-hppah.h
===================================================================
RCS file: /cvs/src/src/gdb/config/pa/nm-hppah.h,v
retrieving revision 1.13
diff -c -3 -p -r1.13 nm-hppah.h
*** config/pa/nm-hppah.h	5 May 2003 17:56:56 -0000	1.13
--- config/pa/nm-hppah.h	26 May 2003 23:15:35 -0000
***************
*** 55,62 ****
  
  /* While this is for use by threaded programs, it doesn't appear
   * to hurt non-threaded ones.  This is used in infrun.c: */
! #define PREPARE_TO_PROCEED(select_it) hppa_prepare_to_proceed()
! extern int hppa_prepare_to_proceed (void);
  
  /* In infptrace.c or infttrace.c: */
  #define CHILD_PID_TO_EXEC_FILE
--- 55,62 ----
  
  /* While this is for use by threaded programs, it doesn't appear
   * to hurt non-threaded ones.  This is used in infrun.c: */
! #define PREPARE_TO_PROCEED(select_it) generic_prepare_to_proceed(select_it)
! extern int generic_prepare_to_proceed (int select_it);
  
  /* In infptrace.c or infttrace.c: */
  #define CHILD_PID_TO_EXEC_FILE
Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.79
diff -c -3 -p -r1.79 hppa-tdep.c
*** hppa-tdep.c	22 May 2003 02:04:05 -0000	1.79
--- hppa-tdep.c	26 May 2003 23:15:32 -0000
*************** unwind_command (char *exp, int from_tty)
*** 4673,4764 ****
    pin (Total_frame_size);
  }
  
- #ifdef PREPARE_TO_PROCEED
- 
- /* If the user has switched threads, and there is a breakpoint
-    at the old thread's pc location, then switch to that thread
-    and return TRUE, else return FALSE and don't do a thread
-    switch (or rather, don't seem to have done a thread switch).
- 
-    Ptrace-based gdb will always return FALSE to the thread-switch
-    query, and thus also to PREPARE_TO_PROCEED.
- 
-    The important thing is whether there is a BPT instruction,
-    not how many user breakpoints there are.  So we have to worry
-    about things like these:
- 
-    o  Non-bp stop -- NO
- 
-    o  User hits bp, no switch -- NO
- 
-    o  User hits bp, switches threads -- YES
- 
-    o  User hits bp, deletes bp, switches threads -- NO
- 
-    o  User hits bp, deletes one of two or more bps
-    at that PC, user switches threads -- YES
- 
-    o  Plus, since we're buffering events, the user may have hit a
-    breakpoint, deleted the breakpoint and then gotten another
-    hit on that same breakpoint on another thread which
-    actually hit before the delete. (FIXME in breakpoint.c
-    so that "dead" breakpoints are ignored?) -- NO
- 
-    For these reasons, we have to violate information hiding and
-    call "breakpoint_here_p".  If core gdb thinks there is a bpt
-    here, that's what counts, as core gdb is the one which is
-    putting the BPT instruction in and taking it out.
- 
-    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
- hppa_prepare_to_proceed (void)
- {
-   pid_t old_thread;
-   pid_t current_thread;
- 
-   old_thread = hppa_switched_threads (PIDGET (inferior_ptid));
-   if (old_thread != 0)
-     {
-       /* Switched over from "old_thread".  Try to do
-          as little work as possible, 'cause mostly
-          we're going to switch back. */
-       CORE_ADDR new_pc;
-       CORE_ADDR old_pc = read_pc ();
- 
-       /* Yuk, shouldn't use global to specify current
-          thread.  But that's how gdb does it. */
-       current_thread = PIDGET (inferior_ptid);
-       inferior_ptid = pid_to_ptid (old_thread);
- 
-       new_pc = read_pc ();
-       if (new_pc != old_pc	/* If at same pc, no need */
- 	  && breakpoint_here_p (new_pc))
- 	{
- 	  /* User hasn't deleted the BP.
- 	     Return TRUE, finishing switch to "old_thread". */
- 	  flush_cached_frames ();
- 	  registers_changed ();
- #if 0
- 	  printf ("---> PREPARE_TO_PROCEED (was %d, now %d)!\n",
- 		  current_thread, PIDGET (inferior_ptid));
- #endif
- 
- 	  return 1;
- 	}
- 
-       /* Otherwise switch back to the user-chosen thread. */
-       inferior_ptid = pid_to_ptid (current_thread);
-       new_pc = read_pc ();	/* Re-prime register cache */
-     }
- 
-   return 0;
- }
- #endif /* PREPARE_TO_PROCEED */
- 
  void
  hppa_skip_permanent_breakpoint (void)
  {
--- 4673,4678 ----

Re: [RFC/RFA] HPUX: Remove (non-useful?) definition of PREPARE_TO_PROCEED

[Andrew Cagney]

> Would you do me the great favor of retesting with changing
>> PREPARE_TO_PROCEED to use generic_prepare_to_proceed instead of
>> hppa_prepare_to_proceed?  And if no one objects in a few days, commit
>> that.  Then the rest falls out, since you will have removed one of the
>> two remaining specializations of this method.
> 
> 
> Sure. Here is a new patch, no new regressions. As suggested, will commit
> in a few days, if noone objects.
> 
> 2003-05-26  J. Brobecker  <brobecker@gnat.com>
> 
>         * config/pa/nm-hppah.h (PREPARE_TO_PROCEED): Use the generic
>         prepare_to_proceed procedure instead of the hppa-specific one.
>         * hppa-tdep.c (hppa_prepare_to_proceed): Remove, no longer used.

I'm not going to object :-)

Andrew

Re: [RFC/RFA] HPUX: Remove (non-useful?) definition of PREPARE_TO_PROCEED

[Joel Brobecker]

> Sure. Here is a new patch, no new regressions. As suggested, will commit
> in a few days, if noone objects.
> 
> 2003-05-26  J. Brobecker  <brobecker@gnat.com>
> 
>         * config/pa/nm-hppah.h (PREPARE_TO_PROCEED): Use the generic
>         prepare_to_proceed procedure instead of the hppa-specific one.
>         * hppa-tdep.c (hppa_prepare_to_proceed): Remove, no longer used.

I finally committed this.

-- 
Joel