[1/2] Eliminate target_have_continuable_watchpoint

Message ID 20180831151159.10583-2-palves@redhat.com
State New
Headers show
Series
  • continuable/steppable/non-steppable watchpoints
Related show

Commit Message

Pedro Alves Aug. 31, 2018, 3:11 p.m.
target_have_continuable_watchpoint isn't used anywhere so remove it.
The property isn't necessary because checking for "continuable" is the
same as checking for "!steppable && !non-steppable".

gdb/ChangeLog:
2018-08-31  Pedro Alves  <palves@redhat.com>

	* nto-procfs.c (nto_procfs_target::have_continuable_watchpoint):
	Delete.
	* s390-linux-nat.c
	(s390_linux_nat_target::have_continuable_watchpoint): Delete.
	* target.h (target_ops::have_continuable_watchpoint): Delete.
	(target_have_continuable_watchpoint): Delete.
	* x86-nat.h (x86_nat_target::have_continuable_watchpoint): Delete.
	* target-delegates.c: Regenerate.
---
 gdb/nto-procfs.c       |  3 ---
 gdb/s390-linux-nat.c   |  1 -
 gdb/target-delegates.c | 27 ---------------------------
 gdb/target.h           |  7 -------
 gdb/x86-nat.h          |  2 --
 5 files changed, 40 deletions(-)

-- 
2.14.4

Comments

Tom Tromey Aug. 31, 2018, 3:32 p.m. | #1
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> target_have_continuable_watchpoint isn't used anywhere so remove it.
Pedro> The property isn't necessary because checking for "continuable" is the
Pedro> same as checking for "!steppable && !non-steppable".

Pedro>  /* If the *_hw_beakpoint functions have not been defined
Pedro> diff --git a/gdb/x86-nat.h b/gdb/x86-nat.h
Pedro> index cc27fa43b4d..f6d8a26baf9 100644
Pedro> --- a/gdb/x86-nat.h
Pedro> +++ b/gdb/x86-nat.h
Pedro> @@ -76,8 +76,6 @@ struct x86_nat_target : public BaseTarget
Pedro>       the one that caused the trap.  Therefore we don't need to step
Pedro>       over it.  But we do need to reset the status register to avoid
Pedro>       another trap.  */
Pedro> -  bool have_continuable_watchpoint () override
Pedro> -  { return true; }

I think the comment just before this function should also be removed.

Tom
Pedro Alves Aug. 31, 2018, 3:40 p.m. | #2
On 08/31/2018 04:32 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> 

> Pedro> target_have_continuable_watchpoint isn't used anywhere so remove it.

> Pedro> The property isn't necessary because checking for "continuable" is the

> Pedro> same as checking for "!steppable && !non-steppable".

> 

> Pedro>  /* If the *_hw_beakpoint functions have not been defined

> Pedro> diff --git a/gdb/x86-nat.h b/gdb/x86-nat.h

> Pedro> index cc27fa43b4d..f6d8a26baf9 100644

> Pedro> --- a/gdb/x86-nat.h

> Pedro> +++ b/gdb/x86-nat.h

> Pedro> @@ -76,8 +76,6 @@ struct x86_nat_target : public BaseTarget

> Pedro>       the one that caused the trap.  Therefore we don't need to step

> Pedro>       over it.  But we do need to reset the status register to avoid

> Pedro>       another trap.  */

> Pedro> -  bool have_continuable_watchpoint () override

> Pedro> -  { return true; }

> 

> I think the comment just before this function should also be removed.


I thought it was still useful as is.  It's describing x86 watchpoints
generically enough before a set of functions implementing watchpoints
support that I thought that it's still positioned in a good place as
is.  WDYT?

Thanks,
Pedro Alves
Tom Tromey Aug. 31, 2018, 3:43 p.m. | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


>> I think the comment just before this function should also be removed.


Pedro> I thought it was still useful as is.  It's describing x86 watchpoints
Pedro> generically enough before a set of functions implementing watchpoints
Pedro> support that I thought that it's still positioned in a good place as
Pedro> is.  WDYT?

I looked at that but I didn't understand how the comment related to the
nearby code; like I didn't see anything else in that file about the
status register.  However I think it's a minor point and if it makes
sense to you it is fine.

Tom
Pedro Alves Aug. 31, 2018, 5:52 p.m. | #4
On 08/31/2018 04:43 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

> 

>>> I think the comment just before this function should also be removed.

> 

> Pedro> I thought it was still useful as is.  It's describing x86 watchpoints

> Pedro> generically enough before a set of functions implementing watchpoints

> Pedro> support that I thought that it's still positioned in a good place as

> Pedro> is.  WDYT?

> 

> I looked at that but I didn't understand how the comment related to the

> nearby code; like I didn't see anything else in that file about the

> status register.  However I think it's a minor point and if it makes

> sense to you it is fine.


Ah, I was looking more at the bit explaining that the watchpoints
are continuable, so don't need to override target_has_steppable_watchpoint.

I won't miss it myself, so if it isn't helping others, might as well
remove it.

I've done that before pushing the patch in.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 117edb161c3..54357af85b2 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -119,9 +119,6 @@  struct nto_procfs_target : public inf_child_target
 
   void interrupt () override;
 
-  bool have_continuable_watchpoint ()
-  { return true; }
-
   const char *extra_thread_info (struct thread_info *) override;
 
   char *pid_to_exec_file (int pid) override;
diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
index 2c605628105..7a04e854eb7 100644
--- a/gdb/s390-linux-nat.c
+++ b/gdb/s390-linux-nat.c
@@ -122,7 +122,6 @@  public:
   int remove_hw_breakpoint (struct gdbarch *, struct bp_target_info *)
     override;
   int region_ok_for_hw_watchpoint (CORE_ADDR, int) override;
-  bool have_continuable_watchpoint () override { return true; }
   bool stopped_by_watchpoint () override;
   int insert_watchpoint (CORE_ADDR, int, enum target_hw_bp_type,
 			 struct expression *) override;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 319a9bb46b4..03136dfacf2 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -36,7 +36,6 @@  struct dummy_target : public target_ops
   int remove_mask_watchpoint (CORE_ADDR arg0, CORE_ADDR arg1, enum target_hw_bp_type arg2) override;
   bool stopped_by_watchpoint () override;
   int have_steppable_watchpoint () override;
-  bool have_continuable_watchpoint () override;
   bool stopped_data_address (CORE_ADDR *arg0) override;
   bool watchpoint_addr_within_range (CORE_ADDR arg0, CORE_ADDR arg1, int arg2) override;
   int region_ok_for_hw_watchpoint (CORE_ADDR arg0, int arg1) override;
@@ -204,7 +203,6 @@  struct debug_target : public target_ops
   int remove_mask_watchpoint (CORE_ADDR arg0, CORE_ADDR arg1, enum target_hw_bp_type arg2) override;
   bool stopped_by_watchpoint () override;
   int have_steppable_watchpoint () override;
-  bool have_continuable_watchpoint () override;
   bool stopped_data_address (CORE_ADDR *arg0) override;
   bool watchpoint_addr_within_range (CORE_ADDR arg0, CORE_ADDR arg1, int arg2) override;
   int region_ok_for_hw_watchpoint (CORE_ADDR arg0, int arg1) override;
@@ -1016,31 +1014,6 @@  debug_target::have_steppable_watchpoint ()
   return result;
 }
 
-bool
-target_ops::have_continuable_watchpoint ()
-{
-  return this->beneath ()->have_continuable_watchpoint ();
-}
-
-bool
-dummy_target::have_continuable_watchpoint ()
-{
-  return false;
-}
-
-bool
-debug_target::have_continuable_watchpoint ()
-{
-  bool result;
-  fprintf_unfiltered (gdb_stdlog, "-> %s->have_continuable_watchpoint (...)\n", this->beneath ()->shortname ());
-  result = this->beneath ()->have_continuable_watchpoint ();
-  fprintf_unfiltered (gdb_stdlog, "<- %s->have_continuable_watchpoint (", this->beneath ()->shortname ());
-  fputs_unfiltered (") = ", gdb_stdlog);
-  target_debug_print_bool (result);
-  fputs_unfiltered ("\n", gdb_stdlog);
-  return result;
-}
-
 bool
 target_ops::stopped_data_address (CORE_ADDR *arg0)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 39aa8c3c730..b29d34b8a65 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -552,8 +552,6 @@  struct target_ops
       TARGET_DEFAULT_RETURN (false);
     virtual int have_steppable_watchpoint ()
       TARGET_DEFAULT_RETURN (false);
-    virtual bool have_continuable_watchpoint ()
-      TARGET_DEFAULT_RETURN (false);
     virtual bool stopped_data_address (CORE_ADDR *)
       TARGET_DEFAULT_RETURN (false);
     virtual bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int)
@@ -1933,11 +1931,6 @@  extern struct thread_info *target_thread_handle_to_thread_info
 #define target_have_steppable_watchpoint \
   (current_top_target ()->have_steppable_watchpoint ())
 
-/* Non-zero if we have continuable watchpoints  */
-
-#define target_have_continuable_watchpoint \
-  (current_top_target ()->have_continuable_watchpoint ())
-
 /* Provide defaults for hardware watchpoint functions.  */
 
 /* If the *_hw_beakpoint functions have not been defined
diff --git a/gdb/x86-nat.h b/gdb/x86-nat.h
index cc27fa43b4d..f6d8a26baf9 100644
--- a/gdb/x86-nat.h
+++ b/gdb/x86-nat.h
@@ -76,8 +76,6 @@  struct x86_nat_target : public BaseTarget
      the one that caused the trap.  Therefore we don't need to step
      over it.  But we do need to reset the status register to avoid
      another trap.  */
-  bool have_continuable_watchpoint () override
-  { return true; }
 
   int can_use_hw_breakpoint (enum bptype type, int cnt, int othertype) override
   { return x86_can_use_hw_breakpoint (type, cnt, othertype); }