From fa4727a64f112282dc798744b98b0f39bd3458e8 Mon Sep 17 00:00:00 2001 From: Daniel Jacobowitz Date: Mon, 3 Mar 2008 13:24:12 +0000 Subject: [PATCH] * breakpoint.c (fetch_watchpoint_value): New function. (update_watchpoint): Set and clear val_valid. Use fetch_watchpoint_value. Handle unreadable values on the value chain. Correct check for user-requested array watchpoints. (breakpoint_init_inferior): Clear val_valid. (watchpoint_value_print): New function. (print_it_typical): Use it. Do not free or clear old_val. Print watchpoints even if old_val == NULL. (watchpoint_check): Use fetch_watchpoint_value. Check for values becoming readable or unreadable. (watch_command_1): Use fetch_watchpoint_value. Set val_valid. (do_enable_watchpoint): Likewise. * breakpoint.h (struct breakpoint): Update comment for val. Add val_valid. * NEWS: Mention watchpoints on inaccessible memory. * gdb.base/watchpoint.c (global_ptr, func4): New. (main): Call func4. * gdb.base/watchpoint.exp: Call test_inaccessible_watchpoint. (test_inaccessible_watchpoint): New. * gdb.texinfo (Set Watchpoints): Mention watchpoints on unreadable memory. Delete obsolete SPARClite reference. --- gdb/ChangeLog | 18 +++ gdb/NEWS | 3 + gdb/breakpoint.c | 188 +++++++++++++++++--------- gdb/breakpoint.h | 7 +- gdb/doc/ChangeLog | 5 + gdb/doc/gdb.texinfo | 21 ++- gdb/testsuite/ChangeLog | 7 + gdb/testsuite/gdb.base/watchpoint.c | 12 ++ gdb/testsuite/gdb.base/watchpoint.exp | 26 ++++ 9 files changed, 213 insertions(+), 74 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f23214c80be..4d21120cf88 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,21 @@ +2008-03-03 Daniel Jacobowitz + + * breakpoint.c (fetch_watchpoint_value): New function. + (update_watchpoint): Set and clear val_valid. Use + fetch_watchpoint_value. Handle unreadable values on the + value chain. Correct check for user-requested array watchpoints. + (breakpoint_init_inferior): Clear val_valid. + (watchpoint_value_print): New function. + (print_it_typical): Use it. Do not free or clear old_val. Print + watchpoints even if old_val == NULL. + (watchpoint_check): Use fetch_watchpoint_value. Check for values + becoming readable or unreadable. + (watch_command_1): Use fetch_watchpoint_value. Set val_valid. + (do_enable_watchpoint): Likewise. + * breakpoint.h (struct breakpoint): Update comment for val. Add + val_valid. + * NEWS: Mention watchpoints on inaccessible memory. + 2007-02-29 Daniel Jacobowitz * Makefile.in (i386-nat.o): Update. diff --git a/gdb/NEWS b/gdb/NEWS index 6bcd129c58c..b689f8cdcfd 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -9,6 +9,9 @@ set debug timetstamp show debug timestamp Display timestamps with GDB debugging output. +* Watchpoints can now be set on unreadable memory locations, e.g. addresses +which will be allocated using malloc later in program execution. + *** Changes in GDB 6.8 * New native configurations diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index a689e3ae44f..8dc6f40127d 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -55,6 +55,7 @@ #include "memattr.h" #include "ada-lang.h" #include "top.h" +#include "wrapper.h" #include "gdb-events.h" #include "mi/mi-common.h" @@ -826,7 +827,65 @@ is_hardware_watchpoint (struct breakpoint *bpt) || bpt->type == bp_access_watchpoint); } -/* Assuming that B is a hardware breakpoint: +/* Find the current value of a watchpoint on EXP. Return the value in + *VALP and *RESULTP and the chain of intermediate and final values + in *VAL_CHAIN. RESULTP and VAL_CHAIN may be NULL if the caller does + not need them. + + If an error occurs while evaluating the expression, *RESULTP will + be set to NULL. *RESULTP may be a lazy value, if the result could + not be read from memory. It is used to determine whether a value + is user-specified (we should watch the whole value) or intermediate + (we should watch only the bit used to locate the final value). + + If the final value, or any intermediate value, could not be read + from memory, *VALP will be set to NULL. *VAL_CHAIN will still be + set to any referenced values. *VALP will never be a lazy value. + This is the value which we store in struct breakpoint. + + If VAL_CHAIN is non-NULL, *VAL_CHAIN will be released from the + value chain. The caller must free the values individually. If + VAL_CHAIN is NULL, all generated values will be left on the value + chain. */ + +static void +fetch_watchpoint_value (struct expression *exp, struct value **valp, + struct value **resultp, struct value **val_chain) +{ + struct value *mark, *new_mark, *result; + + *valp = NULL; + if (resultp) + *resultp = NULL; + if (val_chain) + *val_chain = NULL; + + /* Evaluate the expression. */ + mark = value_mark (); + result = NULL; + gdb_evaluate_expression (exp, &result); + new_mark = value_mark (); + if (mark == new_mark) + return; + if (resultp) + *resultp = result; + + /* Make sure it's not lazy, so that after the target stops again we + have a non-lazy previous value to compare with. */ + if (result != NULL + && (!value_lazy (result) || gdb_value_fetch_lazy (result))) + *valp = result; + + if (val_chain) + { + /* Return the chain of intermediate values. We use this to + decide which addresses to watch. */ + *val_chain = new_mark; + value_release_to_mark (mark); + } +} + +/* Assuming that B is a hardware watchpoint: - Reparse watchpoint expression, is REPARSE is non-zero - Evaluate expression and store the result in B->val - Update the list of values that must be watched in B->loc. @@ -837,7 +896,6 @@ static void update_watchpoint (struct breakpoint *b, int reparse) { int within_current_scope; - struct value *mark = value_mark (); struct frame_id saved_frame_id; struct bp_location *loc; bpstat bs; @@ -889,9 +947,9 @@ update_watchpoint (struct breakpoint *b, int reparse) to the user when the old value and the new value may actually be completely different objects. */ value_free (b->val); - b->val = NULL; + b->val = NULL; + b->val_valid = 0; } - /* If we failed to parse the expression, for example because it refers to a global variable in a not-yet-loaded shared library, @@ -900,43 +958,37 @@ update_watchpoint (struct breakpoint *b, int reparse) is different from out-of-scope watchpoint. */ if (within_current_scope && b->exp) { - struct value *v, *next; + struct value *val_chain, *v, *result, *next; - /* Evaluate the expression and make sure it's not lazy, so that - after target stops again, we have a non-lazy previous value - to compare with. Also, making the value non-lazy will fetch - intermediate values as needed, which we use to decide which - addresses to watch. + fetch_watchpoint_value (b->exp, &v, &result, &val_chain); - The value returned by evaluate_expression is stored in b->val. - In addition, we look at all values which were created - during evaluation, and set watchoints at addresses as needed. - Those values are explicitly deleted here. */ - v = evaluate_expression (b->exp); /* Avoid setting b->val if it's already set. The meaning of b->val is 'the last value' user saw, and we should update it only if we reported that last value to user. As it happens, the code that reports it updates b->val directly. */ - if (b->val == NULL) - b->val = v; - value_contents (v); - value_release_to_mark (mark); + if (!b->val_valid) + { + b->val = v; + b->val_valid = 1; + } /* Look at each value on the value chain. */ - for (; v; v = next) + for (v = val_chain; v; v = next) { /* If it's a memory location, and GDB actually needed its contents to evaluate the expression, then we - must watch it. */ + must watch it. If the first value returned is + still lazy, that means an error occurred reading it; + watch it anyway in case it becomes readable. */ if (VALUE_LVAL (v) == lval_memory - && ! value_lazy (v)) + && (v == val_chain || ! value_lazy (v))) { struct type *vtype = check_typedef (value_type (v)); /* We only watch structs and arrays if user asked for it explicitly, never if they just happen to appear in the middle of some value chain. */ - if (v == b->val + if (v == result || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT && TYPE_CODE (vtype) != TYPE_CODE_ARRAY)) { @@ -1681,6 +1733,7 @@ breakpoint_init_inferior (enum inf_context context) if (b->val) value_free (b->val); b->val = NULL; + b->val_valid = 0; } break; default: @@ -2103,6 +2156,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) + fprintf_unfiltered (stream, _("")); + 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. @@ -2221,26 +2285,21 @@ print_it_typical (bpstat bs) case bp_watchpoint: case bp_hardware_watchpoint: - if (bs->old_val != NULL) - { - annotate_watchpoint (b->number); - if (ui_out_is_mi_like_p (uiout)) - ui_out_field_string - (uiout, "reason", - async_reason_lookup (EXEC_ASYNC_WATCHPOINT_TRIGGER)); - mention (b); - 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 (b->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 (b->number); + if (ui_out_is_mi_like_p (uiout)) + ui_out_field_string + (uiout, "reason", + async_reason_lookup (EXEC_ASYNC_WATCHPOINT_TRIGGER)); + mention (b); + 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 (b->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; @@ -2253,7 +2312,7 @@ print_it_typical (bpstat bs) mention (b); ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "value"); ui_out_text (uiout, "\nValue = "); - value_print (b->val, stb->stream, 0, Val_pretty_default); + watchpoint_value_print (b->val, stb->stream); ui_out_field_stream (uiout, "value", stb); do_cleanups (ui_out_chain); ui_out_text (uiout, "\n"); @@ -2261,7 +2320,7 @@ print_it_typical (bpstat bs) break; case bp_access_watchpoint: - if (bs->old_val != NULL) + if (bs->old_val != NULL) { annotate_watchpoint (b->number); if (ui_out_is_mi_like_p (uiout)) @@ -2271,10 +2330,8 @@ print_it_typical (bpstat bs) mention (b); 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); + watchpoint_value_print (bs->old_val, stb->stream); ui_out_field_stream (uiout, "old", stb); - value_free (bs->old_val); - bs->old_val = NULL; ui_out_text (uiout, "\nNew value = "); } else @@ -2287,7 +2344,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 (b->val, stb->stream, 0,Val_pretty_default); + watchpoint_value_print (b->val, stb->stream); ui_out_field_stream (uiout, "new", stb); do_cleanups (ui_out_chain); ui_out_text (uiout, "\n"); @@ -2574,13 +2631,20 @@ 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 (b->exp); - if (!value_equal (b->val, new_val)) + struct value *new_val; + + fetch_watchpoint_value (b->exp, &new_val, NULL, NULL); + if ((b->val != NULL) != (new_val != NULL) + || (b->val != NULL && !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; + b->val_valid = 1; /* We will stop here */ return WP_VALUE_CHANGED; } @@ -5746,10 +5810,9 @@ watch_command_1 (char *arg, int accessflag, int from_tty) 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); + fetch_watchpoint_value (exp, &val, NULL, NULL); + if (val != NULL) + release_value (val); tok = arg; while (*tok == ' ' || *tok == '\t') @@ -5838,6 +5901,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty) b->exp_valid_block = exp_valid_block; b->exp_string = savestring (exp_start, exp_end - exp_start); b->val = val; + b->val_valid = 1; b->loc->cond = cond; if (cond_start) b->cond_string = savestring (cond_start, cond_end - cond_start); @@ -7721,11 +7785,11 @@ is valid is not currently in scope.\n"), bpt->number); if (bpt->val) 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); - + fetch_watchpoint_value (bpt->exp, &bpt->val, NULL, NULL); + if (bpt->val) + release_value (bpt->val); + bpt->val_valid = 1; + if (bpt->type == bp_hardware_watchpoint || bpt->type == bp_read_watchpoint || bpt->type == bp_access_watchpoint) diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 41730c0b311..16bb927c6fb 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -391,8 +391,13 @@ struct breakpoint /* The largest block within which it is valid, or NULL if it is valid anywhere (e.g. consists just of global symbols). */ struct block *exp_valid_block; - /* Value of the watchpoint the last time we checked it. */ + /* Value of the watchpoint the last time we checked it, or NULL + when we do not know the value yet or the value was not + readable. VAL is never lazy. */ struct value *val; + /* Nonzero if VAL is valid. If VAL_VALID is set but VAL is NULL, + then an error occurred reading the value. */ + int val_valid; /* Holds the address of the related watchpoint_scope breakpoint when using watchpoints on local variables (might the concept diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 95d94d99d97..6e688990f5b 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,8 @@ +2008-03-03 Daniel Jacobowitz + + * gdb.texinfo (Set Watchpoints): Mention watchpoints on + unreadable memory. Delete obsolete SPARClite reference. + 2008-02-28 Daniel Jacobowitz * gdb.texinfo (Starting): Mention always-running targets. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index dfc6e813a89..1a0e9d263e0 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -3211,6 +3211,16 @@ expression can use any operators valid in the program's native language (@pxref{Languages}). @end itemize +You can set a watchpoint on an expression even if the expression can +not be evaluated yet. For instance, you can set a watchpoint on +@samp{*global_ptr} before @samp{global_ptr} is initialized. +@value{GDBN} will stop when your program sets @samp{global_ptr} and +the expression produces a valid value. If the expression becomes +valid in some other way than changing a variable (e.g.@: if the memory +pointed to by @samp{*global_ptr} becomes readable as the result of a +@code{malloc} call), @value{GDBN} may not stop until the next time +the expression changes. + @cindex software watchpoints @cindex hardware watchpoints Depending on your system, watchpoints may be implemented in software or @@ -3338,17 +3348,6 @@ exhaust the resources available for hardware-assisted watchpoints. That's because @value{GDBN} needs to watch every variable in the expression with separately allocated resources. -The SPARClite DSU will generate traps when a program accesses some data -or instruction address that is assigned to the debug registers. For the -data addresses, DSU facilitates the @code{watch} command. However the -hardware breakpoint registers can only take two data watchpoints, and -both watchpoints must be the same kind. For example, you can set two -watchpoints with @code{watch} commands, two with @code{rwatch} commands, -@strong{or} two with @code{awatch} commands, but you cannot set one -watchpoint with one command and the other with a different command. -@value{GDBN} will reject the command if you try to mix watchpoints. -Delete or disable unused watchpoint commands before setting new ones. - If you call a function interactively using @code{print} or @code{call}, any watchpoints you have set will be inactive until @value{GDBN} reaches another kind of breakpoint or the call completes. diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 3886227fd88..fc2a39ca5d6 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2008-03-03 Daniel Jacobowitz + + * gdb.base/watchpoint.c (global_ptr, func4): New. + (main): Call func4. + * gdb.base/watchpoint.exp: Call test_inaccessible_watchpoint. + (test_inaccessible_watchpoint): New. + 2008-02-29 Maciej W. Rozycki * lib/gdb.exp (gdb_expect): Of all the timeouts provided always diff --git a/gdb/testsuite/gdb.base/watchpoint.c b/gdb/testsuite/gdb.base/watchpoint.c index 1a61ba16521..bba97fad99d 100644 --- a/gdb/testsuite/gdb.base/watchpoint.c +++ b/gdb/testsuite/gdb.base/watchpoint.c @@ -39,6 +39,8 @@ struct foo struct1, struct2, *ptr1, *ptr2; 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; } diff --git a/gdb/testsuite/gdb.base/watchpoint.exp b/gdb/testsuite/gdb.base/watchpoint.exp index d27179de667..3c20d10e4fb 100644 --- a/gdb/testsuite/gdb.base/watchpoint.exp +++ b/gdb/testsuite/gdb.base/watchpoint.exp @@ -645,6 +645,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 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 @@ -797,6 +821,8 @@ if [initialize] then { } } + test_inaccessible_watchpoint + # See above. if [istarget "mips-idt-*"] then { gdb_exit -- 2.30.2