From: Daniel Jacobowitz <drow@false.org>
To: Ulrich Weigand <uweigand@de.ibm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [rfc, v2] Fix frame_id_inner comparison false positives
Date: Thu, 21 Aug 2008 20:16:00 -0000 [thread overview]
Message-ID: <20080821201545.GA4790@caradoc.them.org> (raw)
In-Reply-To: <200808211942.m7LJgZZ5010975@d12av02.megacenter.de.ibm.com>
On Thu, Aug 21, 2008 at 09:42:35PM +0200, Ulrich Weigand wrote:
> > On the other hand, if the user has set a backtrace depth limit, then
> > frame_find_by_id may fail if a function with a deep stack is called.
> > We'll discard the dummy frame when it's still valid and get an extra
> > sigtrap. This behavior should be straightforward to write a test case
> > for: set the backtrace limit to ten, call a function which recurses
> > twenty times and then hits a breakpoint, call another function, raise
> > the backtrace limit and see if we've lost the outer dummy frame.
>
> This seems simply a bug of frame_find_by_id. This function should be
> using get_prev_frame_1 instead of get_prev_frame. There is no reason
> why re-identifying a frame you had already selected previously should
> suddenly fail just because some user-interface setting was changed ...
Well, here I'm not sure about. I don't consider 'set backtrace limit'
to be just a user interface change; we use it as a safety net.
For example. Suppose you have an unterminated M68K stack, and the top
says '0xffffffff'. GDB will see that, assume it's a return address,
check whether there's code there, decide there isn't (memory read
fails), and use the stub unwinder - which adds 4 to the stack pointer,
restores the return address, and goes along its merry recursive way.
This is a bug in the 68k unwinder, of course. But it's a very easy
bug to have and a very hard bug to catch. So I don't like algorithms
that make us unwind past the immediate frame.
The only place your patch switched to using frame_find_by_id instead
of frame_id_inner was the dummy frame check, though. So maybe the
right thing to do is just to remove that stale dummy frame check, and
clear the dummy list when we get a new inferior? And stick to
get_prev_frame, at least for the moment.
Other than that, your logic looks entirely right to me. I like
the clever corruption check.
--
Daniel Jacobowitz
CodeSourcery
next prev parent reply other threads:[~2008-08-21 20:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-20 4:42 [rfc] Eliminate frame_id_inner comparisons Ulrich Weigand
2008-06-25 13:32 ` Joel Brobecker
2008-06-25 14:15 ` Daniel Jacobowitz
2008-08-21 19:43 ` [rfc, v2] Fix frame_id_inner comparison false positives Ulrich Weigand
2008-08-21 20:16 ` Daniel Jacobowitz [this message]
2008-08-21 21:47 ` [rfc, v3] " Ulrich Weigand
2008-08-22 1:45 ` Ulrich Weigand
2008-08-26 17:46 ` Ulrich Weigand
2008-08-26 22:19 ` Daniel Jacobowitz
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=20080821201545.GA4790@caradoc.them.org \
--to=drow@false.org \
--cc=gdb-patches@sourceware.org \
--cc=uweigand@de.ibm.com \
/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