* Don't disable the current display in throw_exception
@ 2011-08-05 14:53 Pedro Alves
2011-08-05 15:10 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2011-08-05 14:53 UTC (permalink / raw)
To: gdb-patches
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"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Don't disable the current display in throw_exception
2011-08-05 14:53 Don't disable the current display in throw_exception Pedro Alves
@ 2011-08-05 15:10 ` Tom Tromey
2011-08-05 20:22 ` Daniel Jacobowitz
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2011-08-05 15:10 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
Pedro> An unavailable exception is thrown while evaluating
Pedro> the dwarf location of argc, but that is caught by higher
Pedro> layers, and shouldn't cause a disablement of the display.
Pedro> Patch below fixes it, and adds a test that would
Pedro> fail otherwise, for me at least, on amd64-linux.
Thanks for doing this.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Don't disable the current display in throw_exception
2011-08-05 15:10 ` Tom Tromey
@ 2011-08-05 20:22 ` Daniel Jacobowitz
2011-08-08 14:28 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2011-08-05 20:22 UTC (permalink / raw)
To: Tom Tromey; +Cc: Pedro Alves, gdb-patches
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? 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. I don't know what the "infinite recursion" in question is.
On Fri, Aug 5, 2011 at 11:10 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
>
> Pedro> An unavailable exception is thrown while evaluating
> Pedro> the dwarf location of argc, but that is caught by higher
> Pedro> layers, and shouldn't cause a disablement of the display.
> Pedro> Patch below fixes it, and adds a test that would
> Pedro> fail otherwise, for me at least, on amd64-linux.
>
> Thanks for doing this.
>
> Tom
>
--
Thanks,
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Don't disable the current display in throw_exception
2011-08-05 20:22 ` Daniel Jacobowitz
@ 2011-08-08 14:28 ` Pedro Alves
2011-08-09 16:23 ` Don't disable displays on display eval error (Re: Don't disable the current display in throw_exception) Pedro Alves
2011-08-09 18:20 ` Don't disable the current display in throw_exception Daniel Jacobowitz
0 siblings, 2 replies; 6+ messages in thread
From: Pedro Alves @ 2011-08-08 14:28 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: Tom Tromey, gdb-patches
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 :-)
> I don't know what the "infinite recursion" in question is.
Not sure either. Maybe something with infcalls. I guess
we'd have to go look at a gdb from when any error brought
you back to the top level.
--
Pedro Alves
---
gdb/printcmd.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
Index: src/gdb/printcmd.c
===================================================================
--- src.orig/gdb/printcmd.c 2011-08-05 16:34:30.000000000 +0100
+++ src/gdb/printcmd.c 2011-08-08 15:12:23.908548270 +0100
@@ -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 (¤t_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,8 +1783,16 @@ 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");
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Don't disable displays on display eval error (Re: Don't disable the current display in throw_exception)
2011-08-08 14:28 ` Pedro Alves
@ 2011-08-09 16:23 ` Pedro Alves
2011-08-09 18:20 ` Don't disable the current display in throw_exception Daniel Jacobowitz
1 sibling, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2011-08-09 16:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Daniel Jacobowitz, Tom Tromey
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 (¤t_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"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Don't disable the current display in throw_exception
2011-08-08 14:28 ` Pedro Alves
2011-08-09 16:23 ` Don't disable displays on display eval error (Re: Don't disable the current display in throw_exception) Pedro Alves
@ 2011-08-09 18:20 ` Daniel Jacobowitz
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Jacobowitz @ 2011-08-09 18:20 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
On Mon, Aug 8, 2011 at 10:28 AM, Pedro Alves <pedro@codesourcery.com> 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>
At a glance, the patch looks good to me - and I think the output is a
huge improvement.
> Do I still get cookie? :-P :-)
Sure, here's one:
ID=ba7d44f129f0edf3:FF=0:TM=1312913790:LM=1312913790:S=DnGFaNl9wLPTTtLM
Careful what you ask for...
--
Thanks,
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-09 18:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-05 14:53 Don't disable the current display in throw_exception Pedro Alves
2011-08-05 15:10 ` Tom Tromey
2011-08-05 20:22 ` Daniel Jacobowitz
2011-08-08 14:28 ` Pedro Alves
2011-08-09 16:23 ` Don't disable displays on display eval error (Re: Don't disable the current display in throw_exception) Pedro Alves
2011-08-09 18:20 ` Don't disable the current display in throw_exception Daniel Jacobowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox