Re: [PATCH] improve python finish breakpoint for exceptions/longjmp case.

["Andrew Burgess" <aburgess@broadcom.com>]

On 24/09/2012 11:21 AM, Kevin Pouget wrote:
> On Fri, Sep 21, 2012 at 4:57 PM, Andrew Burgess <aburgess@broadcom.com> wrote:
>> The documentation for the python FinishBreakpoints suggests that when a longjmp or c++ exception is used to terminate a function the out_of_scope method 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 if we leave a function using for longjmp/exception we rely on hitting some other stop point to trigger the call to the out_of_scope method.
>>
>> A further issue is that the testing for FinishBreakpoints, in gdb/testsuite/gdb.python/py-finish-breakpoint2.exp, the test action titled "check FinishBreakpoint 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 happens to be the finish breakpoint, however this is not guaranteed, and means that (a) the test does not match the documentation, and (b) the test is platform 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 some 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 cstdio.  I have to confess self interest here, on my local target the c++ streams 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 important behaviours are only tested on targets that support c++ streams.  However, if this is a problem then I can change the test back to using c++ streams.
>>
>>  2. The patch does not solve all the problems with FinishBreakpoints.  I use set_longjmp_breakpoint to create the breakpoints for the lonjmp/exceptions, however, between creating an instance of FinishBreakpoint, when the breakpoints are created, and hitting these breakpoints, we are free to issue any commands we like, including things like step/finish/etc all of which use set_longjmp_breakpoint themselves.  As these commands (step/finish/etc) complete, they remove the longjmp breakpoints, preventing the FinishBreakpoint from stopping correctly.  This behaviour is tested in my updated test and currently kfails; before committing this patch I'll raise a defect and fill in the new bug id.
>>
>> I have some ideas about how to fix #2 but I'm open to suggestions.  As the gdb is no worse than it was in case #2, and is significantly better in other 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

Thanks for taking a look through this patch.  I had a quick look through
the previous patch you posted, it looks like some of those changes would
certainly help with the nested longjmp issues.

Thanks,
Andrew