Re: [patch v8 05/24] frame: artificial frame id's

[Pedro Alves <palves at redhat dot com>]

On 12/12/2013 09:15 AM, Markus Metzger wrote:
> At the moment, a frame must have a stack - except for the outer frame.
> 
> When we analyze the recorded execution for "record btrace" to detect
> function calls and compute a back trace at some point in the recorded
> execution history, we will end up with frames without a stack.

To be more precise, the frames do have a stack, but it's <unavailable>.
I.e., it existed, but we didn't capture it, so we can't retrieve it.
I presume "p $sp" etc. will show <unavailable>, right?

> 
> To prepare for this, support frame_id's without a stack component.

So special_addr will be a made up number, right?  Will each such
frame have its own special_addr ?

> CC:  Pedro Alves  <palves@redhat.com>
> 
> 2013-12-12  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* frame.h (frame_id_build_artificial): New.
> 	* frame.c (frame_id_build_artificial): New.
> 	(frame_id_p): An artificial frame is valid.
> 	(frame_id_eq): A frame is equal to itself.
> 
> 
> ---
>  gdb/frame.c | 30 +++++++++++++++++++++++-------
>  gdb/frame.h |  6 ++++++
>  2 files changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/frame.c b/gdb/frame.c
> index ddd5e70..37d780e 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -526,6 +526,21 @@ frame_id_build_wild (CORE_ADDR stack_addr)
>    return id;
>  }
>  
> +/* See frame.h.  */
> +
> +struct frame_id
> +frame_id_build_artificial (CORE_ADDR code_addr,
> +			   CORE_ADDR special_addr)
> +{
> +  struct frame_id id = null_frame_id;
> +
> +  id.code_addr = code_addr;
> +  id.code_addr_p = 1;
> +  id.special_addr = special_addr;
> +  id.special_addr_p = 1;
> +  return id;
> +}
> +
>  int
>  frame_id_p (struct frame_id l)
>  {
> @@ -536,6 +551,9 @@ frame_id_p (struct frame_id l)
>    /* outer_frame_id is also valid.  */
>    if (!p && memcmp (&l, &outer_frame_id, sizeof (l)) == 0)
>      p = 1;
> +  /* An artificial frame is also valid.  */
> +  if (!p && l.code_addr_p && l.special_addr_p)
> +    p = 1;
>    if (frame_debug)
>      {
>        fprintf_unfiltered (gdb_stdlog, "{ frame_id_p (l=");
> @@ -559,13 +577,11 @@ frame_id_eq (struct frame_id l, struct frame_id r)
>  {
>    int eq;
>  
> -  if (!l.stack_addr_p && l.special_addr_p
> -      && !r.stack_addr_p && r.special_addr_p)
> -    /* The outermost frame marker is equal to itself.  This is the
> -       dodgy thing about outer_frame_id, since between execution steps
> -       we might step into another function - from which we can't
> -       unwind either.  More thought required to get rid of
> -       outer_frame_id.  */
> +  if (memcmp (&l, &r, sizeof (l)) == 0)
> +    /* Every frame is equal to itself.
> +       This is the dodgy thing about outer_frame_id, since between execution
> +       steps we might step into another function - from which we can't unwind
> +       either.  More thought required to get rid of outer_frame_id.  */
>      eq = 1;
>    else if (!l.stack_addr_p || !r.stack_addr_p)
>      /* Like a NaN, if either ID is invalid, the result is false.

Looks like frame_ id_eq (null_frame_id, null_frame_id) now
returns true, while it returns false before.  If you discussed
all this and came to the conclusion it's OK, please document
it in the commit log.  In any case (I'm not sure offhand if
that's OK), the NaN comment above is no longer correct,
and neither is this one:

  /* Make the sentinel frame's ID valid, but invalid.  That way all
     comparisons with it should fail.  */
  frame->this_id.p = 1;
  frame->this_id.value = null_frame_id;

> diff --git a/gdb/frame.h b/gdb/frame.h
> index e4e6b25..71f07dd 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -174,6 +174,12 @@ extern struct frame_id frame_id_build_special (CORE_ADDR stack_addr,
>     as the special identifier address are set to indicate wild cards.  */
>  extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr);
>  
> +/* Construct an artificial frame ID.  The first parameter is the frame's
> +   constant code address (typically the function entry point), and the
> +   second the frame's special identifier address.  */
> +extern struct frame_id frame_id_build_artificial (CORE_ADDR code_addr,
> +						  CORE_ADDR special_addr);
> +
>  /* Returns non-zero when L is a valid frame (a valid frame has a
>     non-zero .base).  The outermost frame is valid even without an
>     ID.  */
> 


-- 
Pedro Alves