Re: [PATCH perf/core ] perf-probe: Fix to track down unnamed union/structure members

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2015/03/09 11:15), Masami Hiramatsu wrote:
> Fix perf probe to track down unnamed union/structure members.
> perf probe did not track down the tree of unnamed union/structure
> members, since it just failed to find given "name" in a parent
> structure/union. To solve this issue, I've introduced 2 changes.
> 
> - Fix die_find_member() to track down the type-DIE if it is
>   unnamed, and if it contains the specified member, returns the
>   unnamed member.
>   (note that we don't return found member, since unnamed member
>    has the offset in the parent structure)
> - Fix convert_variable_fields() to track down the unnamed union/
>   structure (one-by-one).
> 
> With this patch, perf probe can access unnamed fields.
>   -----
>   #./perf probe -nfx ./perf lock__delete ops 'locked_ops=ops->locked.ops'
>   Added new event:
>     probe_perf:lock__delete (on lock__delete in /home/mhiramat/ksrc/linux-3/tools/perf/perf with ops locked_ops=ops->locked.ops)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe_perf:lock__delete -aR sleep 1
>   -----
> 
> The original report of this issue is: https://lkml.org/lkml/2015/3/5/431
> 
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/util/dwarf-aux.c    |   14 ++++++++++----
>  tools/perf/util/probe-finder.c |    8 +++++++-
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 780b2bc..c34e024 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -801,10 +801,16 @@ static int __die_find_member_cb(Dwarf_Die *die_mem, void *data)
>  {
>  	const char *name = data;
>  
> -	if ((dwarf_tag(die_mem) == DW_TAG_member) &&
> -	    die_compare_name(die_mem, name))
> -		return DIE_FIND_CB_END;
> -
> +	if (dwarf_tag(die_mem) == DW_TAG_member) {
> +		if (die_compare_name(die_mem, name))
> +			return DIE_FIND_CB_END;
> +		else if (!dwarf_diename(die_mem)) {	/* Unnamed structure */
> +			Dwarf_Die type_die, tmp_die;
> +			if (die_get_type(die_mem, &type_die) &&
> +			    die_find_member(&type_die, name, &tmp_die))
> +				return DIE_FIND_CB_END;
> +		}
> +	}
>  	return DIE_FIND_CB_SIBLING;
>  }
>  
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 46f009a..3898eba 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -460,7 +460,8 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
>  			       " nor array.\n", varname);
>  			return -EINVAL;
>  		}
> -		if (field->ref) {
> +		/* While prcessing unnamed field, we don't care about this */
> +		if (field->ref && !dwarf_diename(vr_die)) {
                                  ^^ Oops! this should be inverted!
I've missed to include a local fix to this patch...

Thank you,

>  			pr_err("Semantic error: %s must be referred by '.'\n",
>  			       field->name);
>  			return -EINVAL;
> @@ -491,6 +492,11 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
>  	}
>  	ref->offset += (long)offs;
>  
> +	/* If this member is unnamed, we need to reuse this field */
> +	if (!dwarf_diename(die_mem))
> +		return convert_variable_fields(die_mem, varname, field,
> +						&ref, die_mem);
> +
>  next:
>  	/* Converting next field */
>  	if (field->next)
> 
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com