[patch 3/4] Fix hw watchpoints: remove dead code

[patch 3/4] Fix hw watchpoints: remove dead code

[Jan Kratochvil]

Hi,

simple dead code removal.  Code would become invalid with the changes anyway.

The testcase is in fact unrelated, just I was surprised "hbreak" functionality
is currently not regression tested anywhere.


Thanks,
Jan


gdb/
2009-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* i386-nat.c (i386_stopped_by_hwbp): Remove.

gdb/testsuite/
2009-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/watchpoint-hw.exp: Use prepare_for_testing.  Fix test name
	for "untested".  New variable breakline.
	(hbreak, continue to break-at-exit after hbreak): New tests.

--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -590,27 +590,6 @@ i386_stopped_by_watchpoint (void)
   return target_stopped_data_address (&current_target, &addr);
 }
 
-/* Return non-zero if the inferior has some break/watchpoint that
-   triggered.  */
-
-static int
-i386_stopped_by_hwbp (void)
-{
-  int i;
-
-  dr_status_mirror = i386_dr_low.get_status ();
-  if (maint_show_dr)
-    i386_show_dr ("stopped_by_hwbp", 0, 0, hw_execute);
-
-  ALL_DEBUG_REGISTERS(i)
-    {
-      if (I386_DR_WATCH_HIT (i))
-	return 1;
-    }
-
-  return 0;
-}
-
 /* Insert a hardware-assisted breakpoint at BP_TGT->placed_address.
    Return 0 on success, EBUSY on failure.  */
 static int
--- a/gdb/testsuite/gdb.base/watchpoint-hw.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-hw.exp
@@ -21,19 +21,12 @@ if {(![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"]
     return
 }
 
-set testfile watchpoint-hw
-set srcfile ${testfile}.c
-set binfile ${objdir}/${subdir}/${testfile}
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
-    untested "Couldn't compile test program"
+set test watchpoint-hw
+set srcfile ${test}.c
+if { [prepare_for_testing ${test}.exp ${test} ${srcfile}] } {
     return -1
 }
 
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
-
 # Create the watchpoint before the inferior gets started.  Now the native CPU
 # target is still not active and its `to_can_use_hw_breakpoint' is not
 # installed, therefore only a software watchpoint gets created.
@@ -43,7 +36,7 @@ gdb_test "watch watchee" "atchpoint 1: watchee"
 # `runto_main' or `runto main' would delete the watchpoint created above.
 
 if { [gdb_start_cmd] < 0 } {
-    untested start
+    untested ${test}.exp
     return -1
 }
 gdb_test "" "main .* at .*" "start"
@@ -56,11 +49,27 @@ gdb_test "info watchpoints" "1 *hw watchpoint .* watchee"
 gdb_test "delete 1"
 gdb_test "rwatch watchee"
 
-gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+set breakline [gdb_get_line_number "break-at-exit"]
+gdb_breakpoint $breakline
 
 gdb_test "continue" "Continuing.\r\nHardware read watchpoint 3: watchee\r\n\r\nValue = 0\r\n.*"
 
 # Here should be no repeated notification of the read watchpoint.
 gdb_test "continue" \
 	 "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \
-	 "continue to break-at-exit"
+	 "continue to break-at-exit after rwatch"
+
+clean_restart ${test}
+
+if ![runto_main] {
+    untested ${test}.exp
+    return -1
+}
+
+gdb_test "hbreak ${srcfile}:${breakline}" \
+	 "Hardware assisted breakpoint 2 at 0x\[0-9a-f\]+: file .*${srcfile}, line ${breakline}\\." \
+	 "hbreak"
+
+gdb_test "continue" \
+	 "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \
+	 "continue to break-at-exit after hbreak"

[patch 3/4] Fix hw watchpoints: remove dead code [update]

[Jan Kratochvil]

Hi,

just a rediff on top of the patch 1/4.


Thanks,
Jan

------------------------------------------------------------------------------

simple dead code removal.  Code would become invalid with the changes anyway.

The testcase is in fact unrelated, just I was surprised "hbreak" functionality
is currently not regression tested anywhere.


gdb/
2009-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* i386-nat.c (i386_stopped_by_hwbp): Remove.

gdb/testsuite/
2009-10-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/watchpoint-hw.exp: New variable breakline.
	(hbreak, continue to break-at-exit after hbreak): New tests.

--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -593,27 +593,6 @@ i386_stopped_by_watchpoint (void)
   return target_stopped_data_address (&current_target, &addr);
 }
 
-/* Return non-zero if the inferior has some break/watchpoint that
-   triggered.  */
-
-static int
-i386_stopped_by_hwbp (void)
-{
-  int i;
-
-  dr_status_mirror = i386_dr_low.get_status ();
-  if (maint_show_dr)
-    i386_show_dr ("stopped_by_hwbp", 0, 0, hw_execute);
-
-  ALL_DEBUG_REGISTERS(i)
-    {
-      if (I386_DR_WATCH_HIT (i))
-	return 1;
-    }
-
-  return 0;
-}
-
 /* Insert a hardware-assisted breakpoint at BP_TGT->placed_address.
    Return 0 on success, EBUSY on failure.  */
 static int
--- a/gdb/testsuite/gdb.base/watchpoint-hw.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-hw.exp
@@ -53,11 +53,27 @@ if ![runto_main] {
 
 gdb_test "rwatch watchee"
 
-gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+set breakline [gdb_get_line_number "break-at-exit"]
+gdb_breakpoint $breakline
 
 gdb_test "continue" "Continuing.\r\nHardware read watchpoint \[0-9\]+: watchee\r\n\r\nValue = 0\r\n.*"
 
 # Here should be no repeated notification of the read watchpoint.
 gdb_test "continue" \
 	 "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \
-	 "continue to break-at-exit"
+	 "continue to break-at-exit after rwatch"
+
+clean_restart ${test}
+
+if ![runto_main] {
+    untested ${test}.exp
+    return -1
+}
+
+gdb_test "hbreak ${srcfile}:${breakline}" \
+	 "Hardware assisted breakpoint 2 at 0x\[0-9a-f\]+: file .*${srcfile}, line ${breakline}\\." \
+	 "hbreak"
+
+gdb_test "continue" \
+	 "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \
+	 "continue to break-at-exit after hbreak"

Re: [patch 3/4] Fix hw watchpoints: remove dead code [update]

[Joel Brobecker]

> 2009-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* i386-nat.c (i386_stopped_by_hwbp): Remove.

Thanks for taking care of that. From what I can tell, this function
has never been used ever since it was introduced in 2001! (for everyone's
interest, if you are wondering why we like git so much, git log -p --grep
gave me that answer within a couple of minutes:).

> gdb/testsuite/
> 2009-10-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/watchpoint-hw.exp: New variable breakline.
> 	(hbreak, continue to break-at-exit after hbreak): New tests.

This part is approved. The issue is that the testcase is dependent
on another patch which is still pending.  You can either merge that
patch into the one it depends on, but I'd rather have a separate
small testcase for hbreak.  That way, if hbreak alone breaks, we
only have to look at that, rather than start by isolating the hbreak
problem from the rest of the testcase.

-- 
Joel

Re: [patch 3/4] Fix hw watchpoints: remove dead code [update]

[Joel Brobecker]

> has never been used ever since it was introduced in 2001! (for everyone's
> interest, if you are wondering why we like git so much, git log -p --grep
> gave me that answer within a couple of minutes:).

I take that back, --grep only greps the log. That's too bad.

-- 
Joel

Re: [patch 3/4] Fix hw watchpoints: remove dead code [update]

[Jan Kratochvil]

On Wed, 07 Oct 2009 20:51:10 +0200, Joel Brobecker wrote:
> > 2009-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* i386-nat.c (i386_stopped_by_hwbp): Remove.
> 
> Thanks for taking care of that.

This part has been checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2009-10/msg00053.html

--- src/gdb/ChangeLog	2009/10/07 18:27:22	1.10940
+++ src/gdb/ChangeLog	2009/10/07 19:35:05	1.10941
@@ -1,4 +1,8 @@
-2009-10-09  Joel Brobecker  <brobecker@adacore.com>
+2009-10-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* i386-nat.c (i386_stopped_by_hwbp): Remove.
+
+2009-10-07  Joel Brobecker  <brobecker@adacore.com>
 
 	* NEWS: Update following the GDB 7.0 release.
 
--- src/gdb/i386-nat.c	2009/09/26 12:08:30	1.25
+++ src/gdb/i386-nat.c	2009/10/07 19:35:06	1.26
@@ -581,27 +581,6 @@
   return i386_stopped_data_address (&current_target, &addr);
 }
 
-/* Return non-zero if the inferior has some break/watchpoint that
-   triggered.  */
-
-static int
-i386_stopped_by_hwbp (void)
-{
-  int i;
-
-  dr_status_mirror = i386_dr_low.get_status ();
-  if (maint_show_dr)
-    i386_show_dr ("stopped_by_hwbp", 0, 0, hw_execute);
-
-  ALL_DEBUG_REGISTERS(i)
-    {
-      if (I386_DR_WATCH_HIT (i))
-	return 1;
-    }
-
-  return 0;
-}
-
 /* Insert a hardware-assisted breakpoint at BP_TGT->placed_address.
    Return 0 on success, EBUSY on failure.  */
 static int

Re: [patch 3/4] Fix hw watchpoints: remove dead code [update]

[Jan Kratochvil]

On Wed, 07 Oct 2009 20:51:10 +0200, Joel Brobecker wrote:
> > gdb/testsuite/
> > 2009-10-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* gdb.base/watchpoint-hw.exp: New variable breakline.
> > 	(hbreak, continue to break-at-exit after hbreak): New tests.
> 
> This part is approved. The issue is that the testcase is dependent
> on another patch which is still pending.  You can either merge that
> patch into the one it depends on, but I'd rather have a separate
> small testcase for hbreak.  That way, if hbreak alone breaks, we
> only have to look at that, rather than start by isolating the hbreak
> problem from the rest of the testcase.

Understood and yes, I agree.

Checked-in, I hope it is OK separate this way with no extra approval.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2009-10/msg00054.html

--- src/gdb/testsuite/ChangeLog	2009/10/07 18:17:32	1.1984
+++ src/gdb/testsuite/ChangeLog	2009/10/07 19:52:27	1.1985
@@ -1,3 +1,7 @@
+2009-10-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.base/hbreak.exp, gdb.base/hbreak.c: New.
+
 2009-10-07  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.base/watchpoints.c: Add copyright header.  Reformat one
--- src/gdb/testsuite/gdb.base/hbreak.c
+++ src/gdb/testsuite/gdb.base/hbreak.c	2009-10-07 19:52:48.705986000 +0000
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2009 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  volatile int dummy;
+
+  /* Stub lines are present as no breakpoints/watchpoints gets hit if current PC
+     already stays on the line PC while entering "step"/"continue".  */
+
+  dummy = 0;
+  dummy = 1;	/* break-at-exit */
+
+  return 0;
+}
--- src/gdb/testsuite/gdb.base/hbreak.exp
+++ src/gdb/testsuite/gdb.base/hbreak.exp	2009-10-07 19:52:49.270771000 +0000
@@ -0,0 +1,43 @@
+# Copyright 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Arch not supporting hw watchpoints does not imply no_hardware_watchpoints set.
+if {(![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"]
+     && ![istarget "ia64-*-*"] && ![istarget "s390*-*-*"])
+    || [target_info exists gdb,no_hardware_watchpoints]} then {
+    verbose "Skipping hbreak test."
+    return
+}
+
+set test hbreak
+set srcfile ${test}.c
+if { [prepare_for_testing ${test}.exp ${test} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    untested ${test}.exp
+    return -1
+}
+
+set breakline [gdb_get_line_number "break-at-exit"]
+
+gdb_test "hbreak ${srcfile}:${breakline}" \
+	 "Hardware assisted breakpoint \[0-9\]+ at 0x\[0-9a-f\]+: file .*${srcfile}, line ${breakline}\\." \
+	 "hbreak"
+
+gdb_test "continue" \
+	 "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \
+	 "continue to break-at-exit after hbreak"

Re: [patch 3/4] Fix hw watchpoints: remove dead code [update]

[Joel Brobecker]

> 2009-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* i386-nat.c (i386_stopped_by_hwbp): Remove.

Jan - sorry about this, but I just noticed that this function is still
documented in gdbint... Can you take care of removing it there as well?

Thank you,
-- 
Joel

Re: [patch 3/4] Fix hw watchpoints: remove dead code [update]

[Jan Kratochvil]

On Wed, 07 Oct 2009 23:51:04 +0200, Joel Brobecker wrote:
> > 2009-08-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* i386-nat.c (i386_stopped_by_hwbp): Remove.
> 
> Jan - sorry about this, but I just noticed that this function is still
> documented in gdbint... Can you take care of removing it there as well?

Checked-in.


Sorry,
Jan


http://sourceware.org/ml/gdb-cvs/2009-10/msg00057.html

--- src/gdb/doc/ChangeLog	2009/10/06 18:02:36	1.960
+++ src/gdb/doc/ChangeLog	2009/10/08 13:08:25	1.961
@@ -1,3 +1,7 @@
+2009-10-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdbint.texinfo (i386_stopped_by_hwbp): Remove.
+
 2009-10-06  Michael Eager  <eager@eagercon.com>
 
 	* gdb.texinfo (Contributors): Add self for Xilinx MicroBlaze.
--- src/gdb/doc/gdbint.texinfo	2009/09/13 16:28:29	1.312
+++ src/gdb/doc/gdbint.texinfo	2009/10/08 13:08:26	1.313
@@ -986,13 +986,6 @@
 hardware-assisted breakpoint always requires exactly one debug
 register.
 
-@findex i386_stopped_by_hwbp
-@item i386_stopped_by_hwbp (void)
-This function returns non-zero if the inferior has some watchpoint or
-hardware breakpoint that triggered.  It works like
-@code{i386_stopped_data_address}, except that it doesn't record the
-address whose watchpoint triggered.
-
 @findex i386_cleanup_dregs
 @item i386_cleanup_dregs (void)
 This function clears all the reference counts, addresses, and control