This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Don't disable the current display in throw_exception
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 5 Aug 2011 15:53:01 +0100
- Subject: Don't disable the current display in throw_exception
On Thursday 04 August 2011 19:26:16, Pedro Alves wrote:
> On Wednesday 03 August 2011 20:27:10, Tom Tromey wrote:
> > Want to see something (sort of) related and gross? From
> > throw_exception:
> >
> > /* Perhaps it would be cleaner to do this via the cleanup chain (not sure
> > I can think of a reason why that is vital, though). */
> > if (tp != NULL)
> > {
> > /* Clear queued breakpoint commands. */
> > bpstat_clear_actions (tp->control.stop_bpstat);
> > }
> >
> > disable_current_display ();
>
> Yeah, that so needs to go away! Any inner exception that is
> caught and handled potentially breaks breakpoints and displays...
As e.g., the display case:
(gdb) trace 2453
Tracepoint 3 at 0x40b3e9: file ../../../src/gdb/gdbserver/server.c, line 2453.
(gdb) tstart
(gdb) n
2453 int multi_mode = 0;
(gdb) n
2454 int attach = 0;
(gdb) tstop
(gdb) tfind
Found trace frame 0, tracepoint 3
#0 main (argc=<unavailable>, argv=<unavailable>) at ../../../src/gdb/gdbserver/server.c:2453
2453 int multi_mode = 0;
(gdb) display argc
Disabling display 1 to avoid infinite recursion.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1: argc = <unavailable>
(gdb) info display
Auto-display expressions now in effect:
Num Enb Expression
1: n argc
An unavailable exception is thrown while evaluating
the dwarf location of argc, but that is caught by higher
layers, and shouldn't cause a disablement of the display.
Patch below fixes it, and adds a test that would
fail otherwise, for me at least, on amd64-linux.
I thought of deleting the current_display_number
global, and passing the display pointer to the cleanup,
but disable_current_display is used by infrun.c too.
Pedro Alves
2011-08-05 Pedro Alves <pedro@codesourcery.com>
gdb/
* exceptions.c (throw_exception): Don't disable the current
display.
* printcmd.c (disable_current_display_cleanup): New function.
(do_one_display): Install a cleanup to disable the current display
if doing the display throws.
gdb/testsuite/
* gdb.trace/unavailable.exp (test_maybe_regvar_display): New procedure.
(gdb_collect_args_test, gdb_collect_locals_test): Use it.
---
gdb/exceptions.c | 1
gdb/printcmd.c | 11 ++++++++++
gdb/testsuite/gdb.trace/unavailable.exp | 34 ++++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 1 deletion(-)
Index: src/gdb/exceptions.c
===================================================================
--- src.orig/gdb/exceptions.c 2011-08-05 15:40:32.616662949 +0100
+++ src/gdb/exceptions.c 2011-08-05 15:42:41.000000000 +0100
@@ -223,7 +223,6 @@ throw_exception (struct gdb_exception ex
bpstat_clear_actions (tp->control.stop_bpstat);
}
- disable_current_display ();
do_cleanups (ALL_CLEANUPS);
/* Jump to the containing catch_errors() call, communicating REASON
Index: src/gdb/printcmd.c
===================================================================
--- src.orig/gdb/printcmd.c 2011-08-05 15:46:53.000000000 +0100
+++ src/gdb/printcmd.c 2011-08-05 15:47:06.636663017 +0100
@@ -1656,6 +1656,14 @@ 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. */
@@ -1663,6 +1671,7 @@ undisplay_command (char *args, int from_
static void
do_one_display (struct display *d)
{
+ struct cleanup *old_chain;
int within_current_scope;
if (d->enabled_p == 0)
@@ -1715,6 +1724,7 @@ do_one_display (struct display *d)
return;
current_display_number = d->number;
+ old_chain = make_cleanup (disable_current_display_cleanup, NULL);
annotate_display_begin ();
printf_filtered ("%d", d->number);
@@ -1782,6 +1792,7 @@ do_one_display (struct display *d)
annotate_display_end ();
gdb_flush (gdb_stdout);
+ discard_cleanups (old_chain);
current_display_number = -1;
}
Index: src/gdb/testsuite/gdb.trace/unavailable.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/unavailable.exp 2011-08-05 15:40:32.616662949 +0100
+++ src/gdb/testsuite/gdb.trace/unavailable.exp 2011-08-05 15:42:41.596662971 +0100
@@ -103,6 +103,36 @@ proc run_trace_experiment { test_func }
"tfind test frame"
}
+# Test that "display VAR" works as expected, assuming VAR is wholly
+# unavailable.
+
+proc test_maybe_regvar_display { var } {
+ global gdb_prompt
+
+ # Evaluating VAR's location description may throw an internal
+ # "unavailable" exception, if for example, the value of a register
+ # necessary for computing VAR's location is unavailable. Such an
+ # exception is caught, and should not cause automatic disablement
+ # of the current display being printed. (GDB used to disable the
+ # current display whenever any exception was thrown.)
+ set test "display $var"
+ gdb_test_multiple "$test" "$test" {
+ -re "Disabling display ? to avoid infinite recursion.*$gdb_prompt $" {
+ fail "$test"
+ }
+ -re "display ${var}\r\n1: ${var} = <unavailable>\r\n$gdb_prompt $" {
+ pass "$test"
+ }
+ }
+ gdb_test "info display" ".*1:\[ \t\]+y\[ \t\]+${var}" "display ${var} is enabled"
+
+ gdb_test "undisp" \
+ "" \
+ "delete argc display" \
+ ".*Delete all auto-display expressions.*y or n. $" \
+ "y"
+}
+
#
# Test procs
#
@@ -171,6 +201,8 @@ proc gdb_collect_args_test {} {
set r "${r}argarray = <unavailable>${cr}"
gdb_test "info args" "$r" "info args"
+ test_maybe_regvar_display "argc"
+
gdb_test "tfind none" \
"#0 end .*" \
"cease trace debugging"
@@ -226,6 +258,8 @@ proc gdb_collect_locals_test { func msg
set r "${r}loci = <unavailable>${cr}"
gdb_test "info locals" "$r" "info locals"
+ test_maybe_regvar_display "loci"
+
gdb_test "tfind none" \
"#0 end .*" \
"cease trace debugging"