Don't disable displays on display eval error (Re: Don't disable the current display in throw_exception)

Pedro Alves
2011-08-09T16:23:48Z

On Monday 08 August 2011 15:28:23, Pedro Alves wrote:
> On Friday 05 August 2011 21:22:28, Daniel Jacobowitz wrote:
> > Seconded! Does this also fix the bug where if you display a variable,
> > and it is temporarily at a bad address, the display is disabled?
>
> Nope...
>
> > Or is that still the case? That always drives me nuts - yes, I know foo
> > is NULL, but I want GDB to go back to displaying *foo when it comes
> > back.
>
> That one doesn't look hard. Below's a quick patch at it (not regtested).
> Here's what I get:
>
> (top-gdb) display
> 2: *argv = 0x7fffffffe37c "/home/pedro/gdb/try_catch/build/gdb/gdb"
> (top-gdb) p argv = 0
> $2 = (char **) 0x0
> (top-gdb) display
> 2: *argv = <error: Cannot access memory at address 0x0>
>
> Do I still get cookie? :-P :-)

Alright, did a small tweak, added a test, tested and applied.
Let's see if anything breaks.

--
Pedro Alves

2011-08-09 Pedro Alves <pedro@codesourcery.com>

gdb/
* printcmd.c (current_display_number): Update comment.
(disable_current_display_cleanup): Delete.
(do_one_display): Use make_cleanup_restore_integer. Gracefully
catch errors thrown while evaluating and printing the display.

gdb/testsuite/
* gdb.base/display.c (do_loops): New `p_i' local.
* gdb.base/display.exp: Test displaying a variable that is
temporarily at a bad address.

---
gdb/printcmd.c | 50 ++++++++++++++++++++-----------------
gdb/testsuite/gdb.base/display.c | 2 +
gdb/testsuite/gdb.base/display.exp | 26 +++++++++++++++++++
3 files changed, 56 insertions(+), 22 deletions(-)

Index: src/gdb/printcmd.c
===================================================================
--- src.orig/gdb/printcmd.c 2011-08-09 16:58:49.753505939 +0100
+++ src/gdb/printcmd.c 2011-08-09 17:13:42.443505630 +0100
@@ -129,7 +129,7 @@ show_print_symbol_filename (struct ui_fi
}

/* Number of auto-display expression currently being displayed.
- So that we can disable it if we get an error or a signal within it.
+ So that we can disable it if we get a signal within it.
-1 when not doing one. */

int current_display_number;
@@ -1656,14 +1656,6 @@ undisplay_command (char *args, int from_
dont_repeat ();
}

-/* Cleanup that just disables the current display. */
-
-static void
-disable_current_display_cleanup (void *arg)
-{
- disable_current_display ();
-}
-
/* Display a single auto-display.
Do nothing if the display cannot be printed in the current context,
or if the display is disabled. */
@@ -1723,8 +1715,8 @@ do_one_display (struct display *d)
if (!within_current_scope)
return;

+ old_chain = make_cleanup_restore_integer (&current_display_number);
current_display_number = d->number;
- old_chain = make_cleanup (disable_current_display_cleanup, NULL);

annotate_display_begin ();
printf_filtered ("%d", d->number);
@@ -1732,8 +1724,7 @@ do_one_display (struct display *d)
printf_filtered (": ");
if (d->format.size)
{
- CORE_ADDR addr;
- struct value *val;
+ volatile struct gdb_exception ex;

annotate_display_format ();

@@ -1755,18 +1746,26 @@ do_one_display (struct display *d)
else
printf_filtered (" ");

- val = evaluate_expression (d->exp);
- addr = value_as_address (val);
- if (d->format.format == 'i')
- addr = gdbarch_addr_bits_remove (d->exp->gdbarch, addr);
-
annotate_display_value ();

- do_examine (d->format, d->exp->gdbarch, addr);
+ TRY_CATCH (ex, RETURN_MASK_ERROR)
+ {
+ struct value *val;
+ CORE_ADDR addr;
+
+ val = evaluate_expression (d->exp);
+ addr = value_as_address (val);
+ if (d->format.format == 'i')
+ addr = gdbarch_addr_bits_remove (d->exp->gdbarch, addr);
+ do_examine (d->format, d->exp->gdbarch, addr);
+ }
+ if (ex.reason < 0)
+ fprintf_filtered (gdb_stdout, _("<error: %s>\n"), ex.message);
}
else
{
struct value_print_options opts;
+ volatile struct gdb_exception ex;

annotate_display_format ();

@@ -1784,16 +1783,23 @@ do_one_display (struct display *d)

get_formatted_print_options (&opts, d->format.format);
opts.raw = d->format.raw;
- print_formatted (evaluate_expression (d->exp),
- d->format.size, &opts, gdb_stdout);
+
+ TRY_CATCH (ex, RETURN_MASK_ERROR)
+ {
+ struct value *val;
+
+ val = evaluate_expression (d->exp);
+ print_formatted (val, d->format.size, &opts, gdb_stdout);
+ }
+ if (ex.reason < 0)
+ fprintf_filtered (gdb_stdout, _("<error: %s>"), ex.message);
printf_filtered ("\n");
}

annotate_display_end ();

gdb_flush (gdb_stdout);
- discard_cleanups (old_chain);
- current_display_number = -1;
+ do_cleanups (old_chain);
}

/* Display all of the values on the auto-display chain which can be
Index: src/gdb/testsuite/gdb.base/display.c
===================================================================
--- src.orig/gdb/testsuite/gdb.base/display.c 2011-08-09 17:13:46.000000000 +0100
+++ src/gdb/testsuite/gdb.base/display.c 2011-08-09 17:14:27.183505615 +0100
@@ -14,6 +14,8 @@ int do_loops()
int k=0;
int j=0;
float f=3.1415;
+ int *p_i = &i;
+
for( i = 0; i < LOOP; i++ ) { /* set breakpoint 1 here */
for( j = 0; j < LOOP; j++ ) {
for( k = 0; k < LOOP; k++ ) {
Index: src/gdb/testsuite/gdb.base/display.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/display.exp 2011-08-09 17:13:46.000000000 +0100
+++ src/gdb/testsuite/gdb.base/display.exp 2011-08-09 17:15:11.243505600 +0100
@@ -119,6 +119,32 @@ gdb_test "undisp" \
"y"

+# Test displaying a variable that is temporarily at a bad address.
+# But if we can examine what's at memory address 0, then we'll also be
+# able to display it without error. Don't run the test in that case.
+set can_read_0 0
+gdb_test_multiple "x 0" "memory at address 0" {
+ -re "0x0:.*Cannot access memory at address 0x0.*$gdb_prompt $" { }
+ -re "0x0:.*Error accessing memory address 0x0.*$gdb_prompt $" { }
+ -re ".*$gdb_prompt $" {
+ set can_read_0 1
+ }
+}
+
+if { !$can_read_0 } {
+ gdb_test "disp *p_i" ".*: \\*p_i = 0"
+ gdb_test "p p_i = 0x0" " = \\(int \\*\\) 0x0"
+ gdb_test "display" ".*: \\*p_i = <error: .*>" "display bad address"
+ gdb_test "p p_i = &i" " = \\(int \\*\\) $hex"
+ gdb_test "display" ".*: \\*p_i = 0" "display good address"
+
+ gdb_test "undisp" \
+ "" \
+ "undisp all again" \
+ ".*Delete all auto-display expressions.*y or n. $" \
+ "y"
+}
+
gdb_test "disab 3" ".*.*" "disab 3"
gdb_test "cont" ".*Breakpoint 4.*" "watch off"