This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Fix watchpoint-at-epilogue 7.0 regression (s390x)


On Sat, 26 Dec 2009 06:35:55 +0100, Joel Brobecker wrote:
> > gdb/
> > 2009-12-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* breakpoint.c (watchpoint_check): Check the call
> > 	gdbarch_in_function_epilogue_p before calling frame_find_by_id.
> > 	Extend the comment.
> 
> This part looks OK to me.

> > gdb/testsuite/
> > 2009-12-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* gdb.base/watchpoint-cond-gone.exp, gdb.base/watchpoint-cond-gone.c,
> > 	gdb.base/watchpoint-cond-gone-stripped.c: New.
> 
> OK, with a few changes:
>   - The copyright notices needs to be GPL v3;

Done.

> > +    gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
> 
> We are now trying to get rid of gdb_suppress_entire_file. Can you just
> "return" instead?

Done.


> This reminded me that I wanted to start writing a GDB testcase writing
> cookbook.  Your testcase had a lot of stuff that I wanted to put in there,
> so I just finally started it!  Will send an email to gdb@ when I'm done...

http://sourceware.org/gdb/wiki/GDBTestcaseCookbook

Updated it now with a testcase template.  Unfortunately the GDB wiki always
hangs during check-in and also I never got any change notification.  Therefore
guessing the notifications do not work.


On Sun, 27 Dec 2009 20:59:05 +0100, Eli Zaretskii wrote:
> > --- a/gdb/config/djgpp/fnchange.lst
> > +++ b/gdb/config/djgpp/fnchange.lst
> 
> This is fine.


Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-01/msg00169.html

--- src/gdb/ChangeLog	2010/01/19 18:11:17	1.11276
+++ src/gdb/ChangeLog	2010/01/19 20:31:32	1.11277
@@ -1,3 +1,12 @@
+2010-01-19  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* breakpoint.c (watchpoint_check): Check the call
+	gdbarch_in_function_epilogue_p before calling frame_find_by_id.
+	Extend the comment.
+	* config/djgpp/fnchange.lst: Add translations for 
+	watchpoint-cond-gone.exp, watchpoint-cond-gone.c and
+	watchpoint-cond-gone-stripped.c.
+
 2010-01-19  Tom Tromey  <tromey@redhat.com>
 
 	PR c++/8000:
--- src/gdb/breakpoint.c	2010/01/19 09:39:11	1.450
+++ src/gdb/breakpoint.c	2010/01/19 20:31:34	1.451
@@ -3217,6 +3217,17 @@
       struct gdbarch *frame_arch = get_frame_arch (frame);
       CORE_ADDR frame_pc = get_frame_pc (frame);
 
+      /* in_function_epilogue_p() returns a non-zero value if we're still
+	 in the function but the stack frame has already been invalidated.
+	 Since we can't rely on the values of local variables after the
+	 stack has been destroyed, we are treating the watchpoint in that
+	 state as `not changed' without further checking.  Don't mark
+	 watchpoints as changed if the current frame is in an epilogue -
+	 even if they are in some other frame, our view of the stack
+	 is likely to be wrong and frame_find_by_id could error out.  */
+      if (gdbarch_in_function_epilogue_p (frame_arch, frame_pc))
+	return WP_VALUE_NOT_CHANGED;
+
       fr = frame_find_by_id (b->watchpoint_frame);
       within_current_scope = (fr != NULL);
 
@@ -3233,17 +3244,6 @@
 	    within_current_scope = 0;
 	}
 
-      /* in_function_epilogue_p() returns a non-zero value if we're still
-	 in the function but the stack frame has already been invalidated.
-	 Since we can't rely on the values of local variables after the
-	 stack has been destroyed, we are treating the watchpoint in that
-	 state as `not changed' without further checking.  Don't mark
-	 watchpoints as changed if the current frame is in an epilogue -
-	 even if they are in some other frame, our view of the stack
-	 is likely to be wrong.  */
-      if (gdbarch_in_function_epilogue_p (frame_arch, frame_pc))
-	return WP_VALUE_NOT_CHANGED;
-
       if (within_current_scope)
 	/* If we end up stopping, the current frame will get selected
 	   in normal_stop.  So this call to select_frame won't affect
--- src/gdb/config/djgpp/fnchange.lst	2010/01/01 06:53:34	1.106
+++ src/gdb/config/djgpp/fnchange.lst	2010/01/19 20:31:35	1.107
@@ -401,6 +401,9 @@
 @V@/gdb/testsuite/gdb.base/watchpoint-solib.c @V@/gdb/testsuite/gdb.base/wp-solib.c
 @V@/gdb/testsuite/gdb.base/watchpoint-hw.exp @V@/gdb/testsuite/gdb.base/wp-hw.exp
 @V@/gdb/testsuite/gdb.base/watchpoint-solib.exp @V@/gdb/testsuite/gdb.base/wp-solib.exp
+@V@/gdb/testsuite/gdb.base/watchpoint-cond-gone.exp @V@/gdb/testsuite/gdb.base/wpcondg.exp
+@V@/gdb/testsuite/gdb.base/watchpoint-cond-gone.c @V@/gdb/testsuite/gdb.base/wpcondg.c
+@V@/gdb/testsuite/gdb.base/watchpoint-cond-gone-stripped.c @V@/gdb/testsuite/gdb.base/wpcondgs.c
 @V@/gdb/testsuite/gdb.cell/coremaker-spu.c @V@/gdb/testsuite/gdb.cell/core-spu.c
 @V@/gdb/testsuite/gdb.cell/ea-cache-spu.c @V@/gdb/testsuite/gdb.cell/ea-spu.c
 @V@/gdb/testsuite/gdb.cell/mem-access-spu.c @V@/gdb/testsuite/gdb.cell/mem-spu.c
--- src/gdb/testsuite/ChangeLog	2010/01/19 18:11:19	1.2094
+++ src/gdb/testsuite/ChangeLog	2010/01/19 20:31:37	1.2095
@@ -1,3 +1,8 @@
+2010-01-19  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.base/watchpoint-cond-gone.exp, gdb.base/watchpoint-cond-gone.c,
+	gdb.base/watchpoint-cond-gone-stripped.c: New.
+
 2010-01-19  Tom Tromey  <tromey@redhat.com>
 
 	PR c++/8000:
--- src/gdb/testsuite/gdb.base/watchpoint-cond-gone-stripped.c
+++ src/gdb/testsuite/gdb.base/watchpoint-cond-gone-stripped.c	2010-01-19 20:47:03.774343000 +0000
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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/>.  */
+
+void
+jumper (void (*jumpto) (void))
+{
+  (*jumpto) ();
+}
--- src/gdb/testsuite/gdb.base/watchpoint-cond-gone.c
+++ src/gdb/testsuite/gdb.base/watchpoint-cond-gone.c	2010-01-19 20:47:04.302028000 +0000
@@ -0,0 +1,36 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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/>.  */
+
+extern void jumper (void (*jumpto) (void));
+
+static void
+func (void)
+{
+  volatile int c;
+
+  c = 5;
+  c = 10;	/* watchpoint-here */
+  c = 20;
+}
+
+int
+main (void)
+{
+  jumper (func);
+
+  return 0;
+}
--- src/gdb/testsuite/gdb.base/watchpoint-cond-gone.exp
+++ src/gdb/testsuite/gdb.base/watchpoint-cond-gone.exp	2010-01-19 20:47:04.789355000 +0000
@@ -0,0 +1,51 @@
+# Copyright 2010 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/>.
+
+set testfile "watchpoint-cond-gone"
+set srcfile ${testfile}.c
+set srcfilestripped ${testfile}-stripped.c
+set objfilestripped ${objdir}/${subdir}/${testfile}-stripped.o
+set binfile ${objdir}/${subdir}/${testfile}
+
+# We need to generate a function without DWARF to crash older GDB.
+# Stepping into a dynamic function trampoline or stepping out of MAIN may work
+# but it is not a reliable FAIL case.
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfilestripped}" "${objfilestripped}" object {}] != ""
+      || [gdb_compile "${srcdir}/${subdir}/${srcfile} ${objfilestripped}" "${binfile}" executable {debug}] != "" } {
+    untested watchpoint-cond-gone.exp
+    return -1
+}
+
+clean_restart ${testfile}
+
+# Problem does not occur otherwise.
+gdb_test "set can-use-hw-watchpoints 0"
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "watchpoint-here"]
+gdb_continue_to_breakpoint "Place to set the watchpoint"
+
+# The condition `c == 30' is the subject being tested.
+gdb_test "watch c if c == 30" "" "Place the watchpoint"
+
+# We may stay either in the function itself or only at the first instruction of
+# its caller depending on the epilogue unwinder (or valid epilogue CFI) presence.
+gdb_test "finish" \
+	 "Watchpoint .* deleted because the program has left the block in.*which its expression is valid..*in (jumper|func).*" \
+	 "Catch the no longer valid watchpoint"


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]