From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7428 invoked by alias); 11 Jun 2012 15:22:15 -0000 Received: (qmail 7416 invoked by uid 22791); 11 Jun 2012 15:22:12 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 11 Jun 2012 15:21:58 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 40C6C1C6E1D; Mon, 11 Jun 2012 11:21:58 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id orAIN-Ju84+3; Mon, 11 Jun 2012 11:21:58 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id EA4C71C6E09; Mon, 11 Jun 2012 11:21:57 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 322CC145616; Mon, 11 Jun 2012 08:21:48 -0700 (PDT) Date: Mon, 11 Jun 2012 15:22:00 -0000 From: Joel Brobecker To: Jan Kratochvil Cc: Mark Kettenis , gdb-patches@sourceware.org Subject: Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5 Message-ID: <20120611152148.GA31854@adacore.com> References: <20120309210045.GA30432@host2.jankratochvil.net> <20120326190355.GA11001@host2.jankratochvil.net> <201203261953.q2QJrXX4023325@glazunov.sibelius.xs4all.nl> <20120326203151.GA18085@host2.jankratochvil.net> <201203262145.q2QLjRIJ024024@glazunov.sibelius.xs4all.nl> <20120327081439.GA8387@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120327081439.GA8387@host2.jankratochvil.net> User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2012-06/txt/msg00286.txt.bz2 Hello, Does anyone have any comment on this patch from Jan? It has been identified as necessary before the release process gets started. > gdb/ > 2012-03-27 Jan Kratochvil > > Remove momentary breakpoints for completed inferior calls. > * breakpoint.h (bp_call_dummy): Update the comment. > * dummy-frame.c: Include gdbthread.h. > (pop_dummy_frame_bpt): New function. > (pop_dummy_frame): Initialie DUMMY earlier. Call pop_dummy_frame_bpt. > > gdb/testsuite/ > 2012-03-09 Jan Kratochvil > > Remove momentary breakpoints for completed inferior calls. > * gdb.base/call-signal-resume.exp (maintenance print dummy-frames) > (maintenance info breakpoints): New tests. I took a look, and nothing really obvious jumped at me. I'd wait a few more days, and go ahead and commit if you hear no objection. Just a quick question from me: Why do we need to remove the breakpoint? Is it just for GDB's benefit (purge the breakpoint from our breakpoint list), or also for the inferior's benefit. Given that we are inserting the breakpoint in scratch memory, it shouldn't make any different to the inferior, right? A couple of suggestions on the phrasing of the comment. Thanks! > > --- a/gdb/breakpoint.h > +++ b/gdb/breakpoint.h > @@ -95,10 +95,10 @@ enum bptype > > /* The breakpoint at the end of a call dummy. */ > /* FIXME: What if the function we are calling longjmp()s out of > - the call, or the user gets out with the "return" command? We > - currently have no way of cleaning up the breakpoint in these > - (obscure) situations. (Probably can solve this by noticing > - longjmp, "return", etc., it's similar to noticing when a > + the call? "return" command is handled by pop_dummy_frame_bpt. ^^^ The "return" command > + We currently have no way of cleaning up the breakpoint in such > + (obscure) situation. (Probably can solve this by noticing ^^^ We can probably solve (also lose the extra parens, I don't think they bring anything, and it will avoid the parens nesting) > + longjmp, etc., it's similar to noticing when a > watchpoint on a local variable goes out of scope (with hardware > support for watchpoints)). */ > bp_call_dummy, > --- a/gdb/dummy-frame.c > +++ b/gdb/dummy-frame.c > @@ -29,6 +29,7 @@ > #include "gdbcmd.h" > #include "gdb_string.h" > #include "observer.h" > +#include "gdbthread.h" > > /* Dummy frame. This saves the processor state just prior to setting > up the inferior function call. Older targets save the registers > @@ -108,19 +109,36 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr) > xfree (dummy); > } > > +/* Delete any breakpoint B which is a momentary breakpoint for return from > + inferior call matching DUMMY_VOIDP. */ > + > +static int > +pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp) > +{ > + struct dummy_frame *dummy = dummy_voidp; > + > + if (b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id) > + && b->thread == pid_to_thread_id (inferior_ptid)) > + delete_breakpoint (b); > + > + /* Continue the traversal. */ > + return 0; > +} > + > /* Pop *DUMMY_PTR, restoring program state to that before the > frame was created. */ > > static void > pop_dummy_frame (struct dummy_frame **dummy_ptr) > { > - struct dummy_frame *dummy; > + struct dummy_frame *dummy = *dummy_ptr; > + > + restore_infcall_suspend_state (dummy->caller_state); > > - restore_infcall_suspend_state ((*dummy_ptr)->caller_state); > + iterate_over_breakpoints (pop_dummy_frame_bpt, dummy); > > /* restore_infcall_control_state frees inf_state, > all that remains is to pop *dummy_ptr. */ > - dummy = *dummy_ptr; > *dummy_ptr = dummy->next; > xfree (dummy); > > --- a/gdb/testsuite/gdb.base/call-signal-resume.exp > +++ b/gdb/testsuite/gdb.base/call-signal-resume.exp > @@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*" > gdb_test_no_output "set confirm off" > gdb_test "return" "" > > +# Verify there are no remains of the dummy frame. > +gdb_test_no_output "maintenance print dummy-frames" > +set test "maintenance info breakpoints" > +gdb_test_multiple $test $test { > + -re "call dummy.*\r\n$gdb_prompt $" { > + fail $test > + } > + -re "\r\n$gdb_prompt $" { > + pass $test > + } > +} > + > # Resume execution, the program should continue without any signal. > > gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*" -- Joel