Re: [PATCH] perf probe: Add support for DW_OP_call_frame_cfa vars

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

Em Wed, Apr 01, 2020 at 07:19:52PM +0300, Daniel Shaulov escreveu:
> Add support for probes on variables with DW_OP_call_frame_cfa
> as the dwarf operation in the debug info.
> 
> Some compilers (specifically Golang compiler) output
> DW_OP_call_frame_cfa instead of DW_OP_fbreg for variables
> on the stack. If DW_OP_call_frame_cfa is the only expression
> than it is the same as DW_OP_fbreg with an offset of zero.
> In the case of the Golang compiler, DW_OP_call_frame_cfa may
> be followed by DW_OP_consts, with a number and than DW_OP_plus.
> This trio is the same as DW_OP_fbreg with the number from
> DW_OP_consts as the offset.
> 
> With this change, probing on functions in Golang with variables works.

Masami, can I have your Reviewed-by, please?

Daniel, next time please consider providing precise steps to show what
wasn't working and then how the patch solves the problem, i.e. get some
go program, show the command lines used to build, probe, etc.

- Arnaldo
 
> Signed-off-by: Daniel Shaulov <daniel.shaulov@granulate.io>
> ---
>  tools/perf/util/probe-finder.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index e4cff49384f4..866b17aea263 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -240,11 +240,23 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
>  	}
>  
>  	/* If this is based on frame buffer, set the offset */
> -	if (op->atom == DW_OP_fbreg) {
> +	if (op->atom == DW_OP_fbreg || op->atom == DW_OP_call_frame_cfa) {
>  		if (fb_ops == NULL)
>  			return -ENOTSUP;
>  		ref = true;
> -		offs = op->number;
> +		if (op->atom == DW_OP_fbreg) {
> +			offs = op->number;
> +		} else if (nops == 3) {
> +			/*
> +			 * In the case of DW_OP_call_frame_cfa, we either have
> +			 * an offset of 0 or we have two more expressions that
> +			 * add a const
> +			 */
> +			if ((op + 1)->atom != DW_OP_consts ||
> +			    (op + 2)->atom != DW_OP_plus)
> +				return -ENOTSUP;
> +			offs = (op + 1)->number;
> +		}
>  		op = &fb_ops[0];
>  	}
>  
> -- 
> 2.22.0
> 

-- 

- Arnaldo