From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21527 invoked by alias); 22 Nov 2012 18:15:31 -0000 Received: (qmail 21518 invoked by uid 22791); 22 Nov 2012 18:15:29 -0000 X-SWARE-Spam-Status: No, hits=-6.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BP,TW_GJ,TW_HP X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 22 Nov 2012 18:15:17 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qAMIFBrO016125 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 22 Nov 2012 13:15:16 -0500 Received: from host2.jankratochvil.net (ovpn-116-39.ams2.redhat.com [10.36.116.39]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qAMIF2Wx031380 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 22 Nov 2012 13:15:08 -0500 Date: Thu, 22 Nov 2012 18:15:00 -0000 From: Jan Kratochvil To: Andrew Burgess Cc: "gdb-patches@sourceware.org" Subject: Re: [PATCH] improve python finish breakpoint for exceptions/longjmp case. Message-ID: <20121122181502.GA2658@host2.jankratochvil.net> References: <505C805A.1050400@broadcom.com> <20121011163241.GA9620@host2.jankratochvil.net> <507C7498.7040001@broadcom.com> <20121017162748.GA6546@host2.jankratochvil.net> <508991A7.5030607@broadcom.com> <20121030174041.GA12889@host2.jankratochvil.net> <50991D75.6000102@broadcom.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50991D75.6000102@broadcom.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2012-11/txt/msg00602.txt.bz2 On Tue, 06 Nov 2012 15:23:49 +0100, Andrew Burgess wrote: > On 30/10/2012 5:40 PM, Jan Kratochvil wrote: > > On Thu, 25 Oct 2012 21:23:19 +0200, Andrew Burgess wrote: > > > > Anyway we agree it is not transparent to "finish" anyway but this is more > > a problem there are no observer-like breakpoints: > > > > ==> finish.c <== > > void g (void) {} > > void f (void) { > > g (); > > } > > int main (void) { > > f (); > > return 0; > > } > > > > ==> finish.cmd <== > > file ./finish > > start > > step > > break g > > commands > > echo hook-g\n > > continue > > end > > finish > > > > ------------------- > > > > hook-g > > Actual: > > [Inferior 1 (process 13204) exited normally] > > (gdb) _ > > Expected: > > main () at finish.c:7 > > 7 return 0; > > (gdb) _ > > > > So currently the finish command stops whenever gdb stops, even though in > this case once we stop (at g) we then automatically continue again. I agree. > In the example is it that you'd like to consider the "g" breakpoint > observer-like (just because it contains a continue), Right, but I see it was only my wish, GDB does not behave that way. > or are you suggesting that the "finish" command should not auto-terminate > once gdb stops? I was suggesting "finish" should continue its "finish" work after the "g" breakpoint commands were run. But I see it does not work that way. Let's forget about this part. > I can't really see how the above example relates to the issue with my patch, however, I agree. > I have a new patch in which I believe the level of risk of code corruption should be less. The problem is it is not as safe as set_longjmp_breakpoint_for_call_dummy which I was suggest to hook into. You are hooked by observer_attach_normal_stop (bpfinishpy_handle_stop); but that does not catch for example a "return" command. ==> 65.c <== static int h (void) { return 1; } static int f (void) { return h (); } static int g (void) { return h (); } int main (void) { f (); g (); return 0; } ==> 65.cmd <== set confirm off set height 0 set width 0 shell rm -f 65 shell gcc -o 65 65.c -Wall -g file ./65 start source 65.py break h continue info break python finishbp = MyFinishBreakpoint (gdb.newest_frame ()) info break return return bt continue info break python print finishbp.return_value ==> 65.py <== class MyFinishBreakpoint (gdb.FinishBreakpoint): def __init__(self, frame): gdb.FinishBreakpoint.__init__ (self, frame) print "MyFinishBreakpoint init" def stop(self): print "MyFinishBreakpoint stop" print "return_value is: %d" % int (self.return_value) return True def out_of_scope(self): print "MyFinishBreakpoint out of scope" ============= and its output: Temporary breakpoint 1 at 0x400511: file 65.c, line 5. Temporary breakpoint 1, main () at 65.c:5 5 f (); Breakpoint 2 at 0x4004f0: file 65.c, line 1. Breakpoint 2, h () at 65.c:1 1 static int h (void) { return 1; } Num Type Disp Enb Address What 2 breakpoint keep y 0x00000000004004f0 in h at 65.c:1 breakpoint already hit 1 time Temporary breakpoint 3 at 0x400500: file 65.c, line 2. MyFinishBreakpoint init Num Type Disp Enb Address What 2 breakpoint keep y 0x00000000004004f0 in h at 65.c:1 breakpoint already hit 1 time 3 breakpoint del y 0x0000000000400500 in f at 65.c:2 thread 1 stop only in thread 1 #0 main () at 65.c:6 Breakpoint 2, h () at 65.c:1 1 static int h (void) { return 1; } Num Type Disp Enb Address What 2 breakpoint keep y 0x00000000004004f0 in h at 65.c:1 breakpoint already hit 2 times 3 breakpoint del y 0x0000000000400500 in f at 65.c:2 thread 1 stop only in thread 1 None (gdb) _ You can see in the end we already exited f() and entered it again but the finish breakpoint still exists, out-of-scope detection did not catch it. Unfortunately set_longjmp_breakpoint_for_call_dummy is a bit dummy frame specific so it would need some extensions to be suitable for this purpose. I believe it would be good to hook also into frame_pop(). > As far as I can see the only remaining issue is that using other commands > that set/clear the longjmp breakpoints once a FinishBreakpoint is active > (finish/step/etc) will remove the longjmp breakpoint, we'll then fallback to > only calling the out-of-scope callback at the next stop. To solve this I'd > like to change the longjmp breakpoint mechanism to support having multiple > longjmp breakpoint sets active at once; You would need a new type besides bp_longjmp and bp_exception. This is why I created that bp_longjmp_call_dummy which I was suggesting to extend also for your purposes. But I am OK with new bp_longjmp_py_finish_bpt or something. I do not like much that you share tp->initiating_frame but I haven't found a countercase reproducer. > but I'd rather do this in a follow-up patch, and as the new behaviour is no > worse than the original behaviour this feels like a reasonable compromise to > me (but you might disagree). I am OK with that, in fact I leave further Python API extensions on the other maintainers. [...] > @@ -161,7 +168,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) > PyObject *frame_obj = NULL; > int thread; > struct frame_info *frame, *prev_frame = NULL; > - struct frame_id frame_id; > + struct frame_id prev_frame_id, init_frame_id; > PyObject *internal = NULL; > int internal_bp = 0; > CORE_ADDR finish_pc, pc; > @@ -184,6 +191,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) > > if (frame == NULL) > goto invalid_frame; > + > + init_frame_id = get_frame_id (frame); > > TRY_CATCH (except, RETURN_MASK_ALL) > { > @@ -201,8 +210,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) > } > else > { > - frame_id = get_frame_id (prev_frame); > - if (frame_id_eq (frame_id, null_frame_id)) > + prev_frame_id = get_frame_id (prev_frame); > + if (frame_id_eq (prev_frame_id, null_frame_id)) > PyErr_SetString (PyExc_ValueError, > _("Invalid ID for the `frame' object.")); > } > @@ -295,11 +304,18 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) > AUTO_BOOLEAN_TRUE, > &bkpt_breakpoint_ops, > 0, 1, internal_bp, 0); > + set_longjmp_breakpoint (inferior_thread (), init_frame_id); Here should be prev_frame_id instead. I find it maybe a bit confusing but it is so, check 'case 2' in infrun.c 'case BPSTAT_WHAT_CLEAR_LONGJMP_RESUME'. finish_command puts there the previous frame of where you execute the "finish" command. Countercase: ==> 64.c <== #include static jmp_buf env; static int h (void) { if (setjmp (env)) return 1; longjmp (env, 1); return 2; } int main (void) { return h (); } ==> 64.cmd <== set confirm off set height 0 set width 0 shell rm -f 64 shell gcc -o 64 64.c -Wall -g file ./64 start source 64.py break h continue info break python finishbp = MyFinishBreakpoint (gdb.newest_frame ()) info break continue python print finishbp.return_value ==> 64.py <== class MyFinishBreakpoint (gdb.FinishBreakpoint): def __init__(self, frame): gdb.FinishBreakpoint.__init__ (self, frame) print "MyFinishBreakpoint init" def stop(self): print "MyFinishBreakpoint stop" print "return_value is: %d" % int (self.return_value) return True def out_of_scope(self): print "MyFinishBreakpoint out of scope" ============= and its output: Temporary breakpoint 1 at 0x4005aa: file 64.c, line 9. Temporary breakpoint 1, main () at 64.c:9 9 int main (void) { return h (); } Breakpoint 2 at 0x400580: file 64.c, line 4. Breakpoint 2, h () at 64.c:4 4 if (setjmp (env)) Num Type Disp Enb Address What 2 breakpoint keep y 0x0000000000400580 in h at 64.c:4 breakpoint already hit 1 time Temporary breakpoint 3 at 0x4005af: file 64.c, line 9. MyFinishBreakpoint init Num Type Disp Enb Address What 2 breakpoint keep y 0x0000000000400580 in h at 64.c:4 breakpoint already hit 1 time 3 breakpoint del y 0x00000000004005af in main at 64.c:9 thread 1 stop only in thread 1 0x000000000040058a in h () at 64.c:4 4 if (setjmp (env)) None (gdb) _ Existence of the Python FinishBreakpoint causes a stop during longjmp inside h itself but that should not be caught, the function still has not returned. > + > + /* Set frame to NULL for sanity, creating the breakpoint could cause > + us to switch threads, thus blowing away the frame cache, rendering > + the frame pointer invalid. */ > + frame = NULL; > } > GDB_PY_SET_HANDLE_EXCEPTION (except); > > - self_bpfinish->py_bp.bp->frame_id = frame_id; > + self_bpfinish->py_bp.bp->frame_id = prev_frame_id; > self_bpfinish->py_bp.is_finish_bp = 1; > + self_bpfinish->initiating_frame = init_frame_id; > > /* Bind the breakpoint with the current program space. */ > self_bpfinish->py_bp.bp->pspace = current_program_space; Thanks, Jan