Re: [patch] Warn on constant value watchpoints

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Thu, 26 Jun 2008 16:27:31 +0200, Daniel Jacobowitz wrote:
> Also, I don't know how useful this feature is, but what happens with
> "watch foo()"?

Hmm, it crashes.  A loop of:

#7736 call_function_by_hand (function=0xc7bcb0, nargs=0, args=0x7fffffffc528) at infcall.c:727
#7737 evaluate_subexp_standard (expect_type=0x0, exp=0xcb2f90, pos=0x7fffffffc9dc, noside=EVAL_NORMAL) at eval.c:1296
#7738 evaluate_subexp (expect_type=0x0, exp=0xcb2f90, pos=0x7fffffffc9dc, noside=EVAL_NORMAL) at eval.c:75
#7739 evaluate_expression (exp=0xcb2f90) at eval.c:165
#7740 gdb_evaluate_expression (exp=0xcb2f90, value=0x7fffffffca68) at wrapper.c:47
#7741 fetch_watchpoint_value (exp=0xcb2f90, valp=0x7fffffffcb18, resultp=0x7fffffffcb10, val_chain=0x7fffffffcb20) at breakpoint.c:844
#7742 update_watchpoint (b=0xc741c0, reparse=0) at breakpoint.c:937
#7743 insert_breakpoints () at breakpoint.c:1265
#7744 proceed (addr=18446744073709551615, siggnal=TARGET_SIGNAL_DEFAULT, step=0) at infrun.c:1229


This patch is the one I propose to be possibly accepted.


Regards,
Jan

2008-07-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* infrun.c (decrement_int): New function.
	(proceed): Protect against looping from called INSERT_BREAKPOINTS.

2008-07-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/watchpoint.exp: Call TEST_WATCHPOINT_CALLING_INFERIOR.
	(test_watchpoint_calling_inferior): New function.

--- ./gdb/infrun.c	8 Jul 2008 10:31:16 -0000	1.285
+++ ./gdb/infrun.c	9 Jul 2008 08:13:28 -0000
@@ -1146,6 +1146,15 @@ prepare_to_proceed (int step)
    over calls to it and cleared when the inferior is started.  */
 static CORE_ADDR prev_pc;
 
+/* MAKE_CLEANUP stub.  */
+static void
+decrement_int (void *arg)
+{
+  int *int_pointer = arg;
+
+  (*int_pointer)--;
+}
+
 /* Basic routine for continuing the program in various fashions.
 
    ADDR is the address to resume at, or -1 for resume where stopped.
@@ -1226,7 +1235,20 @@ proceed (CORE_ADDR addr, enum target_sig
      or if we are stepping over one but we're using displaced stepping
      to do so.  */
   if (! stepping_over_breakpoint || use_displaced_stepping (gdbarch))
-    insert_breakpoints ();
+    {
+      struct cleanup *old_cleanups;
+      static int nested;
+
+      /* `watch func()' will nest as it will try to PROCEED to execute inferior
+	 FUNC to complete UPDATE_WATCHPOINT.  */
+      if (nested)
+	error (_("Aborting an inferior call used from a watchpoint"));
+
+      old_cleanups = make_cleanup (decrement_int, &nested);
+      nested++;
+      insert_breakpoints ();
+      do_cleanups (old_cleanups);
+    }
 
   if (siggnal != TARGET_SIGNAL_DEFAULT)
     stop_signal = siggnal;
--- ./gdb/testsuite/gdb.base/watchpoint.exp	15 Apr 2008 14:33:54 -0000	1.18
+++ ./gdb/testsuite/gdb.base/watchpoint.exp	9 Jul 2008 08:14:07 -0000
@@ -644,6 +644,33 @@ proc test_watchpoint_and_breakpoint {} {
 	}
     }
 }
+
+# Test a deadlock on an inferior call from a watchpoint.
+
+proc test_watchpoint_calling_inferior {} {
+    if [runto func3] then {
+	# Formerly GDB would deadlock while slowly allocating memory/stack.
+	global timeout
+	set prev_timeout $timeout
+	set timeout 10
+	verbose "Timeout now 10 sec.\n"
+
+	# Only hardware watchpoints cause the problem (in INSERT_BREAKPOINTS).
+	gdb_test "set can-use-hw-watchpoints 1" "" \
+	         "Enable hw-watchpoint for a watchpoint with an inferior call"
+
+	gdb_test "watch func1()" "atchpoint \[0-9\]+: func1 \\(\\)" \
+		 "Put a watchpoint with an inferior call"
+	gdb_breakpoint "func4"
+	# Either abort with `Aborting an inferior call used from a watchpoint'
+	# or it may get somehow resolved but we must get back a prompt.
+	gdb_test "continue" "" "Check the watchpoint with an inferior call"
+
+	# Restore old timeout
+	set timeout $prev_timeout
+	verbose "Timeout now $timeout sec.\n"
+    }
+}
     
 proc test_inaccessible_watchpoint {} {
     global gdb_prompt
@@ -833,6 +861,17 @@ if [initialize] then {
     }
 
     test_watchpoint_and_breakpoint
+
+    # See above.
+    if [istarget "mips-idt-*"] then {
+	gdb_exit
+	gdb_start
+	gdb_reinitialize_dir $srcdir/$subdir
+	gdb_load $binfile
+	initialize
+    }
+
+    test_watchpoint_calling_inferior
 }
 
 # Restore old timeout