[RFC] Special type for the sentinel frame?

[RFC] Special type for the sentinel frame?

[Mark Kettenis]

Note the comment in the following fragment from sentinel-frame.c:

  const struct frame_unwind sentinel_frame_unwinder =
  {
    /* Should the sentinel frame be given a special type?  */
    NORMAL_FRAME,
    sentinel_frame_this_id,
    sentinel_frame_prev_register
  };

I think the answer to this question is yes.  The following code in
blockframe.c would certainly benefit from it:

  /* return the address of the PC for the given FRAME, ie the current PC value
     if FRAME is the innermost frame, or the address adjusted to point to the
     call instruction if not.  */

  CORE_ADDR
  frame_address_in_block (struct frame_info *frame)
  {
    CORE_ADDR pc = get_frame_pc (frame);

    /* If we are not in the innermost frame, and we are not interrupted
       by a signal, frame->pc points to the instruction following the
       call. As a consequence, we need to get the address of the previous
       instruction. Unfortunately, this is not straightforward to do, so
       we just use the address minus one, which is a good enough
       approximation.  */
    /* FIXME: cagney/2002-11-10: Should this instead test for
       NORMAL_FRAME?  A dummy frame (in fact all the abnormal frames)
       save the PC value in the block.  */
    if (get_next_frame (frame) != 0
	&& get_frame_type (get_next_frame (frame)) != SIGTRAMP_FRAME)
      --pc;

    return pc;
  }

Since the answer to the question in the FIXME would again be yes.
Decreasing PC here would be wrong for sentinel frames in the same way
as it is wrong for dummy frames and signal trampolines.

The reason I bring this to your attention, is that I'm facing a
similar situation in the DWARF CFI frame unwinder.  Of course I can
detect the sentinel frame by looking at the relative frame level.
However, having a seperate frame type for the sentinel frame makes
things cleaner IMHO.

Thoughts?  Andrew?

Mark

Re: [RFC] Special type for the sentinel frame?

[Andrew Cagney]

>     /* If we are not in the innermost frame, and we are not interrupted
>        by a signal, frame->pc points to the instruction following the
>        call. As a consequence, we need to get the address of the previous
>        instruction. Unfortunately, this is not straightforward to do, so
>        we just use the address minus one, which is a good enough
>        approximation.  */
>     /* FIXME: cagney/2002-11-10: Should this instead test for
>        NORMAL_FRAME?  A dummy frame (in fact all the abnormal frames)
>        save the PC value in the block.  */
>     if (get_next_frame (frame) != 0
> 	&& get_frame_type (get_next_frame (frame)) != SIGTRAMP_FRAME)
>       --pc;
> 
>     return pc;
>   }

Here's a better set of comments:

struct frame_unwind
{
   /* The frame's type.  Should this instead be a collection of
      predicates that test the frame for various attributes?  */
   enum frame_type type;
   /* Should an attribute indicating the frame's address-in-block go
      here?  */
   frame_this_id_ftype *this_id;
   frame_prev_register_ftype *prev_register;
};

> Decreasing PC here would be wrong for sentinel frames in the same way
> as it is wrong for dummy frames and signal trampolines.
> 
> The reason I bring this to your attention, is that I'm facing a
> similar situation in the DWARF CFI frame unwinder.  Of course I can
> detect the sentinel frame by looking at the relative frame level.
> However, having a seperate frame type for the sentinel frame makes
> things cleaner IMHO.

I think it should be an attribute, or at least a function that localises 
the logic for determining if/when things should be decremented. 
Otherwize, everytime someone adds a new frame type, they have to audit 
all the calls to determine if additioinal changes are needed.

I suspect that get_frame_type() and this ever increasing list of frame 
types should be eliminated.

Andrew