This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
> From: Tom Tromey <tromey@redhat.com> > Date: Thu, 02 Aug 2012 10:11:47 -0600 > > PR 14100 concerns a way to crash gdb by C-c during a 'bt'. > > The way this happens is that dwarf2_frame_cache initializes the frame's > prologue_cache. Then, it continues to do some more work, including > (indirectly) reading target memory. > > Then, target_read invokes QUIT, throwing an exception. > The cleanups are run, and eventually we get to > frame_cleanup_after_sniffer, which asserts that prologue_cache==NULL. > > This fix assumes that what dwarf2_frame_cache is doing is not > unreasonable, and simply clears the prologue_cache field. > > I am not sure whether this is really correct. > > Another approach would be to change dwarf2_frame_cache to set the > prologue_cache at the end of its work rather than at the beginning. > Then, I suppose, we'd have to document this restriction and audit all > the other sniffers. > > Thoughts? I'd say the gdb_assert() *is* the documentation of the restriction. So I'd say the 2nd approach is the correct approach. Setting prologue_cache at the very end certainly used to be the style that most frame cache routines used. > * frame.c (frame_cleanup_after_sniffer): Remove assert. > Clear frame's prologue_cache. > --- > gdb/frame.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/gdb/frame.c b/gdb/frame.c > index e012f2d..edb379c 100644 > --- a/gdb/frame.c > +++ b/gdb/frame.c > @@ -2392,8 +2392,11 @@ frame_cleanup_after_sniffer (void *arg) > struct frame_info *frame = arg; > > /* The sniffer should not allocate a prologue cache if it did not > - match this frame. */ > - gdb_assert (frame->prologue_cache == NULL); > + match this frame. We used to assert that prologue_cache was NULL > + here -- however, that ran afoul of code paths where the > + prologue_cache was set by the sniffer, but some later processing > + called QUIT. */ > + frame->prologue_cache = NULL; Not a big fan of these comments that try to document history. Better to just state the reason for setting it to NULL.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |