Re: [PATCH -tip v2 2/2] perf-probe: Support basic dwarf-based operations on uprobe events

[David Ahern <dsahern@gmail.com>]

On 12/26/13, 12:41 AM, Masami Hiramatsu wrote:
> And this shows the available variables at the given line of
> the function.
> ----
> # ./perf probe -x perf --vars map__load:8
> Available variables at map__load:8
>          @<map__load+96>
>                  char*   name
>                  struct map*     map
>                  symbol_filter_t filter
>          @<map__find_symbol+112>
>                  char*   name
>                  symbol_filter_t filter
>          @<map__find_symbol_by_name+136>
>                  char*   name
>                  symbol_filter_t filter
>          @<map_groups__find_symbol_by_name+176>
>                  char*   name
>                  struct map*     map
>                  symbol_filter_t filter

Still limitations. This is Fedora 18:

# rpm -qa | grep debug
glibc-debuginfo-common-2.16-34.fc18.x86_64
glibc-debuginfo-2.16-34.fc18.x86_64

# /tmp/perf/perf probe -V malloc -x /lib64/libc-2.16.so
Failed to find variables at malloc (0)

So probing on system libraries does not benefit from this patch.

> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 5681266..20b3e15 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -425,7 +425,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>   	}
>
>   #ifdef HAVE_DWARF_SUPPORT
> -	if (params.show_lines && !params.uprobes) {
> +	if (params.show_lines) {
>   		if (params.mod_events) {
>   			pr_err("  Error: Don't use --line with"
>   			       " --add/--del.\n");

Unrelated change.


> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 05be5de..2f82267 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -172,6 +172,52 @@ const char *kernel_get_module_path(const char *module)
>   	return (dso) ? dso->long_name : NULL;
>   }
>
> +/* Copied from unwind.c */
> +static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
> +				    GElf_Shdr *shp, const char *name)
> +{
> +	Elf_Scn *sec = NULL;
> +
> +	while ((sec = elf_nextscn(elf, sec)) != NULL) {
> +		char *str;
> +
> +		gelf_getshdr(sec, shp);
> +		str = elf_strptr(elf, ep->e_shstrndx, shp->sh_name);
> +		if (!strcmp(name, str))
> +			break;
> +	}
> +
> +	return sec;
> +}

Why copy it? With unwind.c and util/symbol-elf.c we now have 2 copies. 
How about exporting one of those?

> +
> +static int get_text_start_address(const char *exec, unsigned long *address)
> +{
> +	Elf *elf;
> +	GElf_Ehdr ehdr;
> +	GElf_Shdr shdr;
> +	int fd, ret = -ENOENT;
> +
> +	fd = open(exec, O_RDONLY);
> +	if (fd < 0)
> +		return -errno;
> +
> +	elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
> +	if (elf == NULL)
> +		return -EINVAL;
> +
> +	if (gelf_getehdr(elf, &ehdr) == NULL)
> +		goto out;
> +
> +	if (!elf_section_by_name(elf, &ehdr, &shdr, ".text"))
> +		goto out;
> +
> +	*address = shdr.sh_addr - shdr.sh_offset;
> +	ret = 0;
> +out:
> +	elf_end(elf);
> +	return ret;
> +}
> +
>   static int init_user_exec(void)
>   {
>   	int ret = 0;
> @@ -186,6 +232,37 @@ static int init_user_exec(void)
>   	return ret;
>   }
>
> +static int convert_exec_to_group(const char *exec, char **result)
> +{
> +	char *ptr1, *ptr2, *exec_copy;
> +	char buf[64];
> +	int ret;
> +
> +	exec_copy = strdup(exec);
> +	if (!exec_copy)
> +		return -ENOMEM;
> +
> +	ptr1 = basename(exec_copy);
> +	if (!ptr1) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ptr2 = strpbrk(ptr1, "-._");
> +	if (ptr2)
> +		*ptr2 = '\0';
> +	ret = e_snprintf(buf, 64, "%s_%s", PERFPROBE_GROUP, ptr1);
> +	if (ret < 0)
> +		goto out;
> +
> +	*result = strdup(buf);
> +	ret = *result ? 0 : -ENOMEM;
> +
> +out:
> +	free(exec_copy);
> +	return ret;
> +}
> +
>   static int convert_to_perf_probe_point(struct probe_trace_point *tp,
>   					struct perf_probe_point *pp)
>   {
> @@ -261,6 +338,40 @@ static int kprobe_convert_to_perf_probe(struct probe_trace_point *tp,
>   	return 0;
>   }
>
> +static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
> +					  int ntevs, const char *exec)
> +{
> +	int i, ret = 0;
> +	unsigned long offset, stext = 0;
> +	char buf[32];
> +
> +	if (!exec)
> +		return 0;
> +
> +	ret = get_text_start_address(exec, &stext);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < ntevs && ret >= 0; i++) {
> +		offset = tevs[i].point.address - stext;
> +		offset += tevs[i].point.offset;
> +		tevs[i].point.offset = 0;
> +		free(tevs[i].point.symbol);
> +		ret = e_snprintf(buf, 32, "0x%lx", offset);
> +		if (ret < 0)
> +			break;
> +		tevs[i].point.module = strdup(exec);
> +		tevs[i].point.symbol = strdup(buf);
> +		if (!tevs[i].point.symbol || !tevs[i].point.module) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +		tevs[i].uprobes = true;
> +	}
> +
> +	return ret;
> +}
> +

More strdup's. This is library code and we need methods to free that 
memory as well.

David