Re: [RFC 2/5] Frame unwinding using struct value

[Thiago Jung Bauermann <bauerman at br dot ibm dot com>]

On Mon, 2008-03-31 at 18:10 -0400, Daniel Jacobowitz wrote:
> 2008-03-31  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	Convert frame unwinders to use the current frame and
> 	"struct value".

My two cents.

> +      /* Offsets are not supported here; lazy register values must
> +	 refer to the entire register.  */
> +      gdb_assert (value_offset (val) == 0);

Is this a limitation of the current implementation or is there an
underlying reason for it?

> +      while (VALUE_LVAL (new_val) == lval_register
> +	     && value_lazy (new_val))
> +	{

The conditions in the while loop above fit in one line.

> +	  frame = frame_find_by_id (VALUE_FRAME_ID (new_val));

I thought it was strange that VALUE_FRAME_ID expands to a deprecated
function, since it plays such an important role in the new world
order... Do you know why it is deprecated?

> +struct value *
> +frame_unwind_got_address (struct frame_info *frame, int regnum,
> +			  CORE_ADDR addr)
> +{
> +  struct gdbarch *gdbarch = get_frame_arch (frame);
> +  struct value *reg_val;
> +
> +  reg_val = value_zero (register_type (gdbarch, regnum), not_lval);
> +  pack_long (value_contents_writeable (reg_val),
> +	     register_type (gdbarch, regnum), addr);

This code assumes the register type is TYPE_CODE_PTR, right?
Does it make sense to put an assertion here?

> +      /* Use an architecture specific method to extract the this frame's
> +	 dummy ID, assuming it is a dummy frame.  */
> +      this_id = gdbarch_dummy_id (get_frame_arch (this_frame), this_frame);

s/the this/this/

> +@section Selecting an Unwinder
> +
> +The architecture registers a list of frame unwinders (@code{struct
> +frame_unwind}), using the functions
> +@code{frame_unwind_prepend_unwinder} and
> +@code{frame_unwind_append_unwinder}.  Each unwinder includes a
> +sniffer.  Whenever @value{GDBN} needs to unwind a frame (to fetch the
> +previous frame's registers or the current frame's ID), it calls
> +registered sniffers in order to find one which recognizes the frame.
> +The first time a sniffer returns non-zero, the corresponding unwinder
> +is assigned to the frame.

What about changing the last sentence to "The first sniffer returning
non-zero will have its corresponding unwinder assigned to the frame."?

> +@section Unwinding the Frame ID
> +
> +Every frame has an associated ID, of type @code{struct frame_id}.
> +The ID includes the stack base and function start address for
> +the frame.  The ID persists through the entire life of the frame,
> +including while other called frames are running; it is used to
> +locate an appropriate @code{struct frame_info} from the cache.

This looks like a good place to explain the motivation behind frame IDs.
What about the following?

Every time the inferior stops, the frame cache is flushed. Because of
this, parts of @value{GDBN} which need to keep track of individual
frames cannot use pointers to @code{struct frame_info}. This is why
frame IDs are necessary, they provide a stable reference to a frame,
even when the unwinder needs to find it again and generate a new
@code{struct frame_info} for it.

> +The frame's unwinder's @code{this_id} method is called to find the ID.
> +Note that this is different from register unwinding, where the next
> +frame's @code{prev_register} is called to unwind this frame's
> +registers.
> +
> +Both stack base and function address are required to identify the
> +frame, because a recursive function has the same function address for
> +two consecutive frames and a leaf function may have the same stack
> +address as its caller.  On some platforms, a third address is part of
> +the ID to further disambiguate frames---for instance, on IA-64
> +the separate register stack address is included in the ID.
> +
> +An invalid frame ID (@code{null_frame_id}) returned from the
> +@code{this_id} method means to stop unwinding after this frame.

Is this a good place to list the other reasons to stop frame unwinding?
E.g.:

In addition, @value{GDBN} will stop unwinding after this frame if its ID
is inner to the next younger frame (i.e, the unwind is going backwards
in the stack frame), if its ID is equal to the ID of the next younger
frame, or if the unwinder was not able to find a saved program counter.
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center