From: Kevin Pouget <kevin.pouget@gmail.com>
To: Andrew Burgess <aburgess@broadcom.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH] improve python finish breakpoint for exceptions/longjmp case.
Date: Mon, 24 Sep 2012 10:22:00 -0000 [thread overview]
Message-ID: <CAPftXUJCe7bBW-5Af9zo_94eNfVGNY1s=t5eS0zarc0Og-+xUw@mail.gmail.com> (raw)
In-Reply-To: <505C805A.1050400@broadcom.com>
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
Cordially,
Kevin
next prev parent reply other threads:[~2012-09-24 10:22 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-21 14:58 Andrew Burgess
2012-09-24 10:22 ` Kevin Pouget [this message]
2012-10-01 16:33 ` Andrew Burgess
2012-10-01 16:30 ` Andrew Burgess
2012-10-10 21:08 ` Andrew Burgess
2012-10-11 6:46 ` Phil Muldoon
2012-10-11 16:32 ` Jan Kratochvil
2012-10-15 20:40 ` Andrew Burgess
2012-10-17 16:28 ` Jan Kratochvil
2012-10-24 20:48 ` Andrew Burgess
2012-10-25 6:13 ` Jan Kratochvil
2012-10-25 19:23 ` Andrew Burgess
2012-10-30 17:41 ` Jan Kratochvil
2012-11-06 14:24 ` Andrew Burgess
2012-11-22 18:15 ` Jan Kratochvil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPftXUJCe7bBW-5Af9zo_94eNfVGNY1s=t5eS0zarc0Og-+xUw@mail.gmail.com' \
--to=kevin.pouget@gmail.com \
--cc=aburgess@broadcom.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox