RE: [PATCH 07/11] [SQUASH] btrace: Adjust struct btrace_function::up.

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

> -----Original Message-----
> From: Wiederhake, Tim
> Sent: Friday, February 17, 2017 2:26 PM
> To: gdb-patches@sourceware.org
> Cc: Metzger, Markus T <markus.t.metzger@intel.com>
> Subject: [PATCH 07/11] [SQUASH] btrace: Adjust struct btrace_function::up.

Hello Tim,

> +static struct btrace_function *
> +ftrace_find_call_by_number (const struct btrace_thread_info *btinfo,
> +			    unsigned int number)
> +{
> +  if (number == 0 || number > VEC_length (btrace_fun_s, btinfo->functions))
> +    return NULL;
> +
> +  return VEC_index (btrace_fun_s, btinfo->functions, number - 1);
> +}

This new function needs a comment.


> -/* Find the innermost caller in the back trace of BFUN with MFUN/FUN
> -   symbol information.  */
> +/* Find the innermost caller with MFUN/FUN symbol information in the back
> trace
> +   of the function call segment with number NUMBER.  */

Just "function segment" without the "call".  More below.


>  static struct btrace_function *
>  ftrace_find_caller (struct btrace_thread_info *btinfo,
> -		    struct btrace_function *bfun,
> +		    unsigned int number,
>  		    struct minimal_symbol *mfun,
>  		    struct symbol *fun)
>  {
> -  for (; bfun != NULL; bfun = bfun->up)
> +  struct btrace_function *bfun;
> +
> +  while ((bfun = ftrace_find_call_by_number (btinfo, number)) != NULL)
>      {
> -      /* Skip functions with incompatible symbol information.  */
> -      if (ftrace_function_switched (bfun, mfun, fun))
> -	continue;
> +      if (!ftrace_function_switched (bfun, mfun, fun))
> +	break;
> 
> -      /* This is the function segment we're looking for.  */
> -      break;
> +      number = bfun->up;
>      }
> 
>    return bfun;
>  }

Eventually, I think we'd want most functions to take and return indices or function
numbers instead of btrace_function pointers.  They can get the actual btrace_function
in the body.

This will reduce the lifetime of btrace_function pointers and make it easier to deal
with vector reallocations.

If we added helpers to get the up, segment.prev, and segment.next numbers from a
function number, most won't need a btrace_function pointer, at all.  They could work
solely on the function numbers.


> -  caller = bfun->up;
> -  if (caller == NULL)
> +  if (bfun->up == 0)
>      throw_error (NOT_AVAILABLE_ERROR,
>  		 _("No caller in btrace record history"));
> 
> +  caller = VEC_index (btrace_fun_s, cache->tp->btrace.functions, bfun->up - 1);

We should add functions to btrace.h to translate function numbers into
const struct btrace_function *.

Thanks,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928