Re: [PATCH v3 2/7] perf probe: Verify given line is a representive line

From: Arnaldo Carvalho de Melo
Date: Mon Nov 18 2019 - 16:59:14 EST


Em Mon, Nov 18, 2019 at 05:12:00PM +0900, Masami Hiramatsu escreveu:
> Verify user given probe line is a representive line (which doesn't
> share the address with other lines or the line is the least line
> among the lines which shares same address), and if not, it shows
> what is the representive line.
>
> Without this fix, user can put a probe on the lines which is not a
> a representive line. But since this is not a representive line,
> perf probe -l shows a representive line number instead of user given
> line number. e.g. (put kernel_read:3, but listed as kernel_read:2)
>
> # perf probe -a kernel_read:3
> Added new event:
> probe:kernel_read (on kernel_read:3)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:kernel_read -aR sleep 1
>
> # perf probe -l
> probe:kernel_read (on kernel_read:2@xxxxxxxxxxx/fs/read_write.c)
>
> With this fix, perf probe doesn't allow user to put a probe on
> a representive line, and tell what is the representive line.
>
> # perf probe -a kernel_read:3
> This line is sharing the addrees with other lines.
> Please try to probe at kernel_read:2 instead.
> Error: Failed to add events.

Thanks, this fixes the problem I reported, applied.

- Arnaldo

> Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> ---
> tools/perf/util/probe-finder.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 9ecea45da4ca..ef1b320cedf8 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -776,6 +776,39 @@ static Dwarf_Die *find_best_scope(struct probe_finder *pf, Dwarf_Die *die_mem)
> return fsp.found ? die_mem : NULL;
> }
>
> +static int verify_representive_line(struct probe_finder *pf, const char *fname,
> + int lineno, Dwarf_Addr addr)
> +{
> + const char *__fname, *__func = NULL;
> + Dwarf_Die die_mem;
> + int __lineno;
> +
> + /* Verify line number and address by reverse search */
> + if (cu_find_lineinfo(&pf->cu_die, addr, &__fname, &__lineno) < 0)
> + return 0;
> +
> + pr_debug2("Reversed line: %s:%d\n", __fname, __lineno);
> + if (strcmp(fname, __fname) || lineno == __lineno)
> + return 0;
> +
> + pr_warning("This line is sharing the addrees with other lines.\n");
> +
> + if (pf->pev->point.function) {
> + /* Find best match function name and lines */
> + pf->addr = addr;
> + if (find_best_scope(pf, &die_mem)
> + && die_match_name(&die_mem, pf->pev->point.function)
> + && dwarf_decl_line(&die_mem, &lineno) == 0) {
> + __func = dwarf_diename(&die_mem);
> + __lineno -= lineno;
> + }
> + }
> + pr_warning("Please try to probe at %s:%d instead.\n",
> + __func ? : __fname, __lineno);
> +
> + return -ENOENT;
> +}
> +
> static int probe_point_line_walker(const char *fname, int lineno,
> Dwarf_Addr addr, void *data)
> {
> @@ -786,6 +819,9 @@ static int probe_point_line_walker(const char *fname, int lineno,
> if (lineno != pf->lno || strtailcmp(fname, pf->fname) != 0)
> return 0;
>
> + if (verify_representive_line(pf, fname, lineno, addr))
> + return -ENOENT;
> +
> pf->addr = addr;
> sc_die = find_best_scope(pf, &die_mem);
> if (!sc_die) {

--

- Arnaldo