This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[rfc] Allow watchpoints on inaccessible memory
- From: Daniel Jacobowitz <drow at false dot org>
- To: gdb-patches at sourceware dot org
- Date: Tue, 21 Aug 2007 10:25:00 -0400
- Subject: [rfc] Allow watchpoints on inaccessible memory
Here's something I've been meaning to try for ages. Suppose you have
a global variable pointing to some dynamically allocated storage, and
you want to find writes into that storage. When the pointer isn't
initialized you can't even set the watchpoint:
#include <stdlib.h>
char *ptr;
int
main ()
{
ptr = malloc (65536);
ptr[32768] = 1;
}
(gdb) watch ptr[32768]
Cannot access memory at address 0x8000
When it is initialized you can, but if you re-run then GDB falls over:
(gdb) r
Starting program: /space/fsf/watch
Error in re-setting breakpoint 2:
Cannot access memory at address 0x8000
Cannot access memory at address 0x8000
This limitation is unnecessary. Hardware watchpoint registers work
by trapping actual write operations; they don't (usually anyway)
require access to the actual contents. If we treat "inaccessible" as
a sentinel value not equal to any other value, we can set the
watchpoint at any time.
(gdb) r
Starting program: /space/fsf/watch
Hardware watchpoint 1: ptr[32768]
Hardware watchpoint 1: ptr[32768]
Hardware watchpoint 1: ptr[32768]
Old value = <unknown>
New value = 0 '\0'
main () at watch.c:10
10 ptr[32768] = 1;
(gdb) c
Continuing.
Hardware watchpoint 1: ptr[32768]
Old value = 0 '\0'
New value = 1 '\001'
main () at watch.c:11
11 }
(gdb)
Continuing.
Program exited with code 020.
The first <unknown> -> 0 transition is the initialization of "ptr".
Suddenly we're not dereferencing NULL any more, we're looking at some
newly allocated storage. A hardware watchpoint on the pointer detects
this automatically. The 0 -> 1 transition then is obvious.
This is much nicer :-) Any opinions on this change? Also, does it
deserve a NEWS entry and should it go in 6.7?
--
Daniel Jacobowitz
CodeSourcery
2007-08-21 Daniel Jacobowitz <dan@codesourcery.com>
* Makefile.in (breakpoint.o): Update.
* breakpoint.c (insert_bp_location): Handle exceptions while
evaluating and fetching the watchpoint expression. Always watch
the outermost value even if it is lazy.
(insert_breakpoints, remove_breakpoint, bpstat_stop_status)
(watch_command_1, can_use_hardware_watchpoint, breakpoint_re_set_one)
(do_enable_breakpoint): Likewise.
(watchpoint_check): Likewise. Report changes when a value becomes
readable or unreadable.
(watchpoint_value_print): New.
(print_it_typical): Use it. Do not free or clear old_val. Print
watchpoints even if old_val == NULL.
2007-08-21 Daniel Jacobowitz <dan@codesourcery.com>
* gdb.base/watchpoint.c (global_ptr, func4): New.
(main): Call func4.
* gdb.base/watchpoint.exp: Call test_inaccessible_watchpoint.
(test_inaccessible_watchpoint): New.
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.928
diff -u -p -r1.928 Makefile.in
--- Makefile.in 10 Aug 2007 17:52:09 -0000 1.928
+++ Makefile.in 21 Aug 2007 12:45:03 -0000
@@ -1853,7 +1853,7 @@ breakpoint.o: breakpoint.c $(defs_h) $(s
$(objfiles_h) $(source_h) $(linespec_h) $(completer_h) $(gdb_h) \
$(ui_out_h) $(cli_script_h) $(gdb_assert_h) $(block_h) $(solib_h) \
$(solist_h) $(observer_h) $(exceptions_h) $(gdb_events_h) \
- $(mi_common_h) $(memattr_h) $(ada_lang_h) $(top_h)
+ $(mi_common_h) $(memattr_h) $(ada_lang_h) $(top_h) $(wrapper_h)
bsd-kvm.o: bsd-kvm.c $(defs_h) $(cli_cmds_h) $(command_h) $(frame_h) \
$(regcache_h) $(target_h) $(value_h) $(gdbcore_h) $(gdb_assert_h) \
$(readline_h) $(bsd_kvm_h)
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.261
diff -u -p -r1.261 breakpoint.c
--- breakpoint.c 17 Aug 2007 17:06:02 -0000 1.261
+++ breakpoint.c 21 Aug 2007 13:18:28 -0000
@@ -56,6 +56,7 @@
#include "memattr.h"
#include "ada-lang.h"
#include "top.h"
+#include "wrapper.h"
#include "gdb-events.h"
#include "mi/mi-common.h"
@@ -1025,8 +1026,6 @@ Note: automatically using hardware break
convert this. */
int within_current_scope;
- struct value *mark = value_mark ();
- struct value *v;
struct frame_id saved_frame_id;
/* Save the current frame's ID so we can restore it after
@@ -1050,6 +1049,9 @@ Note: automatically using hardware break
if (within_current_scope)
{
+ struct value *mark = value_mark ();
+ struct value *v = NULL;
+
free_valchain (bpt);
/* Evaluate the expression and cut the chain of values
@@ -1058,9 +1060,12 @@ Note: automatically using hardware break
Make sure the value returned isn't lazy; we use
laziness to determine what memory GDB actually needed
in order to compute the value of the expression. */
- v = evaluate_expression (bpt->owner->exp);
- value_contents (v);
- value_release_to_mark (mark);
+ gdb_evaluate_expression (bpt->owner->exp, &v);
+ if (v != NULL)
+ {
+ gdb_value_fetch_lazy (v);
+ value_release_to_mark (mark);
+ }
bpt->owner->val_chain = v;
bpt->inserted = 1;
@@ -1072,7 +1077,7 @@ Note: automatically using hardware break
its contents to evaluate the expression, then we
must watch it. */
if (VALUE_LVAL (v) == lval_memory
- && ! value_lazy (v))
+ && (v == bpt->owner->val_chain || ! value_lazy (v)))
{
struct type *vtype = check_typedef (value_type (v));
@@ -1255,11 +1260,13 @@ insert_breakpoints (void)
if ((b->loc_type == bp_loc_hardware_watchpoint
|| b->owner->type == bp_watchpoint) && !b->owner->val)
{
- struct value *val;
- val = evaluate_expression (b->owner->exp);
- release_value (val);
- if (value_lazy (val))
- value_fetch_lazy (val);
+ struct value *val = NULL;
+ gdb_evaluate_expression (b->owner->exp, &val);
+ if (val != NULL)
+ {
+ gdb_value_fetch_lazy (val);
+ release_value (val);
+ }
b->owner->val = val;
}
@@ -1577,7 +1584,7 @@ remove_breakpoint (struct bp_location *b
/* For each memory reference remove the watchpoint
at that address. */
if (VALUE_LVAL (v) == lval_memory
- && ! value_lazy (v))
+ && (v == b->owner->val_chain || ! value_lazy (v)))
{
struct type *vtype = check_typedef (value_type (v));
@@ -2129,6 +2136,17 @@ top:
do_cleanups (old_chain);
}
+/* Print out the (old or new) value associated with a watchpoint. */
+
+static void
+watchpoint_value_print (struct value *val, struct ui_file *stream)
+{
+ if (val == NULL || value_lazy (val))
+ fprintf_unfiltered (stream, _("<unknown>"));
+ else
+ value_print (val, stream, 0, Val_pretty_default);
+}
+
/* This is the normal print function for a bpstat. In the future,
much of this logic could (should?) be moved to bpstat_stop_status,
by having it set different print_it values.
@@ -2305,26 +2323,21 @@ print_it_typical (bpstat bs)
case bp_watchpoint:
case bp_hardware_watchpoint:
- if (bs->old_val != NULL)
- {
- annotate_watchpoint (bs->breakpoint_at->number);
- if (ui_out_is_mi_like_p (uiout))
- ui_out_field_string
- (uiout, "reason",
- async_reason_lookup (EXEC_ASYNC_WATCHPOINT_TRIGGER));
- mention (bs->breakpoint_at);
- ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "value");
- ui_out_text (uiout, "\nOld value = ");
- value_print (bs->old_val, stb->stream, 0, Val_pretty_default);
- ui_out_field_stream (uiout, "old", stb);
- ui_out_text (uiout, "\nNew value = ");
- value_print (bs->breakpoint_at->val, stb->stream, 0, Val_pretty_default);
- ui_out_field_stream (uiout, "new", stb);
- do_cleanups (ui_out_chain);
- ui_out_text (uiout, "\n");
- value_free (bs->old_val);
- bs->old_val = NULL;
- }
+ annotate_watchpoint (bs->breakpoint_at->number);
+ if (ui_out_is_mi_like_p (uiout))
+ ui_out_field_string
+ (uiout, "reason",
+ async_reason_lookup (EXEC_ASYNC_WATCHPOINT_TRIGGER));
+ mention (bs->breakpoint_at);
+ ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "value");
+ ui_out_text (uiout, "\nOld value = ");
+ watchpoint_value_print (bs->old_val, stb->stream);
+ ui_out_field_stream (uiout, "old", stb);
+ ui_out_text (uiout, "\nNew value = ");
+ watchpoint_value_print (bs->breakpoint_at->val, stb->stream);
+ ui_out_field_stream (uiout, "new", stb);
+ do_cleanups (ui_out_chain);
+ ui_out_text (uiout, "\n");
/* More than one watchpoint may have been triggered. */
return PRINT_UNKNOWN;
break;
@@ -2337,7 +2350,7 @@ print_it_typical (bpstat bs)
mention (bs->breakpoint_at);
ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "value");
ui_out_text (uiout, "\nValue = ");
- value_print (bs->breakpoint_at->val, stb->stream, 0, Val_pretty_default);
+ watchpoint_value_print (bs->breakpoint_at->val, stb->stream);
ui_out_field_stream (uiout, "value", stb);
do_cleanups (ui_out_chain);
ui_out_text (uiout, "\n");
@@ -2345,7 +2358,7 @@ print_it_typical (bpstat bs)
break;
case bp_access_watchpoint:
- if (bs->old_val != NULL)
+ if (bs->old_val != NULL)
{
annotate_watchpoint (bs->breakpoint_at->number);
if (ui_out_is_mi_like_p (uiout))
@@ -2357,8 +2370,6 @@ print_it_typical (bpstat bs)
ui_out_text (uiout, "\nOld value = ");
value_print (bs->old_val, stb->stream, 0, Val_pretty_default);
ui_out_field_stream (uiout, "old", stb);
- value_free (bs->old_val);
- bs->old_val = NULL;
ui_out_text (uiout, "\nNew value = ");
}
else
@@ -2371,7 +2382,7 @@ print_it_typical (bpstat bs)
ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "value");
ui_out_text (uiout, "\nValue = ");
}
- value_print (bs->breakpoint_at->val, stb->stream, 0,Val_pretty_default);
+ watchpoint_value_print (bs->breakpoint_at->val, stb->stream);
ui_out_field_stream (uiout, "new", stb);
do_cleanups (ui_out_chain);
ui_out_text (uiout, "\n");
@@ -2590,11 +2601,23 @@ watchpoint_check (void *p)
we might be in the middle of evaluating a function call. */
struct value *mark = value_mark ();
- struct value *new_val = evaluate_expression (bs->breakpoint_at->exp);
- if (!value_equal (b->val, new_val))
+ struct value *new_val = NULL;
+ int old_valid, new_valid;
+
+ gdb_evaluate_expression (bs->breakpoint_at->exp, &new_val);
+ if (new_val != NULL)
+ gdb_value_fetch_lazy (new_val);
+
+ old_valid = (b->val != NULL && ! value_lazy (b->val));
+ new_valid = (new_val != NULL && ! value_lazy (new_val));
+ if (old_valid != new_valid
+ || (old_valid && !value_equal (b->val, new_val)))
{
- release_value (new_val);
- value_free_to_mark (mark);
+ if (new_val != NULL)
+ {
+ release_value (new_val);
+ value_free_to_mark (mark);
+ }
bs->old_val = b->val;
b->val = new_val;
/* We will stop here */
@@ -2824,7 +2847,7 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
for (v = b->val_chain; v; v = value_next (v))
{
if (VALUE_LVAL (v) == lval_memory
- && ! value_lazy (v))
+ && (v == b->val_chain || ! value_lazy (v)))
{
struct type *vtype = check_typedef (value_type (v));
@@ -5723,10 +5746,13 @@ watch_command_1 (char *arg, int accessfl
exp_end = arg;
exp_valid_block = innermost_block;
mark = value_mark ();
- val = evaluate_expression (exp);
- release_value (val);
- if (value_lazy (val))
- value_fetch_lazy (val);
+ val = NULL;
+ gdb_evaluate_expression (exp, &val);
+ if (val != NULL)
+ {
+ gdb_value_fetch_lazy (val);
+ release_value (val);
+ }
tok = arg;
while (*tok == ' ' || *tok == '\t')
@@ -5894,7 +5920,7 @@ can_use_hardware_watchpoint (struct valu
{
if (VALUE_LVAL (v) == lval_memory)
{
- if (value_lazy (v))
+ if (v != head && value_lazy (v))
/* A lazy memory lvalue is one that GDB never needed to fetch;
we either just used its address (e.g., `a' in `a.b') or
we never needed it at all (e.g., `a' in `a,b'). */
@@ -7319,10 +7345,13 @@ breakpoint_re_set_one (void *bint)
evaluate_expression. */
b->val = NULL;
}
- b->val = evaluate_expression (b->exp);
- release_value (b->val);
- if (value_lazy (b->val) && breakpoint_enabled (b))
- value_fetch_lazy (b->val);
+ gdb_evaluate_expression (b->exp, &b->val);
+ if (b->val)
+ {
+ release_value (b->val);
+ if (value_lazy (b->val) && breakpoint_enabled (b))
+ gdb_value_fetch_lazy (b->val);
+ }
if (b->cond_string != NULL)
{
@@ -7675,10 +7704,13 @@ is valid is not currently in scope.\n"),
value_free (bpt->val);
mark = value_mark ();
- bpt->val = evaluate_expression (bpt->exp);
- release_value (bpt->val);
- if (value_lazy (bpt->val))
- value_fetch_lazy (bpt->val);
+ bpt->val = NULL;
+ gdb_evaluate_expression (bpt->exp, &bpt->val);
+ if (bpt->val)
+ {
+ release_value (bpt->val);
+ gdb_value_fetch_lazy (bpt->val);
+ }
if (bpt->type == bp_hardware_watchpoint ||
bpt->type == bp_read_watchpoint ||
Index: testsuite/gdb.base/watchpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/watchpoint.c,v
retrieving revision 1.2
diff -u -p -r1.2 watchpoint.c
--- testsuite/gdb.base/watchpoint.c 17 Mar 2003 19:51:58 -0000 1.2
+++ testsuite/gdb.base/watchpoint.c 21 Aug 2007 14:23:20 -0000
@@ -39,6 +39,8 @@ struct foo struct1, struct2, *ptr1, *ptr
int doread = 0;
+char *global_ptr;
+
void marker1 ()
{
}
@@ -110,6 +112,14 @@ func1 ()
return 73;
}
+void
+func4 ()
+{
+ buf[0] = 3;
+ global_ptr = buf;
+ buf[0] = 7;
+}
+
int main ()
{
#ifdef usestubs
@@ -185,5 +195,7 @@ int main ()
func3 ();
+ func4 ();
+
return 0;
}
Index: testsuite/gdb.base/watchpoint.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/watchpoint.exp,v
retrieving revision 1.14
diff -u -p -r1.14 watchpoint.exp
--- testsuite/gdb.base/watchpoint.exp 2 Mar 2007 22:11:15 -0000 1.14
+++ testsuite/gdb.base/watchpoint.exp 21 Aug 2007 14:23:20 -0000
@@ -646,6 +646,30 @@ proc test_watchpoint_and_breakpoint {} {
}
}
+proc test_inaccessible_watchpoint {} {
+ global gdb_prompt
+
+ # This is a test for watchpoints on currently inaccessible (but later
+ # valid) memory.
+
+ if [runto func4] then {
+ gdb_test "watch *global_ptr" ".*atchpoint \[0-9\]+: \\*global_ptr"
+ gdb_test "next" ".*global_ptr = buf.*"
+ gdb_test_multiple "next" "next over ptr init" {
+ -re ".*atchpoint \[0-9\]+: \\*global_ptr\r\n\r\nOld value = .*\r\nNew value = 3 .*\r\n.*$gdb_prompt $" {
+ # We can not test for <unknown> here because NULL may be readable.
+ # This test does rely on *NULL != 3.
+ pass "next over ptr init"
+ }
+ }
+ gdb_test_multiple "next" "next over buffer set" {
+ -re ".*atchpoint \[0-9\]+: \\*global_ptr\r\n\r\nOld value = 3 .*\r\nNew value = 7 .*\r\n.*$gdb_prompt $" {
+ pass "next over buffer set"
+ }
+ }
+ }
+}
+
# Start with a fresh gdb.
gdb_exit
@@ -798,6 +822,8 @@ if [initialize] then {
}
}
+ test_inaccessible_watchpoint
+
# See above.
if [istarget "mips-idt-*"] then {
gdb_exit