Re: [patch v4 09/24] btrace: add replay position to btrace thread info

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Wed, 03 Jul 2013 11:14:19 +0200, Markus Metzger wrote:
> Add a branch trace instruction iterator pointing to the current replay position
> to the branch trace thread info struct.
> 
> Free the iterator when btrace is cleared.
> 
> Start at the replay position for the instruction and function-call histories.
> 
> 2013-07-03  Markus Metzger  <markus.t.metzger@intel.com>
> 
>     * btrace.h (replay) <replay>: New.
>     (btrace_is_replaying): New.
>     * btrace.c (btrace_clear): Free replay iterator.
>     (btrace_is_replaying): New.
>     * record-btrace.c (record_btrace_is_replaying): New.
>     (record_btrace_info): Print insn number if replaying.
>     (record_btrace_insn_history): Start at replay position.
>     (record_btrace_call_history): Start at replay position.
>     (init_record_btrace_ops): Init to_record_is_replaying.
> 
> 
> ---
>  gdb/btrace.c        |   10 ++++++
>  gdb/btrace.h        |    6 ++++
>  gdb/record-btrace.c |   80 +++++++++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 88 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index 006deaa..0bec2cf 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -771,9 +771,11 @@ btrace_clear (struct thread_info *tp)
>  
>    xfree (btinfo->insn_history);
>    xfree (btinfo->call_history);
> +  xfree (btinfo->replay);
>  
>    btinfo->insn_history = NULL;
>    btinfo->call_history = NULL;
> +  btinfo->replay = NULL;
>  }
>  
>  /* See btrace.h.  */
> @@ -1371,3 +1373,11 @@ btrace_set_call_history (struct btrace_thread_info *btinfo,
>    btinfo->call_history->begin = *begin;
>    btinfo->call_history->end = *end;
>  }
> +
> +/* See btrace.h.  */
> +
> +int
> +btrace_is_replaying (struct thread_info *tp)
> +{
> +  return tp->btrace.replay != NULL;
> +}
> diff --git a/gdb/btrace.h b/gdb/btrace.h
> index a3322d2..5a5b297 100644
> --- a/gdb/btrace.h
> +++ b/gdb/btrace.h
> @@ -181,6 +181,9 @@ struct btrace_thread_info
>  
>    /* The function call history iterator.  */
>    struct btrace_call_history *call_history;
> +
> +  /* The current replay position.  NULL if not replaying.  */
> +  struct btrace_insn_iterator *replay;
>  };
>  
>  /* Enable branch tracing for a thread.  */
> @@ -301,4 +304,7 @@ extern void btrace_set_call_history (struct btrace_thread_info *,
>  				     const struct btrace_call_iterator *begin,
>  				     const struct btrace_call_iterator *end);
>  
> +/* Determine if branch tracing is currently replaying TP.  */
> +extern int btrace_is_replaying (struct thread_info *tp);
> +
>  #endif /* BTRACE_H */
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index c7d6e9f..5e41b20 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -237,6 +237,10 @@ record_btrace_info (void)
>    printf_unfiltered (_("Recorded %u instructions in %u functions for thread "
>  		       "%d (%s).\n"), insns, calls, tp->num,
>  		     target_pid_to_str (tp->ptid));
> +
> +  if (btrace_is_replaying (tp))
> +    printf_unfiltered (_("Replay in progress.  At instruction %u.\n"),
> +		       btrace_insn_number (btinfo->replay));
>  }
>  
>  /* Print an unsigned int.  */
> @@ -301,13 +305,34 @@ record_btrace_insn_history (int size, int flags)
>    history = btinfo->insn_history;
>    if (history == NULL)
>      {
> -      /* No matter the direction, we start with the tail of the trace.  */
> -      btrace_insn_end (&begin, btinfo);
> -      end = begin;
> +      struct btrace_insn_iterator *replay;
>  
>        DEBUG ("insn-history (0x%x): %d", flags, size);
>  
> -      covered = btrace_insn_prev (&begin, context);
> +      /* If we're replaying, we start at the replay position.  Otherwise, we
> +	 start at the tail of the trace.  */
> +      replay = btinfo->replay;
> +      if (replay != NULL)
> +	begin = *replay;
> +      else
> +	btrace_insn_end (&begin, btinfo);
> +
> +      /* We start from here and expand in the requested direction.  Then we
> +	 expand in the other direction, as well, to fill up any remaining
> +	 context.  */
> +      end = begin;
> +      if (size < 0)
> +	{
> +	  /* We want the current position covered, as well.  */
> +	  covered = btrace_insn_next (&end, 1);
> +	  covered += btrace_insn_prev (&begin, context - covered);
> +	  covered += btrace_insn_next (&end, context - covered);
> +	}
> +      else
> +	{
> +	  covered = btrace_insn_next (&end, context);
> +	  covered += btrace_insn_prev (&begin, context - covered);
> +	}

These two COVERED calculations do not seem right to me, pointer is moving NEXT
and PREV so the directions should be both added and subtracted.


>      }
>    else
>      {
> @@ -562,13 +587,37 @@ record_btrace_call_history (int size, int flags)
>    history = btinfo->call_history;
>    if (history == NULL)
>      {
> -      /* No matter the direction, we start with the tail of the trace.  */
> -      btrace_call_end (&begin, btinfo);
> -      end = begin;
> +      struct btrace_insn_iterator *replay;
>  
>        DEBUG ("call-history (0x%x): %d", flags, size);
>  
> -      covered = btrace_call_prev (&begin, context);
> +      /* If we're replaying, we start at the replay position.  Otherwise, we
> +	 start at the tail of the trace.  */
> +      replay = btinfo->replay;
> +      if (replay != NULL)
> +	{
> +	  begin.function = replay->function;
> +	  begin.btinfo = btinfo;
> +	}
> +      else
> +	btrace_call_end (&begin, btinfo);
> +
> +      /* We start from here and expand in the requested direction.  Then we
> +	 expand in the other direction, as well, to fill up any remaining
> +	 context.  */
> +      end = begin;
> +      if (size < 0)
> +	{
> +	  /* We want the current position covered, as well.  */
> +	  covered = btrace_call_next (&end, 1);
> +	  covered += btrace_call_prev (&begin, context - covered);
> +	  covered += btrace_call_next (&end, context - covered);
> +	}
> +      else
> +	{
> +	  covered = btrace_call_next (&end, context);
> +	  covered += btrace_call_prev (&begin, context- covered);
> +	}

These two COVERED calculations do not seem right to me, pointer is moving NEXT
and PREV so the directions should be both added and subtracted.


>      }
>    else
>      {
> @@ -689,6 +738,20 @@ record_btrace_call_history_from (ULONGEST from, int size, int flags)
>    record_btrace_call_history_range (begin, end, flags);
>  }
>  
> +/* The to_record_is_replaying method of target record-btrace.  */
> +
> +static int
> +record_btrace_is_replaying (void)
> +{
> +  struct thread_info *tp;
> +
> +  ALL_THREADS (tp)
> +    if (btrace_is_replaying (tp))
> +      return 1;
> +
> +  return 0;
> +}
> +
>  /* Initialize the record-btrace target ops.  */
>  
>  static void
> @@ -715,6 +778,7 @@ init_record_btrace_ops (void)
>    ops->to_call_history = record_btrace_call_history;
>    ops->to_call_history_from = record_btrace_call_history_from;
>    ops->to_call_history_range = record_btrace_call_history_range;
> +  ops->to_record_is_replaying = record_btrace_is_replaying;
>    ops->to_stratum = record_stratum;
>    ops->to_magic = OPS_MAGIC;
>  }
> -- 
> 1.7.1