This is the mail archive of the
gdb-patches@sourceware.cygnus.com
mailing list for the GDB project.
Re: Patch: catch_errors() cleanup chain restore
- To: Andrew Cagney <ac131313 at cygnus dot com>
- Subject: Re: Patch: catch_errors() cleanup chain restore
- From: Elena Zannoni <ezannoni at cygnus dot com>
- Date: Wed, 16 Feb 2000 20:54:08 -0500 (EST)
- Cc: nsd at cygnus dot com, gdb-patches at sourceware dot cygnus dot com
- References: <200002112031.UAA02868@nog.bosbc.com><38AB5030.C2544706@cygnus.com>
Andrew Cagney writes:
> nsd@cygnus.com wrote:
> >
> > Hi,
> >
> > If a quit happens within catch_errors(RETURN_MASK_ERROR), the cleanup
> > chain gets lost.
> >
> > This is the offending sequence of events:
> > 1. catch_errors() calls save_cleanups(), which zeros the cleanup chain.
> > 2. catch_errors() redirects the error longjmp target to a location that
> > will call restore_cleanups(), but doesn't redirect the quit longjmp
> > target similarly when RETURN_MASK_ERROR is specified.
> > 3. A subsequent quit event longjmps back to top level, skipping past
> > the restore_cleanups() call in catch_errors().
> >
> > An error within catch_errors(RETURN_MASK_QUIT) would have the same effect,
> > but there aren't any such calls in gdb at the moment.
> >
> > It's easy to fix by:
> > 1. unconditionally redirecting both the error and quit longjmp targets;
> > 2. calling the function argument;
> > 3. restoring the cleanup chain;
> > 4. calling return_to_top_level() if appropriate.
> >
> > I've appended a patch to do that. Comments welcome,
> >
> > Nick Duffek
> > nsd@cygnus.com
>
> Um, I think you've just pointed to a far nastier bug:
>
> Consider the stack:
>
> catch_errors (MASK_ALL)
> error_return = quit_return = (1)
> ...
> -> catch_errors (MASK_ERRORS)
> saved_error = error_return /* (1) */
> error_return = (2)
> ...
> -> catch_errors (MASK_QUIT)
> saved_quit = quit_return /* (1) */
> quit_return = (3)
> ...
> -> error ()
> -> return_to_top_level (ERROR)
>
> where (1), (2), and (3) denote setjmp points. After the long jmp we end
> up with:
>
> catch_errors (MASK_ALL)
>
> ...
>
> with:
>
> error_return == (1) and quit_return == (3)
>
> A subsequent call to quit() would land GDB in the middle of (3) a popped
> stack :-(
>
> Can anyone confirm this?
>
It seems right. That's a bug.
> Andrew
>
> PS: Goto's result in bad kama :-)
>
I would prefer if this patch wasn't applied as is, we can solve Nick's
immediate problem by adding another catch_errors(..RETURN_MASK_QUIT)
call on top of the original catch_errors(..RETURN_MASK_ALL) where the
problem occurred, and do something meaningful with the return values.
[BTW, for the curious, this is in a (the?) call to
print_only_stack_frame_stub()]
Elena