From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2262 invoked by alias); 24 Sep 2012 10:22:28 -0000 Received: (qmail 2247 invoked by uid 22791); 24 Sep 2012 10:22:27 -0000 X-SWARE-Spam-Status: No, hits=-4.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_GJ X-Spam-Check-By: sourceware.org Received: from mail-vc0-f169.google.com (HELO mail-vc0-f169.google.com) (209.85.220.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 24 Sep 2012 10:22:14 +0000 Received: by vcbfl17 with SMTP id fl17so6729818vcb.0 for ; Mon, 24 Sep 2012 03:22:13 -0700 (PDT) Received: by 10.58.13.33 with SMTP id e1mr7169705vec.51.1348482133286; Mon, 24 Sep 2012 03:22:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.156.81 with HTTP; Mon, 24 Sep 2012 03:21:53 -0700 (PDT) In-Reply-To: <505C805A.1050400@broadcom.com> References: <505C805A.1050400@broadcom.com> From: Kevin Pouget Date: Mon, 24 Sep 2012 10:22:00 -0000 Message-ID: Subject: Re: [PATCH] improve python finish breakpoint for exceptions/longjmp case. To: Andrew Burgess Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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-09/txt/msg00492.txt.bz2 On Fri, Sep 21, 2012 at 4:57 PM, Andrew Burgess wro= te: > The documentation for the python FinishBreakpoints suggests that when a l= ongjmp or c++ exception is used to terminate a function the out_of_scope me= thod will be called. A quick look inside gdb/python/py-finishbreakpoint.c = shows that we take no action to ensure that we will break on exceptions or = longjmps (for example using breakpoint.c:set_longjmp_breakpoint), instead i= f we leave a function using for longjmp/exception we rely on hitting some o= ther stop point to trigger the call to the out_of_scope method. > > A further issue is that the testing for FinishBreakpoints, in gdb/testsui= te/gdb.python/py-finish-breakpoint2.exp, the test action titled "check Fini= shBreakpoint in catch()" expects the "stop" method to fire rather than the = "out_of_scope" method, this is due to the generated code (on x86 and maybe = other targets), the first breakpoint we hit after throwing the exception ha= ppens to be the finish breakpoint, however this is not guaranteed, and mean= s that (a) the test does not match the documentation, and (b) the test is p= latform specific. > > I have a patch below that improves and extends the testing to cover more = cases, and a patch for py-finishbreakpoint.c that goes some way to fixing s= ome of the problems. > > There are a couple of comments to go with the patch, > > 1. I've changed the c++ test program from using c++ streams to using cst= dio. I have to confess self interest here, on my local target the c++ stre= ams are too large to build in, so any test that uses them will not work for= me. I'm not the only target for which this is the case, see the comments = in gdb/testsuite/lib/gdb.exp. Though we _obviously_ have to have good test= coverage for gdb with c++ streams, I think it would be a shame if some imp= ortant behaviours are only tested on targets that support c++ streams. How= ever, if this is a problem then I can change the test back to using c++ str= eams. > > 2. The patch does not solve all the problems with FinishBreakpoints. I = use set_longjmp_breakpoint to create the breakpoints for the lonjmp/excepti= ons, however, between creating an instance of FinishBreakpoint, when the br= eakpoints are created, and hitting these breakpoints, we are free to issue = any commands we like, including things like step/finish/etc all of which us= e set_longjmp_breakpoint themselves. As these commands (step/finish/etc) c= omplete, they remove the longjmp breakpoints, preventing the FinishBreakpoi= nt from stopping correctly. This behaviour is tested in my updated test an= d currently kfails; before committing this patch I'll raise a defect and fi= ll in the new bug id. > > I have some ideas about how to fix #2 but I'm open to suggestions. As th= e gdb is no worse than it was in case #2, and is significantly better in ot= her cases, I'd like to push this patch now (given that I'll raise a bug for= broken case). > > Let me know what you think, OK to commit? > > Thanks, > Andrew Hello, thanks for your interest in Finish Breakpoints. I recognize that I did not really focus on these "abnormal" function returns during the initial work, so I'm glad you decided to improve it :) Your explanations and patch seem to make sense, so let's just wait for a maintainer to review it. BTW, I wrote a patch in January that get lost in the mailing list which _may_ be related and/or useful for your work, if you cant to take a look: http://permalink.gmane.org/gmane.comp.gdb.patches/72592 Cordially, Kevin