Re: [patch v8 21/24] record-btrace: extend unwinder

[Pedro Alves <palves@redhat.com>]

On 12/12/2013 09:15 AM, Markus Metzger wrote:

> diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
> index 4efa7d8..9617c12 100644
> --- a/gdb/dwarf2-frame.c
> +++ b/gdb/dwarf2-frame.c
> @@ -31,6 +31,7 @@
>  #include "objfiles.h"
>  #include "regcache.h"
>  #include "value.h"
> +#include "record.h"
>  
>  #include "gdb_assert.h"
>  #include <string.h>
> @@ -1511,6 +1512,11 @@ dwarf2_frame_base_sniffer (struct frame_info *this_frame)
>  CORE_ADDR
>  dwarf2_frame_cfa (struct frame_info *this_frame)
>  {
> +  if (frame_unwinder_is (this_frame, &record_btrace_tailcall_frame_unwind)
> +      || frame_unwinder_is (this_frame, &record_btrace_frame_unwind))
> +    throw_error (NOT_AVAILABLE_ERROR,
> +		 _("cfa not available for record btrace target"));

Kind of odd abstraction boundaries broken here.  On the one hand,
we go through the target for the unwind sniffing, but OTOH, here
we refer to the unwinders directly.  Hmm.

Ah, I don't think it really matters -- if you convert
to the frame_info.stack_status == FID_STACK_UNAVAILABLE
mechanism, then this can check for that instead, and the
get_frame_unwind_stop_reason check can even be eliminated.
Perhaps it should be even get_frame_base the one that
throws.

>  /* Level of the selected frame: 0 for innermost, 1 for its caller, ...
> diff --git a/gdb/frame.h b/gdb/frame.h
> index 71f07dd..b20b69f 100644
> --- a/gdb/frame.h
> +++ b/gdb/frame.h
> @@ -222,7 +222,11 @@ enum frame_type
>    ARCH_FRAME,
>    /* Sentinel or registers frame.  This frame obtains register values
>       direct from the inferior's registers.  */
> -  SENTINEL_FRAME
> +  SENTINEL_FRAME,
> +  /* A branch tracing frame.  */
> +  BTRACE_FRAME,
> +  /* A branch tracing tail call frame.  */
> +  BTRACE_TAILCALL_FRAME
>  };

Hmm?  Why were these needed?  Missing patch rationale...  And missing
comments, I suppose.

-- 
Pedro Alves