[COMMIT] HP-UX hardware watchpoint fallout

[COMMIT] HP-UX hardware watchpoint fallout

[Mark Kettenis]

The code dealing with disabling "hardware" watchpoints in systems
calls on HP-UX now lives where it belongs in inf-ttrace.c.  The
attached patch removes the remaining junk from core GDB.

Committed as obvious,

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* target.h (TARGET_DISABLE_HW_WATCHPOINTS)
	(TARGET_ENABLE_HW_WATCHPOINTS): Remove macros.
	* infrun.c (struct execution_control_state): Remove
	enable_hw_watchpoints_after_wait member.
	(number_of_threads_in_syscalls): Remove variable.
	(init_wait_for_inferior): Don't initialize
	number_of_threads_in_syscalls.
	(init_execution_control_state): Don't initialize
	ECS->enable_hw_watchpoints_after_wait.
	(handle_inferior_event): Never invoke TARGET_ENABLE_HW_WATCHPOINTS
	or TARGET_DISABLE_HW_WATCHPOINTS.

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.184
diff -u -p -r1.184 infrun.c
--- infrun.c 3 Dec 2004 23:59:53 -0000 1.184
+++ infrun.c 24 Dec 2004 22:45:34 -0000
@@ -280,14 +280,6 @@ static int stop_print_frame;
 
 static struct breakpoint *step_resume_breakpoint = NULL;
 
-/* On some platforms (e.g., HP-UX), hardware watchpoints have bad
-   interactions with an inferior that is running a kernel function
-   (aka, a system call or "syscall").  wait_for_inferior therefore
-   may have a need to know when the inferior is in a syscall.  This
-   is a count of the number of inferior threads which are known to
-   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()/deprecated_target_wait_hook().  This
    information is returned by get_last_target_status().  */
@@ -854,9 +846,6 @@ init_wait_for_inferior (void)
   /* The first resume is not following a fork/vfork/exec. */
   pending_follow.kind = TARGET_WAITKIND_SPURIOUS;	/* I.e., none. */
 
-  /* See wait_for_inferior's handling of SYSCALL_ENTRY/RETURN events. */
-  number_of_threads_in_syscalls = 0;
-
   clear_proceed_status ();
 
   stepping_past_singlestep_breakpoint = 0;
@@ -913,7 +902,6 @@ struct execution_control_state
   int step_after_step_resume_breakpoint;
   int stepping_through_solib_after_catch;
   bpstat stepping_through_solib_catchpoints;
-  int enable_hw_watchpoints_after_wait;
   int new_thread_event;
   struct target_waitstatus tmpstatus;
   enum infwait_states infwait_state;
@@ -1066,7 +1054,6 @@ init_execution_control_state (struct exe
   ecs->handling_longjmp = 0;	/* FIXME */
   ecs->stepping_through_solib_after_catch = 0;
   ecs->stepping_through_solib_catchpoints = NULL;
-  ecs->enable_hw_watchpoints_after_wait = 0;
   ecs->sal = find_pc_line (prev_pc, 0);
   ecs->current_line = ecs->sal.line;
   ecs->current_symtab = ecs->sal.symtab;
@@ -1239,26 +1226,11 @@ handle_inferior_event (struct execution_
         printf_unfiltered ("infrun: infwait_thread_hop_state\n");
       /* Cancel the waiton_ptid. */
       ecs->waiton_ptid = pid_to_ptid (-1);
-      /* See comments where a TARGET_WAITKIND_SYSCALL_RETURN event
-         is serviced in this loop, below. */
-      if (ecs->enable_hw_watchpoints_after_wait)
-	{
-	  TARGET_ENABLE_HW_WATCHPOINTS (PIDGET (inferior_ptid));
-	  ecs->enable_hw_watchpoints_after_wait = 0;
-	}
-      stepped_after_stopped_by_watchpoint = 0;
       break;
 
     case infwait_normal_state:
       if (debug_infrun)
         printf_unfiltered ("infrun: infwait_normal_state\n");
-      /* See comments where a TARGET_WAITKIND_SYSCALL_RETURN event
-         is serviced in this loop, below. */
-      if (ecs->enable_hw_watchpoints_after_wait)
-	{
-	  TARGET_ENABLE_HW_WATCHPOINTS (PIDGET (inferior_ptid));
-	  ecs->enable_hw_watchpoints_after_wait = 0;
-	}
       stepped_after_stopped_by_watchpoint = 0;
       break;
 
@@ -1467,31 +1439,11 @@ handle_inferior_event (struct execution_
 	}
       goto process_event_stop_test;
 
-      /* These syscall events are returned on HP-UX, as part of its
-         implementation of page-protection-based "hardware" watchpoints.
-         HP-UX has unfortunate interactions between page-protections and
-         some system calls.  Our solution is to disable hardware watches
-         when a system call is entered, and reenable them when the syscall
-         completes.  The downside of this is that we may miss the precise
-         point at which a watched piece of memory is modified.  "Oh well."
-
-         Note that we may have multiple threads running, which may each
-         enter syscalls at roughly the same time.  Since we don't have a
-         good notion currently of whether a watched piece of memory is
-         thread-private, we'd best not have any page-protections active
-         when any thread is in a syscall.  Thus, we only want to reenable
-         hardware watches when no threads are in a syscall.
-
-         Also, be careful not to try to gather much state about a thread
-         that's in a syscall.  It's frequently a losing proposition. */
+      /* Be careful not to try to gather much state about a thread
+         that's in a syscall.  It's frequently a losing proposition.  */
     case TARGET_WAITKIND_SYSCALL_ENTRY:
       if (debug_infrun)
         printf_unfiltered ("infrun: TARGET_WAITKIND_SYSCALL_ENTRY\n");
-      number_of_threads_in_syscalls++;
-      if (number_of_threads_in_syscalls == 1)
-	{
-	  TARGET_DISABLE_HW_WATCHPOINTS (PIDGET (inferior_ptid));
-	}
       resume (0, TARGET_SIGNAL_0);
       prepare_to_wait (ecs);
       return;
@@ -1500,27 +1452,11 @@ handle_inferior_event (struct execution_
          get it entirely out of the syscall.  (We get notice of the
          event when the thread is just on the verge of exiting a
          syscall.  Stepping one instruction seems to get it back
-         into user code.)
-
-         Note that although the logical place to reenable h/w watches
-         is here, we cannot.  We cannot reenable them before stepping
-         the thread (this causes the next wait on the thread to hang).
-
-         Nor can we enable them after stepping until we've done a wait.
-         Thus, we simply set the flag ecs->enable_hw_watchpoints_after_wait
-         here, which will be serviced immediately after the target
-         is waited on. */
+         into user code.)  */
     case TARGET_WAITKIND_SYSCALL_RETURN:
       if (debug_infrun)
         printf_unfiltered ("infrun: TARGET_WAITKIND_SYSCALL_RETURN\n");
       target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);
-
-      if (number_of_threads_in_syscalls > 0)
-	{
-	  number_of_threads_in_syscalls--;
-	  ecs->enable_hw_watchpoints_after_wait =
-	    (number_of_threads_in_syscalls == 0);
-	}
       prepare_to_wait (ecs);
       return;
 
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.65
diff -u -p -r1.65 target.h
--- target.h 8 Oct 2004 20:29:55 -0000 1.65
+++ target.h 24 Dec 2004 22:45:35 -0000
@@ -1017,18 +1017,6 @@ extern void (*deprecated_target_new_objf
    (current_target.to_have_continuable_watchpoint)
 #endif
 
-/* HP-UX supplies these operations, which respectively disable and enable
-   the memory page-protections that are used to implement hardware watchpoints
-   on that platform.  See wait_for_inferior's use of these.  */
-
-#if !defined(TARGET_DISABLE_HW_WATCHPOINTS)
-#define TARGET_DISABLE_HW_WATCHPOINTS(pid)
-#endif
-
-#if !defined(TARGET_ENABLE_HW_WATCHPOINTS)
-#define TARGET_ENABLE_HW_WATCHPOINTS(pid)
-#endif
-
 /* Provide defaults for hardware watchpoint functions.  */
 
 /* If the *_hw_beakpoint functions have not been defined

Re: [COMMIT] HP-UX hardware watchpoint fallout

[Daniel Jacobowitz]

On Fri, Dec 24, 2004 at 11:58:14PM +0100, Mark Kettenis wrote:
> The code dealing with disabling "hardware" watchpoints in systems
> calls on HP-UX now lives where it belongs in inf-ttrace.c.  The
> attached patch removes the remaining junk from core GDB.
> 
> Committed as obvious,

You, Mark, are my hero.  This is great!

-- 
Daniel Jacobowitz

Re: [COMMIT] HP-UX hardware watchpoint fallout

[Eli Zaretskii]

> Date: Fri, 24 Dec 2004 23:58:14 +0100 (CET)
> From: Mark Kettenis <kettenis@gnu.org>
> 
> from  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* target.h (TARGET_DISABLE_HW_WATCHPOINTS)
> 	(TARGET_ENABLE_HW_WATCHPOINTS): Remove macros.

Thanks.

Please also remove their documentation from gdbint.texinfo.  (In
general, when a macro is removed from target.h, one should search
gdbint.texinfo for that macro.)

TIA

Re: [COMMIT] HP-UX hardware watchpoint fallout

[Mark Kettenis]

   Date: Sat, 25 Dec 2004 12:01:50 +0200
   From: "Eli Zaretskii" <eliz@gnu.org>

   > Date: Fri, 24 Dec 2004 23:58:14 +0100 (CET)
   > From: Mark Kettenis <kettenis@gnu.org>
   > 
   > from  Mark Kettenis  <kettenis@gnu.org>
   > 
   > 	* target.h (TARGET_DISABLE_HW_WATCHPOINTS)
   > 	(TARGET_ENABLE_HW_WATCHPOINTS): Remove macros.

   Thanks.

   Please also remove their documentation from gdbint.texinfo.  (In
   general, when a macro is removed from target.h, one should search
   gdbint.texinfo for that macro.)

Oops, I forgot to commit that part.  Here it is.  Guess it's obvious,
so it's committed now.

Mark

P.S. Still working on that general description about implementing
hardware watchpoints using memory protections.


Index: doc/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
retrieving revision 1.464
diff -u -p -r1.464 ChangeLog
--- doc/ChangeLog	8 Dec 2004 05:28:31 -0000	1.464
+++ doc/ChangeLog	25 Dec 2004 10:15:54 -0000
@@ -1,3 +1,8 @@
+2004-12-24  Mark Kettenis  <kettenis@gnu.org>
+
+	* gdbint.texinfo (Algorithms): Remove description of
+	TARGET_DISABLE_HW_WATCHPOINTS and TARGET_ENABLE_HW_WATCHPOINTS.
+
 2004-12-07  Jim Blandy  <jimb@redhat.com>
 
 	* gdb.texinfo (General Query Packets): Specify that thread ID's in
Index: doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.227
diff -u -p -r1.227 gdbint.texinfo
--- doc/gdbint.texinfo	4 Dec 2004 14:02:44 -0000	1.227
+++ doc/gdbint.texinfo	25 Dec 2004 10:16:01 -0000
@@ -440,20 +440,6 @@ whose size is @var{size}.  @value{GDBN} 
 fall-back, in case @code{TARGET_REGION_OK_FOR_HW_WATCHPOINT} is not
 defined.
 
-@findex TARGET_DISABLE_HW_WATCHPOINTS
-@item TARGET_DISABLE_HW_WATCHPOINTS (@var{pid})
-Disables watchpoints in the process identified by @var{pid}.  This is
-used, e.g., on HPPA-RISC machines running HP-UX, which provide
-operations to disable and enable the page-level memory protection that
-implements hardware watchpoints on that platform.
-
-@findex TARGET_ENABLE_HW_WATCHPOINTS
-@item TARGET_ENABLE_HW_WATCHPOINTS (@var{pid})
-Enables watchpoints in the process identified by @var{pid}.  This is
-used, e.g., on HPPA-RISC machines running HP-UX, which provide
-operations to disable and enable the page-level memory protection that
-implements hardware watchpoints on that platform.
-
 @cindex insert or remove hardware watchpoint
 @findex target_insert_watchpoint
 @findex target_remove_watchpoint