[RFA] Use TRY/CATCH in remove_prev_frame

  This changes remove_prev_frame to use TRY/CATCH instead of a cleanup.
TRY/CATCH seemed appropriate here because the cleanup is only needed
in the case where an exception is thrown.

Tested by the buildbot.

ChangeLog
2018-05-24  Tom Tromey  <tom@tromey.com>

	* frame.c (remove_prev_frame): Remove.
	(get_prev_frame_if_no_cycle): Use TRY/CATCH.
---
 gdb/ChangeLog |  5 +++++
 gdb/frame.c   | 58 +++++++++++++++++++++++++---------------------------------
 2 files changed, 30 insertions(+), 33 deletions(-)
  

Quoting a "git diff -w" version of the patch, for easier context.

On 05/25/2018 04:36 AM, Tom Tromey wrote:

> diff --git a/gdb/frame.c b/gdb/frame.c
> index 1d0eb725145..e55b4e16305 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1871,22 +1871,6 @@ frame_register_unwind_location (struct frame_info *this_frame, int regnum,
>      }
>  }
>  
> -/* Called during frame unwinding to remove a previous frame pointer from a
> -   frame passed in ARG.  */
> -
> -static void
> -remove_prev_frame (void *arg)
> -{
> -  struct frame_info *this_frame, *prev_frame;
> -
> -  this_frame = (struct frame_info *) arg;
> -  prev_frame = this_frame->prev;
> -  gdb_assert (prev_frame != NULL);
> -
> -  prev_frame->next = NULL;
> -  this_frame->prev = NULL;
> -}
> -
>  /* Get the previous raw frame, and check that it is not identical to
>     same other frame frame already in the chain.  If it is, there is
>     most likely a stack cycle, so we discard it, and mark THIS_FRAME as
> @@ -1899,7 +1883,6 @@ static struct frame_info *
>  get_prev_frame_if_no_cycle (struct frame_info *this_frame)
>  {
>    struct frame_info *prev_frame;
> -  struct cleanup *prev_frame_cleanup;
>  
>    prev_frame = get_prev_frame_raw (this_frame);
>  
> @@ -1915,10 +1898,8 @@ get_prev_frame_if_no_cycle (struct frame_info *this_frame)
>    if (prev_frame->level == 0)
>      return prev_frame;
>  
> -  /* The cleanup will remove the previous frame that get_prev_frame_raw
> -     linked onto THIS_FRAME.  */
> -  prev_frame_cleanup = make_cleanup (remove_prev_frame, this_frame);
> -
> +  TRY
> +    {
>        compute_frame_id (prev_frame);
>        if (!frame_stash_add (prev_frame))
>  	{
> @@ -1936,8 +1917,19 @@ get_prev_frame_if_no_cycle (struct frame_info *this_frame)
>  	  this_frame->prev = NULL;
>  	  prev_frame = NULL;
>  	}
> +    }
> +  CATCH (ex, RETURN_MASK_ERROR)

Use RETURN_MASK_ALL.

> +    {
> +      prev_frame = this_frame->prev;
> +      gdb_assert (prev_frame != NULL);

I think these two lines aren't necessary?  It seems like
they existed in remove_prev_frame because that needs to
find the prev_frame from its passed-in argument, which is
just this_frame.  But here we already have prev_frame handy.

OK with those changes.

> +
> +      prev_frame->next = NULL;
> +      this_frame->prev = NULL;
> +
> +      throw_exception (ex);
> +    }
> +  END_CATCH
>  
> -  discard_cleanups (prev_frame_cleanup);
>    return prev_frame;
>  }
>  
>
  

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> +  CATCH (ex, RETURN_MASK_ERROR)

Pedro> Use RETURN_MASK_ALL.

I made this change but someone may want to audit the other uses of
TRY/CATCH in frame.c.  They all use RETURN_MASK_ERROR.  Or I could just
convert them all.

Tom
  

On 05/25/2018 03:58 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
>>> +  CATCH (ex, RETURN_MASK_ERROR)
> 
> Pedro> Use RETURN_MASK_ALL.
> 
> I made this change but someone may want to audit the other uses of
> TRY/CATCH in frame.c.  They all use RETURN_MASK_ERROR.  Or I could just
> convert them all.

The other uses are all correct.  They want to handle specific
types of exceptions, like:

  CATCH (ex, RETURN_MASK_ERROR)
    {
      if (ex.error == MEMORY_ERROR)

  CATCH (ex, RETURN_MASK_ERROR)
    {
      if (ex.error == NOT_AVAILABLE_ERROR)

All other types of exception are rethrown.  So it would be
useless to use RETURN_MASK_ALL for those, because it would
mean we would catch Ctrl-C just to rethrow it again
immediately.

Thanks,
Pedro Alves
  

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> The other uses are all correct.

Thanks, I see.

Tom