linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions
@ 2019-10-25  8:46 Masami Hiramatsu
  2019-10-25  8:46 ` [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification Masami Hiramatsu
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25  8:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Hi Arnaldo,

Here are some bugfixes for the bugs you found in previous series
and similar one.

I found that there are some dwarf_entrypc() related bugs in perf
probe and fixed all except one special add-hoc code in
convert_variable_location()(*).

This time I embedded before-and-after examples in each commit.
Please check it how to reproduce it.

(*) This had been introduced for fixup fentry related gcc bug,
see commit 3d918a12a1b3 ("perf probe: Find fentry mcount fuzzed
parameter location") for detail. Nowadays gcc already fixed
this issue and this seems like a dead code. Moreover, it is not
sure such old gcc can generate DIE without entry_pc attribute.
So I decided not to touch it.

Thank you,

---

Masami Hiramatsu (6):
      perf/probe: Fix wrong address verification
      perf/probe: Fix to probe a function which has no entry pc
      perf/probe: Fix to probe an inline function which has no entry pc
      perf/probe: Fix to list probe event with correct line number
      perf/probe: Fix to show inlined function callsite without entry_pc
      perf/probe: Fix to show ranges of variables in functions without entry_pc


 tools/perf/util/dwarf-aux.c    |    6 +++---
 tools/perf/util/probe-finder.c |   40 ++++++++++++++--------------------------
 2 files changed, 17 insertions(+), 29 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification
  2019-10-25  8:46 [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions Masami Hiramatsu
@ 2019-10-25  8:46 ` Masami Hiramatsu
  2019-10-25 12:14   ` Arnaldo Carvalho de Melo
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-25  8:46 ` [BUGFIX PATCH 2/6] perf/probe: Fix to probe a function which has no entry pc Masami Hiramatsu
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25  8:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Since there are some DIE which has only ranges instead of the
combination of entrypc/highpc, address verification must use
dwarf_haspc() instead of dwarf_entrypc/dwarf_highpc.

Also, the ranges only DIE will have a partial code in different
section (e.g. unlikely code will be in text.unlikely as "FUNC.cold"
symbol). In that case, we can not use dwarf_entrypc() or
die_entrypc(), because the offset from original DIE can be
a minus value.

Instead, this simply gets the symbol and offset from symtab.

Without this patch;
  # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
  Failed to get entry address of clear_tasks_mm_cpumask
    Error: Failed to add events.

And with this patch
  # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
  p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0
  p:probe/clear_tasks_mm_cpumask_1 clear_tasks_mm_cpumask+5
  p:probe/clear_tasks_mm_cpumask_2 clear_tasks_mm_cpumask+8
  p:probe/clear_tasks_mm_cpumask_3 clear_tasks_mm_cpumask+16
  p:probe/clear_tasks_mm_cpumask_4 clear_tasks_mm_cpumask+82

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Fixes: 576b523721b7 ("perf probe: Fix probing symbols with optimization suffix")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-finder.c |   32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index cd9f95e5044e..2b6513e5725c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -604,38 +604,26 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwfl_Module *mod,
 				  const char *function,
 				  struct probe_trace_point *tp)
 {
-	Dwarf_Addr eaddr, highaddr;
+	Dwarf_Addr eaddr;
 	GElf_Sym sym;
 	const char *symbol;
 
 	/* Verify the address is correct */
-	if (dwarf_entrypc(sp_die, &eaddr) != 0) {
-		pr_warning("Failed to get entry address of %s\n",
-			   dwarf_diename(sp_die));
-		return -ENOENT;
-	}
-	if (dwarf_highpc(sp_die, &highaddr) != 0) {
-		pr_warning("Failed to get end address of %s\n",
-			   dwarf_diename(sp_die));
-		return -ENOENT;
-	}
-	if (paddr > highaddr) {
-		pr_warning("Offset specified is greater than size of %s\n",
+	if (!dwarf_haspc(sp_die, paddr)) {
+		pr_warning("Specified offset is out of %s\n",
 			   dwarf_diename(sp_die));
 		return -EINVAL;
 	}
 
-	symbol = dwarf_diename(sp_die);
+	/* Try to get actual symbol name from symtab */
+	symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
 	if (!symbol) {
-		/* Try to get the symbol name from symtab */
-		symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
-		if (!symbol) {
-			pr_warning("Failed to find symbol at 0x%lx\n",
-				   (unsigned long)paddr);
-			return -ENOENT;
-		}
-		eaddr = sym.st_value;
+		pr_warning("Failed to find symbol at 0x%lx\n",
+			   (unsigned long)paddr);
+		return -ENOENT;
 	}
+	eaddr = sym.st_value;
+
 	tp->offset = (unsigned long)(paddr - eaddr);
 	tp->address = (unsigned long)paddr;
 	tp->symbol = strdup(symbol);


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [BUGFIX PATCH 2/6] perf/probe: Fix to probe a function which has no entry pc
  2019-10-25  8:46 [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions Masami Hiramatsu
  2019-10-25  8:46 ` [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification Masami Hiramatsu
@ 2019-10-25  8:46 ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-25  8:46 ` [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline " Masami Hiramatsu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25  8:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Fix perf probe to probe a function which has no entry pc or
low pc but only has ranges attribute.

probe_point_search_cb() uses dwarf_entrypc() to get the
probe address, but that doesn't work for the function DIE
which has only ranges attribute. Use die_entrypc() instead.

Without this fix,
  # tools/perf/perf probe -k ../build-x86_64/vmlinux -D clear_tasks_mm_cpumask:0
  Probe point 'clear_tasks_mm_cpumask' not found.
    Error: Failed to add events.

With this,
  # tools/perf/perf probe -k ../build-x86_64/vmlinux -D clear_tasks_mm_cpumask:0
  p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Fixes: e1ecbbc3fa83 ("perf probe: Fix to handle optimized not-inlined functions")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-finder.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 2b6513e5725c..71633f55f045 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -982,7 +982,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
 		param->retval = find_probe_point_by_line(pf);
 	} else if (die_is_func_instance(sp_die)) {
 		/* Instances always have the entry address */
-		dwarf_entrypc(sp_die, &pf->addr);
+		die_entrypc(sp_die, &pf->addr);
 		/* But in some case the entry address is 0 */
 		if (pf->addr == 0) {
 			pr_debug("%s has no entry PC. Skipped\n",


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline function which has no entry pc
  2019-10-25  8:46 [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions Masami Hiramatsu
  2019-10-25  8:46 ` [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification Masami Hiramatsu
  2019-10-25  8:46 ` [BUGFIX PATCH 2/6] perf/probe: Fix to probe a function which has no entry pc Masami Hiramatsu
@ 2019-10-25  8:46 ` Masami Hiramatsu
  2019-10-25 14:35   ` Arnaldo Carvalho de Melo
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-25  8:46 ` [BUGFIX PATCH 4/6] perf/probe: Fix to list probe event with correct line number Masami Hiramatsu
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25  8:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Fix perf probe to probe an inlne function which has no entry pc
or low pc but only has ranges attribute.

This seems very rare case, but I could find a few examples, as
same as probe_point_search_cb(), use die_entrypc() to get the
entry address in probe_point_inline_cb() too.

Without this patch,
  # tools/perf/perf probe -D __amd_put_nb_event_constraints
  Failed to get entry address of __amd_put_nb_event_constraints.
  Probe point '__amd_put_nb_event_constraints' not found.
    Error: Failed to add events.

With this patch,
  # tools/perf/perf probe -D __amd_put_nb_event_constraints
  p:probe/__amd_put_nb_event_constraints amd_put_event_constraints+43

Fixes: 4ea42b181434 ("perf: Add perf probe subcommand, a kprobe-event setup helper")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-finder.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 71633f55f045..2fa932bcf960 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -930,7 +930,7 @@ static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
 		ret = find_probe_point_lazy(in_die, pf);
 	else {
 		/* Get probe address */
-		if (dwarf_entrypc(in_die, &addr) != 0) {
+		if (die_entrypc(in_die, &addr) != 0) {
 			pr_warning("Failed to get entry address of %s.\n",
 				   dwarf_diename(in_die));
 			return -ENOENT;


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [BUGFIX PATCH 4/6] perf/probe: Fix to list probe event with correct line number
  2019-10-25  8:46 [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2019-10-25  8:46 ` [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline " Masami Hiramatsu
@ 2019-10-25  8:46 ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-25  8:47 ` [BUGFIX PATCH 5/6] perf/probe: Fix to show inlined function callsite without entry_pc Masami Hiramatsu
  2019-10-25  8:47 ` [BUGFIX PATCH 6/6] perf/probe: Fix to show ranges of variables in functions " Masami Hiramatsu
  5 siblings, 1 reply; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25  8:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Since debuginfo__find_probe_point() uses dwarf_entrypc() for
finding the entry address of the function on which a probe is,
it will fail when the function DIE has only ranges attribute.

To fix this issue, use die_entrypc() instead of dwarf_entrypc().

Without this fix, perf probe -l shows incorrect offset.
  # perf probe -l
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask+18446744071579263632@work/linux/linux/kernel/cpu.c)
    probe:clear_tasks_mm_cpumask_1 (on clear_tasks_mm_cpumask+18446744071579263752@work/linux/linux/kernel/cpu.c)

With this,
  # perf probe -l
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask@work/linux/linux/kernel/cpu.c)
    probe:clear_tasks_mm_cpumask_1 (on clear_tasks_mm_cpumask:21@work/linux/linux/kernel/cpu.c)

Fixes: 1d46ea2a6a40 ("perf probe: Fix listing incorrect line number with inline function")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/probe-finder.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 2fa932bcf960..88e17a4f5ac3 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1566,7 +1566,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
 		/* Get function entry information */
 		func = basefunc = dwarf_diename(&spdie);
 		if (!func ||
-		    dwarf_entrypc(&spdie, &baseaddr) != 0 ||
+		    die_entrypc(&spdie, &baseaddr) != 0 ||
 		    dwarf_decl_line(&spdie, &baseline) != 0) {
 			lineno = 0;
 			goto post;
@@ -1583,7 +1583,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
 		while (die_find_top_inlinefunc(&spdie, (Dwarf_Addr)addr,
 						&indie)) {
 			/* There is an inline function */
-			if (dwarf_entrypc(&indie, &_addr) == 0 &&
+			if (die_entrypc(&indie, &_addr) == 0 &&
 			    _addr == addr) {
 				/*
 				 * addr is at an inline function entry.


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [BUGFIX PATCH 5/6] perf/probe: Fix to show inlined function callsite without entry_pc
  2019-10-25  8:46 [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2019-10-25  8:46 ` [BUGFIX PATCH 4/6] perf/probe: Fix to list probe event with correct line number Masami Hiramatsu
@ 2019-10-25  8:47 ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  2019-10-25  8:47 ` [BUGFIX PATCH 6/6] perf/probe: Fix to show ranges of variables in functions " Masami Hiramatsu
  5 siblings, 1 reply; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25  8:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Fix perf-probe --line option to show inlined function callsite
lines even if the function DIE has only ranges.

Without this,
  # tools/perf/perf probe -L amd_put_event_constraints
  ...
      2  {
      3         if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
                        __amd_put_nb_event_constraints(cpuc, event);
      5  }

With this patch,
  # tools/perf/perf probe -L amd_put_event_constraints
  ...
      2  {
      3         if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
      4                 __amd_put_nb_event_constraints(cpuc, event);
      5  }

Fixes: 4cc9cec636e7 ("perf probe: Introduce lines walker interface")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/dwarf-aux.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 063f71da6b63..e0c507d6b3b4 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -695,7 +695,7 @@ static int __die_walk_funclines_cb(Dwarf_Die *in_die, void *data)
 	if (dwarf_tag(in_die) == DW_TAG_inlined_subroutine) {
 		fname = die_get_call_file(in_die);
 		lineno = die_get_call_lineno(in_die);
-		if (fname && lineno > 0 && dwarf_entrypc(in_die, &addr) == 0) {
+		if (fname && lineno > 0 && die_entrypc(in_die, &addr) == 0) {
 			lw->retval = lw->callback(fname, lineno, addr, lw->data);
 			if (lw->retval != 0)
 				return DIE_FIND_CB_END;


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [BUGFIX PATCH 6/6] perf/probe: Fix to show ranges of variables in functions without entry_pc
  2019-10-25  8:46 [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2019-10-25  8:47 ` [BUGFIX PATCH 5/6] perf/probe: Fix to show inlined function callsite without entry_pc Masami Hiramatsu
@ 2019-10-25  8:47 ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  5 siblings, 1 reply; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25  8:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Masami Hiramatsu, linux-kernel

Fix to show ranges of variables (--range and --vars option) in
functions which DIE has only ranges but no entry_pc attribute.

Without this fix,
  # tools/perf/perf probe --range -V clear_tasks_mm_cpumask
  Available variables at clear_tasks_mm_cpumask
  	@<clear_tasks_mm_cpumask+0>
  		(No matched variables)

With this fix,
  # tools/perf/perf probe --range -V clear_tasks_mm_cpumask
  Available variables at clear_tasks_mm_cpumask
	@<clear_tasks_mm_cpumask+0>
		[VAL]	int	cpu	@<clear_tasks_mm_cpumask+[0-35,317-317,2052-2059]>

Fixes: 349e8d261131 ("perf probe: Add --range option to show a variable's location range")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/perf/util/dwarf-aux.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index e0c507d6b3b4..ac82fd937e4b 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1019,7 +1019,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
 	bool first = true;
 	const char *name;
 
-	ret = dwarf_entrypc(sp_die, &entry);
+	ret = die_entrypc(sp_die, &entry);
 	if (ret)
 		return ret;
 
@@ -1082,7 +1082,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
 	bool first = true;
 	const char *name;
 
-	ret = dwarf_entrypc(sp_die, &entry);
+	ret = die_entrypc(sp_die, &entry);
 	if (ret)
 		return ret;
 


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification
  2019-10-25  8:46 ` [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification Masami Hiramatsu
@ 2019-10-25 12:14   ` Arnaldo Carvalho de Melo
  2019-10-25 12:36     ` Masami Hiramatsu
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  1 sibling, 1 reply; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-25 12:14 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Em Fri, Oct 25, 2019 at 05:46:25PM +0900, Masami Hiramatsu escreveu:
> Since there are some DIE which has only ranges instead of the
> combination of entrypc/highpc, address verification must use
> dwarf_haspc() instead of dwarf_entrypc/dwarf_highpc.
> 
> Also, the ranges only DIE will have a partial code in different
> section (e.g. unlikely code will be in text.unlikely as "FUNC.cold"
> symbol). In that case, we can not use dwarf_entrypc() or
> die_entrypc(), because the offset from original DIE can be
> a minus value.
> 
> Instead, this simply gets the symbol and offset from symtab.
> 
> Without this patch;
>   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
>   Failed to get entry address of clear_tasks_mm_cpumask
>     Error: Failed to add events.
> 
> And with this patch
>   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
>   p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0
>   p:probe/clear_tasks_mm_cpumask_1 clear_tasks_mm_cpumask+5
>   p:probe/clear_tasks_mm_cpumask_2 clear_tasks_mm_cpumask+8
>   p:probe/clear_tasks_mm_cpumask_3 clear_tasks_mm_cpumask+16
>   p:probe/clear_tasks_mm_cpumask_4 clear_tasks_mm_cpumask+82

Ok, so this just asks for the definition, but doesn't try to actually
_use_ it, which I did and it fails:

[root@quaco tracebuffer]# perf probe -D clear_tasks_mm_cpumask:1
p:probe/clear_tasks_mm_cpumask _text+919968
p:probe/clear_tasks_mm_cpumask_1 _text+919973
p:probe/clear_tasks_mm_cpumask_2 _text+919976
[root@quaco tracebuffer]#
[root@quaco tracebuffer]# perf probe clear_tasks_mm_cpumask
Probe point 'clear_tasks_mm_cpumask' not found.
  Error: Failed to add events.
[root@quaco tracebuffer]#

So I'll tentatively continue to apply the other patches in this series,
maybe one of them will fix this.

- Arnaldo
 
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Fixes: 576b523721b7 ("perf probe: Fix probing symbols with optimization suffix")
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-finder.c |   32 ++++++++++----------------------
>  1 file changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index cd9f95e5044e..2b6513e5725c 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -604,38 +604,26 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwfl_Module *mod,
>  				  const char *function,
>  				  struct probe_trace_point *tp)
>  {
> -	Dwarf_Addr eaddr, highaddr;
> +	Dwarf_Addr eaddr;
>  	GElf_Sym sym;
>  	const char *symbol;
>  
>  	/* Verify the address is correct */
> -	if (dwarf_entrypc(sp_die, &eaddr) != 0) {
> -		pr_warning("Failed to get entry address of %s\n",
> -			   dwarf_diename(sp_die));
> -		return -ENOENT;
> -	}
> -	if (dwarf_highpc(sp_die, &highaddr) != 0) {
> -		pr_warning("Failed to get end address of %s\n",
> -			   dwarf_diename(sp_die));
> -		return -ENOENT;
> -	}
> -	if (paddr > highaddr) {
> -		pr_warning("Offset specified is greater than size of %s\n",
> +	if (!dwarf_haspc(sp_die, paddr)) {
> +		pr_warning("Specified offset is out of %s\n",
>  			   dwarf_diename(sp_die));
>  		return -EINVAL;
>  	}
>  
> -	symbol = dwarf_diename(sp_die);
> +	/* Try to get actual symbol name from symtab */
> +	symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
>  	if (!symbol) {
> -		/* Try to get the symbol name from symtab */
> -		symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
> -		if (!symbol) {
> -			pr_warning("Failed to find symbol at 0x%lx\n",
> -				   (unsigned long)paddr);
> -			return -ENOENT;
> -		}
> -		eaddr = sym.st_value;
> +		pr_warning("Failed to find symbol at 0x%lx\n",
> +			   (unsigned long)paddr);
> +		return -ENOENT;
>  	}
> +	eaddr = sym.st_value;
> +
>  	tp->offset = (unsigned long)(paddr - eaddr);
>  	tp->address = (unsigned long)paddr;
>  	tp->symbol = strdup(symbol);

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification
  2019-10-25 12:14   ` Arnaldo Carvalho de Melo
@ 2019-10-25 12:36     ` Masami Hiramatsu
  2019-10-25 14:28       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-25 12:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Hi,

On Fri, 25 Oct 2019 09:14:48 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Fri, Oct 25, 2019 at 05:46:25PM +0900, Masami Hiramatsu escreveu:
> > Since there are some DIE which has only ranges instead of the
> > combination of entrypc/highpc, address verification must use
> > dwarf_haspc() instead of dwarf_entrypc/dwarf_highpc.
> > 
> > Also, the ranges only DIE will have a partial code in different
> > section (e.g. unlikely code will be in text.unlikely as "FUNC.cold"
> > symbol). In that case, we can not use dwarf_entrypc() or
> > die_entrypc(), because the offset from original DIE can be
> > a minus value.
> > 
> > Instead, this simply gets the symbol and offset from symtab.
> > 
> > Without this patch;
> >   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
> >   Failed to get entry address of clear_tasks_mm_cpumask
> >     Error: Failed to add events.
> > 
> > And with this patch
> >   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
> >   p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0
> >   p:probe/clear_tasks_mm_cpumask_1 clear_tasks_mm_cpumask+5
> >   p:probe/clear_tasks_mm_cpumask_2 clear_tasks_mm_cpumask+8
> >   p:probe/clear_tasks_mm_cpumask_3 clear_tasks_mm_cpumask+16
> >   p:probe/clear_tasks_mm_cpumask_4 clear_tasks_mm_cpumask+82
> 
> Ok, so this just asks for the definition, but doesn't try to actually
> _use_ it, which I did and it fails:
> 
> [root@quaco tracebuffer]# perf probe -D clear_tasks_mm_cpumask:1
> p:probe/clear_tasks_mm_cpumask _text+919968
> p:probe/clear_tasks_mm_cpumask_1 _text+919973
> p:probe/clear_tasks_mm_cpumask_2 _text+919976
> [root@quaco tracebuffer]#
> [root@quaco tracebuffer]# perf probe clear_tasks_mm_cpumask
> Probe point 'clear_tasks_mm_cpumask' not found.
>   Error: Failed to add events.
> [root@quaco tracebuffer]#
> 
> So I'll tentatively continue to apply the other patches in this series,
> maybe one of them will fix this.

Yes, it should be fixed by [2/6] :)

Actually, a probe point with offset line and no-offset are handled
a bit differently.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification
  2019-10-25 12:36     ` Masami Hiramatsu
@ 2019-10-25 14:28       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-25 14:28 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Em Fri, Oct 25, 2019 at 09:36:33PM +0900, Masami Hiramatsu escreveu:
> Hi,
> 
> On Fri, 25 Oct 2019 09:14:48 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Fri, Oct 25, 2019 at 05:46:25PM +0900, Masami Hiramatsu escreveu:
> > > Since there are some DIE which has only ranges instead of the
> > > combination of entrypc/highpc, address verification must use
> > > dwarf_haspc() instead of dwarf_entrypc/dwarf_highpc.
> > > 
> > > Also, the ranges only DIE will have a partial code in different
> > > section (e.g. unlikely code will be in text.unlikely as "FUNC.cold"
> > > symbol). In that case, we can not use dwarf_entrypc() or
> > > die_entrypc(), because the offset from original DIE can be
> > > a minus value.
> > > 
> > > Instead, this simply gets the symbol and offset from symtab.
> > > 
> > > Without this patch;
> > >   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
> > >   Failed to get entry address of clear_tasks_mm_cpumask
> > >     Error: Failed to add events.
> > > 
> > > And with this patch
> > >   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
> > >   p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0
> > >   p:probe/clear_tasks_mm_cpumask_1 clear_tasks_mm_cpumask+5
> > >   p:probe/clear_tasks_mm_cpumask_2 clear_tasks_mm_cpumask+8
> > >   p:probe/clear_tasks_mm_cpumask_3 clear_tasks_mm_cpumask+16
> > >   p:probe/clear_tasks_mm_cpumask_4 clear_tasks_mm_cpumask+82
> > 
> > Ok, so this just asks for the definition, but doesn't try to actually
> > _use_ it, which I did and it fails:
> > 
> > [root@quaco tracebuffer]# perf probe -D clear_tasks_mm_cpumask:1
> > p:probe/clear_tasks_mm_cpumask _text+919968
> > p:probe/clear_tasks_mm_cpumask_1 _text+919973
> > p:probe/clear_tasks_mm_cpumask_2 _text+919976
> > [root@quaco tracebuffer]#
> > [root@quaco tracebuffer]# perf probe clear_tasks_mm_cpumask
> > Probe point 'clear_tasks_mm_cpumask' not found.
> >   Error: Failed to add events.
> > [root@quaco tracebuffer]#
> > 
> > So I'll tentatively continue to apply the other patches in this series,
> > maybe one of them will fix this.
> 
> Yes, it should be fixed by [2/6] :)

Yeah, it is, works there, but unfortunately I couldn't see it in action
even having put it successfuly into place:

[root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
Added new event:
  probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask)

You can now use it in all perf tools, such as:

	perf record -e probe:clear_tasks_mm_cpumask -aR sleep 1

[root@quaco ~]# perf trace -e probe:clear_tasks_mm_cpumask/max-stack=16/

Doesn't seem to be used in x86-64...

[acme@quaco perf]$ find . -name "*.c" | xargs grep clear_tasks_mm_cpumask
./kernel/cpu.c: * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU
./kernel/cpu.c:void clear_tasks_mm_cpumask(int cpu)
./arch/xtensa/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/csky/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/sh/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/arm/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/powerpc/mm/nohash/mmu_context.c:	clear_tasks_mm_cpumask(cpu);
[acme@quaco perf]$ find . -name "*.h" | xargs grep clear_tasks_mm_cpumask
./include/linux/cpu.h:void clear_tasks_mm_cpumask(int cpu);
[acme@quaco perf]$

:-)
 
> Actually, a probe point with offset line and no-offset are handled
> a bit differently.

What is the difference? Anyway, things seem to work as advertised, so
I'm happy ;-)

- Arnaldo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline function which has no entry pc
  2019-10-25  8:46 ` [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline " Masami Hiramatsu
@ 2019-10-25 14:35   ` Arnaldo Carvalho de Melo
  2019-10-25 14:39     ` Arnaldo Carvalho de Melo
  2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
  1 sibling, 1 reply; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-25 14:35 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Em Fri, Oct 25, 2019 at 05:46:43PM +0900, Masami Hiramatsu escreveu:
> Fix perf probe to probe an inlne function which has no entry pc
> or low pc but only has ranges attribute.
> 
> This seems very rare case, but I could find a few examples, as
> same as probe_point_search_cb(), use die_entrypc() to get the
> entry address in probe_point_inline_cb() too.
> 
> Without this patch,
>   # tools/perf/perf probe -D __amd_put_nb_event_constraints
>   Failed to get entry address of __amd_put_nb_event_constraints.
>   Probe point '__amd_put_nb_event_constraints' not found.
>     Error: Failed to add events.
> 
> With this patch,
>   # tools/perf/perf probe -D __amd_put_nb_event_constraints
>   p:probe/__amd_put_nb_event_constraints amd_put_event_constraints+43

Here I got it slightly different:

Before:

  [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
  Failed to get entry address of __amd_put_nb_event_constraints.
  Probe point '__amd_put_nb_event_constraints' not found.
    Error: Failed to add events.
  [root@quaco ~]#

After:

  [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
  p:probe/__amd_put_nb_event_constraints _text+33789
  [root@quaco ~]#


----

I'm now checking if this is because I applied patch 4/6 before 3/6
 
> Fixes: 4ea42b181434 ("perf: Add perf probe subcommand, a kprobe-event setup helper")
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/probe-finder.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 71633f55f045..2fa932bcf960 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -930,7 +930,7 @@ static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
>  		ret = find_probe_point_lazy(in_die, pf);
>  	else {
>  		/* Get probe address */
> -		if (dwarf_entrypc(in_die, &addr) != 0) {
> +		if (die_entrypc(in_die, &addr) != 0) {
>  			pr_warning("Failed to get entry address of %s.\n",
>  				   dwarf_diename(in_die));
>  			return -ENOENT;

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline function which has no entry pc
  2019-10-25 14:35   ` Arnaldo Carvalho de Melo
@ 2019-10-25 14:39     ` Arnaldo Carvalho de Melo
  2019-10-26  8:04       ` Masami Hiramatsu
  0 siblings, 1 reply; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-10-25 14:39 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Em Fri, Oct 25, 2019 at 11:35:13AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Oct 25, 2019 at 05:46:43PM +0900, Masami Hiramatsu escreveu:
> > Fix perf probe to probe an inlne function which has no entry pc
> > or low pc but only has ranges attribute.
> > 
> > This seems very rare case, but I could find a few examples, as
> > same as probe_point_search_cb(), use die_entrypc() to get the
> > entry address in probe_point_inline_cb() too.
> > 
> > Without this patch,
> >   # tools/perf/perf probe -D __amd_put_nb_event_constraints
> >   Failed to get entry address of __amd_put_nb_event_constraints.
> >   Probe point '__amd_put_nb_event_constraints' not found.
> >     Error: Failed to add events.
> > 
> > With this patch,
> >   # tools/perf/perf probe -D __amd_put_nb_event_constraints
> >   p:probe/__amd_put_nb_event_constraints amd_put_event_constraints+43
> 
> Here I got it slightly different:
> 
> Before:
> 
>   [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
>   Failed to get entry address of __amd_put_nb_event_constraints.
>   Probe point '__amd_put_nb_event_constraints' not found.
>     Error: Failed to add events.
>   [root@quaco ~]#
> 
> After:
> 
>   [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
>   p:probe/__amd_put_nb_event_constraints _text+33789
>   [root@quaco ~]#
> 
> 
> ----
> 
> I'm now checking if this is because I applied patch 4/6 before 3/6

Nope, even then:

[root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
p:probe/__amd_put_nb_event_constraints _text+33789
[root@quaco ~]# grep __amd_put_nb_event_constraints /proc/kallsyms
[root@quaco ~]#

Ok, maybe this may help:

[root@quaco ~]# perf probe -v -D __amd_put_nb_event_constraints |& grep vmlinux
Looking at the vmlinux_path (8 entries long)
Using /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux for symbols
Open Debuginfo file: /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux
[root@quaco ~]#

[root@quaco ~]# readelf -wi /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux | grep __amd_put_nb_event_constraints -B1 -A7
 <1><192640>: Abbrev Number: 123 (DW_TAG_subprogram)
    <192641>   DW_AT_name        : (indirect string, offset: 0x299576): __amd_put_nb_event_constraints
    <192645>   DW_AT_decl_file   : 1
    <192646>   DW_AT_decl_line   : 361
    <192648>   DW_AT_decl_column : 13
    <192649>   DW_AT_prototyped  : 1
    <192649>   DW_AT_inline      : 1	(inlined)
    <19264a>   DW_AT_sibling     : <0x192700>
 <2><19264e>: Abbrev Number: 38 (DW_TAG_formal_parameter)
^C
[root@quaco ~]#

Continuing to process the other patches...

- Arnaldo
  
> > Fixes: 4ea42b181434 ("perf: Add perf probe subcommand, a kprobe-event setup helper")
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/util/probe-finder.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 71633f55f045..2fa932bcf960 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -930,7 +930,7 @@ static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
> >  		ret = find_probe_point_lazy(in_die, pf);
> >  	else {
> >  		/* Get probe address */
> > -		if (dwarf_entrypc(in_die, &addr) != 0) {
> > +		if (die_entrypc(in_die, &addr) != 0) {
> >  			pr_warning("Failed to get entry address of %s.\n",
> >  				   dwarf_diename(in_die));
> >  			return -ENOENT;
> 
> -- 
> 
> - Arnaldo

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline function which has no entry pc
  2019-10-25 14:39     ` Arnaldo Carvalho de Melo
@ 2019-10-26  8:04       ` Masami Hiramatsu
  0 siblings, 0 replies; 19+ messages in thread
From: Masami Hiramatsu @ 2019-10-26  8:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Namhyung Kim, linux-kernel

Hi Arnaldo 

On Fri, 25 Oct 2019 11:39:10 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Fri, Oct 25, 2019 at 11:35:13AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Oct 25, 2019 at 05:46:43PM +0900, Masami Hiramatsu escreveu:
> > > Fix perf probe to probe an inlne function which has no entry pc
> > > or low pc but only has ranges attribute.
> > > 
> > > This seems very rare case, but I could find a few examples, as
> > > same as probe_point_search_cb(), use die_entrypc() to get the
> > > entry address in probe_point_inline_cb() too.
> > > 
> > > Without this patch,
> > >   # tools/perf/perf probe -D __amd_put_nb_event_constraints
> > >   Failed to get entry address of __amd_put_nb_event_constraints.
> > >   Probe point '__amd_put_nb_event_constraints' not found.
> > >     Error: Failed to add events.
> > > 
> > > With this patch,
> > >   # tools/perf/perf probe -D __amd_put_nb_event_constraints
> > >   p:probe/__amd_put_nb_event_constraints amd_put_event_constraints+43
> > 
> > Here I got it slightly different:
> > 
> > Before:
> > 
> >   [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
> >   Failed to get entry address of __amd_put_nb_event_constraints.
> >   Probe point '__amd_put_nb_event_constraints' not found.
> >     Error: Failed to add events.
> >   [root@quaco ~]#
> > 
> > After:
> > 
> >   [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
> >   p:probe/__amd_put_nb_event_constraints _text+33789
> >   [root@quaco ~]#

Ah, sorry, it was my mistake, when I copy the command, I lacked -k option,
which means using offline kernel image. I'll update the patch description
and resend it.

With online kernel (same buildid), perf-probe modifies the address with
_text based offset for avoiding mixed up with same-name symbols. Even if
there are several same-name symbols (like local functions),  the
"_text + offset" expression can identify one of them.
However, if the buildid of the given kernel image (specified by -k option)
doesn't match, it generates event definition based on the symbol name.
Hmm, I think we can use _text even with off-line kernel, I'll check it.

> > 
> > 
> > ----
> > 
> > I'm now checking if this is because I applied patch 4/6 before 3/6
> 
> Nope, even then:
> 
> [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
> p:probe/__amd_put_nb_event_constraints _text+33789
> [root@quaco ~]# grep __amd_put_nb_event_constraints /proc/kallsyms
> [root@quaco ~]#

So, _text + offset is normal behavior. I'll try to renew the example
with actual options. Sorry for confusing.

Thank you,


> 
> Ok, maybe this may help:
> 
> [root@quaco ~]# perf probe -v -D __amd_put_nb_event_constraints |& grep vmlinux
> Looking at the vmlinux_path (8 entries long)
> Using /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux for symbols
> Open Debuginfo file: /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux
> [root@quaco ~]#
> 
> [root@quaco ~]# readelf -wi /usr/lib/debug/lib/modules/5.2.18-200.fc30.x86_64/vmlinux | grep __amd_put_nb_event_constraints -B1 -A7
>  <1><192640>: Abbrev Number: 123 (DW_TAG_subprogram)
>     <192641>   DW_AT_name        : (indirect string, offset: 0x299576): __amd_put_nb_event_constraints
>     <192645>   DW_AT_decl_file   : 1
>     <192646>   DW_AT_decl_line   : 361
>     <192648>   DW_AT_decl_column : 13
>     <192649>   DW_AT_prototyped  : 1
>     <192649>   DW_AT_inline      : 1	(inlined)
>     <19264a>   DW_AT_sibling     : <0x192700>
>  <2><19264e>: Abbrev Number: 38 (DW_TAG_formal_parameter)
> ^C
> [root@quaco ~]#
> 
> Continuing to process the other patches...
> 
> - Arnaldo
>   
> > > Fixes: 4ea42b181434 ("perf: Add perf probe subcommand, a kprobe-event setup helper")
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > ---
> > >  tools/perf/util/probe-finder.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > > index 71633f55f045..2fa932bcf960 100644
> > > --- a/tools/perf/util/probe-finder.c
> > > +++ b/tools/perf/util/probe-finder.c
> > > @@ -930,7 +930,7 @@ static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
> > >  		ret = find_probe_point_lazy(in_die, pf);
> > >  	else {
> > >  		/* Get probe address */
> > > -		if (dwarf_entrypc(in_die, &addr) != 0) {
> > > +		if (die_entrypc(in_die, &addr) != 0) {
> > >  			pr_warning("Failed to get entry address of %s.\n",
> > >  				   dwarf_diename(in_die));
> > >  			return -ENOENT;
> > 
> > -- 
> > 
> > - Arnaldo
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [tip: perf/core] perf probe: Fix to show inlined function callsite without entry_pc
  2019-10-25  8:47 ` [BUGFIX PATCH 5/6] perf/probe: Fix to show inlined function callsite without entry_pc Masami Hiramatsu
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Jiri Olsa,
	Namhyung Kim, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     18e21eb671dc87a4f0546ba505a89ea93598a634
Gitweb:        https://git.kernel.org/tip/18e21eb671dc87a4f0546ba505a89ea93598a634
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Fri, 25 Oct 2019 17:47:01 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Fix to show inlined function callsite without entry_pc

Fix 'perf probe --line' option to show inlined function callsite lines
even if the function DIE has only ranges.

Without this:

  # perf probe -L amd_put_event_constraints
  ...
      2  {
      3         if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
                        __amd_put_nb_event_constraints(cpuc, event);
      5  }

With this patch:

  # perf probe -L amd_put_event_constraints
  ...
      2  {
      3         if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
      4                 __amd_put_nb_event_constraints(cpuc, event);
      5  }

Committer testing:

Before:

  [root@quaco ~]# perf probe -L amd_put_event_constraints
  <amd_put_event_constraints@/usr/src/debug/kernel-5.2.fc30/linux-5.2.18-200.fc30.x86_64/arch/x86/events/amd/core.c:0>
        0  static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
                                                struct perf_event *event)
        2  {
        3         if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
                          __amd_put_nb_event_constraints(cpuc, event);
        5  }

           PMU_FORMAT_ATTR(event, "config:0-7,32-35");
           PMU_FORMAT_ATTR(umask, "config:8-15"   );

  [root@quaco ~]#

After:

  [root@quaco ~]# perf probe -L amd_put_event_constraints
  <amd_put_event_constraints@/usr/src/debug/kernel-5.2.fc30/linux-5.2.18-200.fc30.x86_64/arch/x86/events/amd/core.c:0>
        0  static void amd_put_event_constraints(struct cpu_hw_events *cpuc,
                                                struct perf_event *event)
        2  {
        3         if (amd_has_nb(cpuc) && amd_is_nb_event(&event->hw))
        4                 __amd_put_nb_event_constraints(cpuc, event);
        5  }

           PMU_FORMAT_ATTR(event, "config:0-7,32-35");
           PMU_FORMAT_ATTR(umask, "config:8-15"   );

  [root@quaco ~]# perf probe amd_put_event_constraints:4
  Added new event:
    probe:amd_put_event_constraints (on amd_put_event_constraints:4)

  You can now use it in all perf tools, such as:

  	perf record -e probe:amd_put_event_constraints -aR sleep 1

  [root@quaco ~]#

  [root@quaco ~]# perf probe -l
    probe:amd_put_event_constraints (on amd_put_event_constraints:4@arch/x86/events/amd/core.c)
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask@kernel/cpu.c)
  [root@quaco ~]#

Using it:

  [root@quaco ~]# perf trace -e probe:*
  ^C[root@quaco ~]#

Ok, Intel system here... :-)

Fixes: 4cc9cec636e7 ("perf probe: Introduce lines walker interface")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157199322107.8075.12659099000567865708.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 063f71d..e0c507d 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -695,7 +695,7 @@ static int __die_walk_funclines_cb(Dwarf_Die *in_die, void *data)
 	if (dwarf_tag(in_die) == DW_TAG_inlined_subroutine) {
 		fname = die_get_call_file(in_die);
 		lineno = die_get_call_lineno(in_die);
-		if (fname && lineno > 0 && dwarf_entrypc(in_die, &addr) == 0) {
+		if (fname && lineno > 0 && die_entrypc(in_die, &addr) == 0) {
 			lw->retval = lw->callback(fname, lineno, addr, lw->data);
 			if (lw->retval != 0)
 				return DIE_FIND_CB_END;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [tip: perf/core] perf probe: Fix to show ranges of variables in functions without entry_pc
  2019-10-25  8:47 ` [BUGFIX PATCH 6/6] perf/probe: Fix to show ranges of variables in functions " Masami Hiramatsu
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Jiri Olsa,
	Namhyung Kim, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     af04dd2f8ebaa8fbd46f698714acbf43da14da45
Gitweb:        https://git.kernel.org/tip/af04dd2f8ebaa8fbd46f698714acbf43da14da45
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Fri, 25 Oct 2019 17:47:10 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Fix to show ranges of variables in functions without entry_pc

Fix to show ranges of variables (--range and --vars option) in functions
which DIE has only ranges but no entry_pc attribute.

Without this fix:

  # perf probe --range -V clear_tasks_mm_cpumask
  Available variables at clear_tasks_mm_cpumask
  	@<clear_tasks_mm_cpumask+0>
  		(No matched variables)

With this fix:

  # perf probe --range -V clear_tasks_mm_cpumask
  Available variables at clear_tasks_mm_cpumask
	@<clear_tasks_mm_cpumask+0>
		[VAL]	int	cpu	@<clear_tasks_mm_cpumask+[0-35,317-317,2052-2059]>

Committer testing:

Before:

  [root@quaco ~]# perf probe --range -V clear_tasks_mm_cpumask
  Available variables at clear_tasks_mm_cpumask
          @<clear_tasks_mm_cpumask+0>
                  (No matched variables)
  [root@quaco ~]#

After:

  [root@quaco ~]# perf probe --range -V clear_tasks_mm_cpumask
  Available variables at clear_tasks_mm_cpumask
          @<clear_tasks_mm_cpumask+0>
                  [VAL]   int     cpu     @<clear_tasks_mm_cpumask+[0-23,23-105,105-106,106-106,1843-1850,1850-1862]>
  [root@quaco ~]#

Using it:

  [root@quaco ~]# perf probe clear_tasks_mm_cpumask cpu
  Added new event:
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask with cpu)

  You can now use it in all perf tools, such as:

  	perf record -e probe:clear_tasks_mm_cpumask -aR sleep 1

  [root@quaco ~]# perf probe -l
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask@kernel/cpu.c with cpu)
  [root@quaco ~]#
  [root@quaco ~]# perf trace -e probe:*cpumask
  ^C[root@quaco ~]#

Fixes: 349e8d261131 ("perf probe: Add --range option to show a variable's location range")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157199323018.8075.8179744380479673672.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index e0c507d..ac82fd9 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1019,7 +1019,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
 	bool first = true;
 	const char *name;
 
-	ret = dwarf_entrypc(sp_die, &entry);
+	ret = die_entrypc(sp_die, &entry);
 	if (ret)
 		return ret;
 
@@ -1082,7 +1082,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
 	bool first = true;
 	const char *name;
 
-	ret = dwarf_entrypc(sp_die, &entry);
+	ret = die_entrypc(sp_die, &entry);
 	if (ret)
 		return ret;
 

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [tip: perf/core] perf probe: Fix to list probe event with correct line number
  2019-10-25  8:46 ` [BUGFIX PATCH 4/6] perf/probe: Fix to list probe event with correct line number Masami Hiramatsu
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Jiri Olsa,
	Namhyung Kim, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     3895534dd78f0fd4d3f9e05ee52b9cdd444a743e
Gitweb:        https://git.kernel.org/tip/3895534dd78f0fd4d3f9e05ee52b9cdd444a743e
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Fri, 25 Oct 2019 17:46:52 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Fix to list probe event with correct line number

Since debuginfo__find_probe_point() uses dwarf_entrypc() for finding the
entry address of the function on which a probe is, it will fail when the
function DIE has only ranges attribute.

To fix this issue, use die_entrypc() instead of dwarf_entrypc().

Without this fix, perf probe -l shows incorrect offset:

  # perf probe -l
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask+18446744071579263632@work/linux/linux/kernel/cpu.c)
    probe:clear_tasks_mm_cpumask_1 (on clear_tasks_mm_cpumask+18446744071579263752@work/linux/linux/kernel/cpu.c)

With this:

  # perf probe -l
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask@work/linux/linux/kernel/cpu.c)
    probe:clear_tasks_mm_cpumask_1 (on clear_tasks_mm_cpumask:21@work/linux/linux/kernel/cpu.c)

Committer testing:

Before:

  [root@quaco ~]# perf probe -l
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask+18446744071579765152@kernel/cpu.c)
  [root@quaco ~]#

After:

  [root@quaco ~]# perf probe -l
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask@kernel/cpu.c)
  [root@quaco ~]#

Fixes: 1d46ea2a6a40 ("perf probe: Fix listing incorrect line number with inline function")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157199321227.8075.14655572419136993015.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 2fa932b..88e17a4 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1566,7 +1566,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
 		/* Get function entry information */
 		func = basefunc = dwarf_diename(&spdie);
 		if (!func ||
-		    dwarf_entrypc(&spdie, &baseaddr) != 0 ||
+		    die_entrypc(&spdie, &baseaddr) != 0 ||
 		    dwarf_decl_line(&spdie, &baseline) != 0) {
 			lineno = 0;
 			goto post;
@@ -1583,7 +1583,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, unsigned long addr,
 		while (die_find_top_inlinefunc(&spdie, (Dwarf_Addr)addr,
 						&indie)) {
 			/* There is an inline function */
-			if (dwarf_entrypc(&indie, &_addr) == 0 &&
+			if (die_entrypc(&indie, &_addr) == 0 &&
 			    _addr == addr) {
 				/*
 				 * addr is at an inline function entry.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [tip: perf/core] perf probe: Fix to probe a function which has no entry pc
  2019-10-25  8:46 ` [BUGFIX PATCH 2/6] perf/probe: Fix to probe a function which has no entry pc Masami Hiramatsu
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  0 siblings, 0 replies; 19+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo,
	Masami Hiramatsu, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     5d16dbcc311d91267ddb45c6da4f187be320ecee
Gitweb:        https://git.kernel.org/tip/5d16dbcc311d91267ddb45c6da4f187be320ecee
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Fri, 25 Oct 2019 17:46:34 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Fix to probe a function which has no entry pc

Fix 'perf probe' to probe a function which has no entry pc or low pc but
only has ranges attribute.

probe_point_search_cb() uses dwarf_entrypc() to get the probe address,
but that doesn't work for the function DIE which has only ranges
attribute. Use die_entrypc() instead.

Without this fix:

  # perf probe -k ../build-x86_64/vmlinux -D clear_tasks_mm_cpumask:0
  Probe point 'clear_tasks_mm_cpumask' not found.
    Error: Failed to add events.

With this:

  # perf probe -k ../build-x86_64/vmlinux -D clear_tasks_mm_cpumask:0
  p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0

Committer testing:

Before:

  [root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
  Probe point 'clear_tasks_mm_cpumask' not found.
    Error: Failed to add events.
  [root@quaco ~]#

After:

  [root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
  Added new event:
    probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask)

  You can now use it in all perf tools, such as:

  	perf record -e probe:clear_tasks_mm_cpumask -aR sleep 1

  [root@quaco ~]#

Using it with 'perf trace':

  [root@quaco ~]# perf trace -e probe:clear_tasks_mm_cpumask

Doesn't seem to be used in x86_64:

  $ find . -name "*.c" | xargs grep clear_tasks_mm_cpumask
  ./kernel/cpu.c: * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU
  ./kernel/cpu.c:void clear_tasks_mm_cpumask(int cpu)
  ./arch/xtensa/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
  ./arch/csky/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
  ./arch/sh/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
  ./arch/arm/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
  ./arch/powerpc/mm/nohash/mmu_context.c:	clear_tasks_mm_cpumask(cpu);
  $ find . -name "*.h" | xargs grep clear_tasks_mm_cpumask
  ./include/linux/cpu.h:void clear_tasks_mm_cpumask(int cpu);
  $ find . -name "*.S" | xargs grep clear_tasks_mm_cpumask
  $

Fixes: e1ecbbc3fa83 ("perf probe: Fix to handle optimized not-inlined functions")
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157199319438.8075.4695576954550638618.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 2b6513e..71633f5 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -982,7 +982,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
 		param->retval = find_probe_point_by_line(pf);
 	} else if (die_is_func_instance(sp_die)) {
 		/* Instances always have the entry address */
-		dwarf_entrypc(sp_die, &pf->addr);
+		die_entrypc(sp_die, &pf->addr);
 		/* But in some case the entry address is 0 */
 		if (pf->addr == 0) {
 			pr_debug("%s has no entry PC. Skipped\n",

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [tip: perf/core] perf probe: Fix to probe an inline function which has no entry pc
  2019-10-25  8:46 ` [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline " Masami Hiramatsu
  2019-10-25 14:35   ` Arnaldo Carvalho de Melo
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 19+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Masami Hiramatsu, Arnaldo Carvalho de Melo, Jiri Olsa,
	Namhyung Kim, Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     eb6933b29d20bf2c3053883d409a53f462c1a3ac
Gitweb:        https://git.kernel.org/tip/eb6933b29d20bf2c3053883d409a53f462c1a3ac
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Fri, 25 Oct 2019 17:46:43 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Fix to probe an inline function which has no entry pc

Fix perf probe to probe an inlne function which has no entry pc
or low pc but only has ranges attribute.

This seems very rare case, but I could find a few examples, as
same as probe_point_search_cb(), use die_entrypc() to get the
entry address in probe_point_inline_cb() too.

Without this patch:

  # perf probe -D __amd_put_nb_event_constraints
  Failed to get entry address of __amd_put_nb_event_constraints.
  Probe point '__amd_put_nb_event_constraints' not found.
    Error: Failed to add events.

With this patch:

  # perf probe -D __amd_put_nb_event_constraints
  p:probe/__amd_put_nb_event_constraints amd_put_event_constraints+43

Committer testing:

Before:

  [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
  Failed to get entry address of __amd_put_nb_event_constraints.
  Probe point '__amd_put_nb_event_constraints' not found.
    Error: Failed to add events.
  [root@quaco ~]#

After:

  [root@quaco ~]# perf probe -D __amd_put_nb_event_constraints
  p:probe/__amd_put_nb_event_constraints _text+33789
  [root@quaco ~]#

Fixes: 4ea42b181434 ("perf: Add perf probe subcommand, a kprobe-event setup helper")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157199320336.8075.16189530425277588587.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 71633f5..2fa932b 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -930,7 +930,7 @@ static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
 		ret = find_probe_point_lazy(in_die, pf);
 	else {
 		/* Get probe address */
-		if (dwarf_entrypc(in_die, &addr) != 0) {
+		if (die_entrypc(in_die, &addr) != 0) {
 			pr_warning("Failed to get entry address of %s.\n",
 				   dwarf_diename(in_die));
 			return -ENOENT;

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [tip: perf/core] perf probe: Fix wrong address verification
  2019-10-25  8:46 ` [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification Masami Hiramatsu
  2019-10-25 12:14   ` Arnaldo Carvalho de Melo
@ 2019-11-12 11:18   ` tip-bot2 for Masami Hiramatsu
  1 sibling, 0 replies; 19+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2019-11-12 11:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Arnaldo Carvalho de Melo, Arnaldo Carvalho de Melo,
	Masami Hiramatsu, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     07d369857808b7e8e471bbbbb0074a6718f89b31
Gitweb:        https://git.kernel.org/tip/07d369857808b7e8e471bbbbb0074a6718f89b31
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Fri, 25 Oct 2019 17:46:25 +09:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 06 Nov 2019 15:43:06 -03:00

perf probe: Fix wrong address verification

Since there are some DIE which has only ranges instead of the
combination of entrypc/highpc, address verification must use
dwarf_haspc() instead of dwarf_entrypc/dwarf_highpc.

Also, the ranges only DIE will have a partial code in different section
(e.g. unlikely code will be in text.unlikely as "FUNC.cold" symbol). In
that case, we can not use dwarf_entrypc() or die_entrypc(), because the
offset from original DIE can be a minus value.

Instead, this simply gets the symbol and offset from symtab.

Without this patch;

  # perf probe -D clear_tasks_mm_cpumask:1
  Failed to get entry address of clear_tasks_mm_cpumask
    Error: Failed to add events.

And with this patch:

  # perf probe -D clear_tasks_mm_cpumask:1
  p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0
  p:probe/clear_tasks_mm_cpumask_1 clear_tasks_mm_cpumask+5
  p:probe/clear_tasks_mm_cpumask_2 clear_tasks_mm_cpumask+8
  p:probe/clear_tasks_mm_cpumask_3 clear_tasks_mm_cpumask+16
  p:probe/clear_tasks_mm_cpumask_4 clear_tasks_mm_cpumask+82

Committer testing:

I managed to reproduce the above:

  [root@quaco ~]# perf probe -D clear_tasks_mm_cpumask:1
  p:probe/clear_tasks_mm_cpumask _text+919968
  p:probe/clear_tasks_mm_cpumask_1 _text+919973
  p:probe/clear_tasks_mm_cpumask_2 _text+919976
  [root@quaco ~]#

But then when trying to actually put the probe in place, it fails if I
use :0 as the offset:

  [root@quaco ~]# perf probe -L clear_tasks_mm_cpumask | head -5
  <clear_tasks_mm_cpumask@/usr/src/debug/kernel-5.2.fc30/linux-5.2.18-200.fc30.x86_64/kernel/cpu.c:0>
        0  void clear_tasks_mm_cpumask(int cpu)
        1  {
        2  	struct task_struct *p;

  [root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
  Probe point 'clear_tasks_mm_cpumask' not found.
    Error: Failed to add events.
  [root@quaco

The next patch is needed to fix this case.

Fixes: 576b523721b7 ("perf probe: Fix probing symbols with optimization suffix")
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lore.kernel.org/lkml/157199318513.8075.10463906803299647907.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index cd9f95e..2b6513e 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -604,38 +604,26 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwfl_Module *mod,
 				  const char *function,
 				  struct probe_trace_point *tp)
 {
-	Dwarf_Addr eaddr, highaddr;
+	Dwarf_Addr eaddr;
 	GElf_Sym sym;
 	const char *symbol;
 
 	/* Verify the address is correct */
-	if (dwarf_entrypc(sp_die, &eaddr) != 0) {
-		pr_warning("Failed to get entry address of %s\n",
-			   dwarf_diename(sp_die));
-		return -ENOENT;
-	}
-	if (dwarf_highpc(sp_die, &highaddr) != 0) {
-		pr_warning("Failed to get end address of %s\n",
-			   dwarf_diename(sp_die));
-		return -ENOENT;
-	}
-	if (paddr > highaddr) {
-		pr_warning("Offset specified is greater than size of %s\n",
+	if (!dwarf_haspc(sp_die, paddr)) {
+		pr_warning("Specified offset is out of %s\n",
 			   dwarf_diename(sp_die));
 		return -EINVAL;
 	}
 
-	symbol = dwarf_diename(sp_die);
+	/* Try to get actual symbol name from symtab */
+	symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
 	if (!symbol) {
-		/* Try to get the symbol name from symtab */
-		symbol = dwfl_module_addrsym(mod, paddr, &sym, NULL);
-		if (!symbol) {
-			pr_warning("Failed to find symbol at 0x%lx\n",
-				   (unsigned long)paddr);
-			return -ENOENT;
-		}
-		eaddr = sym.st_value;
+		pr_warning("Failed to find symbol at 0x%lx\n",
+			   (unsigned long)paddr);
+		return -ENOENT;
 	}
+	eaddr = sym.st_value;
+
 	tp->offset = (unsigned long)(paddr - eaddr);
 	tp->address = (unsigned long)paddr;
 	tp->symbol = strdup(symbol);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2019-11-12 11:21 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-25  8:46 [BUGFIX PATCH 0/6] perf/probe: Additional fixes for range only functions Masami Hiramatsu
2019-10-25  8:46 ` [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification Masami Hiramatsu
2019-10-25 12:14   ` Arnaldo Carvalho de Melo
2019-10-25 12:36     ` Masami Hiramatsu
2019-10-25 14:28       ` Arnaldo Carvalho de Melo
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-25  8:46 ` [BUGFIX PATCH 2/6] perf/probe: Fix to probe a function which has no entry pc Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-25  8:46 ` [BUGFIX PATCH 3/6] perf/probe: Fix to probe an inline " Masami Hiramatsu
2019-10-25 14:35   ` Arnaldo Carvalho de Melo
2019-10-25 14:39     ` Arnaldo Carvalho de Melo
2019-10-26  8:04       ` Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-25  8:46 ` [BUGFIX PATCH 4/6] perf/probe: Fix to list probe event with correct line number Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-25  8:47 ` [BUGFIX PATCH 5/6] perf/probe: Fix to show inlined function callsite without entry_pc Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu
2019-10-25  8:47 ` [BUGFIX PATCH 6/6] perf/probe: Fix to show ranges of variables in functions " Masami Hiramatsu
2019-11-12 11:18   ` [tip: perf/core] perf probe: " tip-bot2 for Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).