From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13506 invoked by alias); 5 Aug 2011 14:53:29 -0000 Received: (qmail 13483 invoked by uid 22791); 5 Aug 2011 14:53:25 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 05 Aug 2011 14:53:08 +0000 Received: (qmail 3156 invoked from network); 5 Aug 2011 14:53:07 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 5 Aug 2011 14:53:07 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Don't disable the current display in throw_exception Date: Fri, 05 Aug 2011 14:53:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-8-generic; KDE/4.6.2; x86_64; ; ) MIME-Version: 1.0 Content-Type: Text/Plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Message-Id: <201108051553.02017.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00111.txt.bz2 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=, argv=) at ../../../src/gdb/gdbserver/server.c:2453 2453 int multi_mode = 0; (gdb) display argc Disabling display 1 to avoid infinite recursion. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1: argc = (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 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} = \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 = ${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 = ${cr}" gdb_test "info locals" "$r" "info locals" + test_maybe_regvar_display "loci" + gdb_test "tfind none" \ "#0 end .*" \ "cease trace debugging"