[PATCH 1/2] perf probe: Helper function to check if probe with variable

[PATCH 1/2] perf probe: Helper function to check if probe with variable

[Ravi Bangoria]

Introduce helper function instead of inline code and replace hardcoded
strings "$vars" and "$params" with their corresponding macros.

perf_probe_with_var is not declared as static since it will be called
from different file in subsequent patch.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/probe-event.c | 22 +++++++++++++++-------
 tools/perf/util/probe-event.h |  2 ++
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 953dc1a..bc9317e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1592,19 +1592,27 @@ out:
 	return ret;
 }
 
+/* Returns true if *any* ARG is either C variable, $params or $vars. */
+bool perf_probe_with_var(struct perf_probe_event *pev)
+{
+	int i = 0;
+
+	for (i = 0; i < pev->nargs; i++)
+		if (is_c_varname(pev->args[i].var)              ||
+		    !strcmp(pev->args[i].var, PROBE_ARG_PARAMS) ||
+		    !strcmp(pev->args[i].var, PROBE_ARG_VARS))
+			return true;
+	return false;
+}
+
 /* Return true if this perf_probe_event requires debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev)
 {
-	int i;
-
 	if (pev->point.file || pev->point.line || pev->point.lazy_line)
 		return true;
 
-	for (i = 0; i < pev->nargs; i++)
-		if (is_c_varname(pev->args[i].var) ||
-		    !strcmp(pev->args[i].var, "$params") ||
-		    !strcmp(pev->args[i].var, "$vars"))
-			return true;
+	if (perf_probe_with_var(pev))
+		return true;
 
 	return false;
 }
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e18ea9f..4d1139b 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -128,6 +128,8 @@ char *synthesize_perf_probe_point(struct perf_probe_point *pp);
 int perf_probe_event__copy(struct perf_probe_event *dst,
 			   struct perf_probe_event *src);
 
+bool perf_probe_with_var(struct perf_probe_event *pev);
+
 /* Check the perf_probe_event needs debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
 
-- 
2.5.5

[PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Ravi Bangoria]

Function prologue prepares stack and registers before executing function
logic. When target program is compiled without optimization, function
parameter information is only valid after prologue. When we probe entrypc
of the function, and try to record function parameter, it contains
garbage value.

For example,
  $ vim test.c
    #include <stdio.h>

    void foo(int i)
    {
       printf("i: %d\n", i);
    }

    int main()
    {
      foo(42);
      return 0;
    }

  $ gcc -g test.c -o test
  $ objdump -dl test | less
    foo():
    /home/ravi/test.c:4
      400536:       55                      push   %rbp
      400537:       48 89 e5                mov    %rsp,%rbp
      40053a:       48 83 ec 10             sub    -bashx10,%rsp
      40053e:       89 7d fc                mov    %edi,-0x4(%rbp)
    /home/ravi/test.c:5
      400541:       8b 45 fc                mov    -0x4(%rbp),%eax
    ...
    ...
    main():
    /home/ravi/test.c:9
      400558:       55                      push   %rbp
      400559:       48 89 e5                mov    %rsp,%rbp
    /home/ravi/test.c:10
      40055c:       bf 2a 00 00 00          mov    -bashx2a,%edi
      400561:       e8 d0 ff ff ff          callq  400536 <foo>
    /home/ravi/test.c:11

  $ ./perf probe -x ./test 'foo i'
  $ cat /sys/kernel/debug/tracing/uprobe_events
     p:probe_test/foo /home/ravi/test:0x0000000000000536 i=-12(%sp):s32

  $ ./perf record -e probe_test:foo ./test
  $ ./perf script
     test  5778 [001]  4918.562027: probe_test:foo: (400536) i=0

Here variable 'i' is passed via stack which is pushed on stack at
0x40053e. But we are probing at 0x400536.

To resolve this issues, we need to probe on next instruction after
prologue. gdb and systemtap also does same thing. I've implemented
this patch based on approach systemtap has used.

After applying patch:

  $ ./perf probe -x ./test 'foo i'
  $ cat /sys/kernel/debug/tracing/uprobe_events
    p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32

  $ ./perf record -e probe_test:foo ./test
  $ ./perf script
    test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42

No need to skip prologue for optimized case since debug info is correct
for each instructions for -O2 -g. For more details please visit:
        https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
Changes in v2:
  - Skipping prologue only when any ARG is either C variable, $params
    or $vars.
  - Probe on line(:1) may not be always possible. Recommend only address
    to force probe on function entry.

 tools/perf/util/probe-finder.c | 164 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f2d9ff0..b2bc77c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
 	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
 }
 
+static bool var_has_loclist(Dwarf_Die *die)
+{
+	Dwarf_Attribute loc;
+	int tag = dwarf_tag(die);
+
+	if (tag != DW_TAG_formal_parameter &&
+	    tag != DW_TAG_variable)
+		return false;
+
+	return (dwarf_attr_integrate(die, DW_AT_location, &loc) &&
+		dwarf_whatform(&loc) == DW_FORM_sec_offset);
+}
+
+/*
+ * For any object in given CU whose DW_AT_location is a location list,
+ * target program is compiled with optimization.
+ */
+static bool optimized_target(Dwarf_Die *die)
+{
+	Dwarf_Die tmp_die;
+
+	if (var_has_loclist(die))
+		return true;
+
+	if (!dwarf_child(die, &tmp_die) && optimized_target(&tmp_die))
+		return true;
+
+	if (!dwarf_siblingof(die, &tmp_die) && optimized_target(&tmp_die))
+		return true;
+
+	return false;
+}
+
+static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
+			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
+{
+	unsigned long i;
+	Dwarf_Addr addr;
+
+	for (i = 0; i < nr_lines; i++) {
+		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
+			return false;
+
+		if (addr == pf_addr) {
+			*entrypc_idx = i;
+			return true;
+		}
+	}
+	return false;
+}
+
+static bool get_postprologue_addr(unsigned long entrypc_idx,
+				  Dwarf_Lines *lines,
+				  unsigned long nr_lines,
+				  Dwarf_Addr highpc,
+				  Dwarf_Addr *postprologue_addr)
+{
+	unsigned long i;
+	int entrypc_lno, lno;
+	Dwarf_Line *line;
+	Dwarf_Addr addr;
+	bool p_end;
+
+	/* entrypc_lno is actual source line number */
+	line = dwarf_onesrcline(lines, entrypc_idx);
+	if (dwarf_lineno(line, &entrypc_lno))
+		return false;
+
+	for (i = entrypc_idx; i < nr_lines; i++) {
+		line = dwarf_onesrcline(lines, i);
+
+		if (dwarf_lineaddr(line, &addr) ||
+		    dwarf_lineno(line, &lno)    ||
+		    dwarf_lineprologueend(line, &p_end))
+			return false;
+
+		/* highpc is exclusive. [entrypc,highpc) */
+		if (addr >= highpc)
+			break;
+
+		/* clang supports prologue-end marker */
+		if (p_end)
+			break;
+
+		/* Actual next line in source */
+		if (lno != entrypc_lno)
+			break;
+
+		/*
+		 * Single source line can have multiple line records.
+		 * For Example,
+		 *     void foo() { printf("hello\n"); }
+		 * contains two line records. One points to declaration and
+		 * other points to printf() line. Variable 'lno' won't get
+		 * incremented in this case but 'i' will.
+		 */
+		if (i != entrypc_idx)
+			break;
+	}
+
+	dwarf_lineaddr(line, postprologue_addr);
+	if (*postprologue_addr >= highpc)
+		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
+			       postprologue_addr);
+
+	return true;
+}
+
+static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
+{
+	unsigned long nr_lines = 0, entrypc_idx = 0;
+	Dwarf_Lines *lines = NULL;
+	Dwarf_Addr postprologue_addr;
+	Dwarf_Addr highpc;
+
+	if (dwarf_highpc(sp_die, &highpc))
+		return;
+
+	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
+		return;
+
+	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
+		return;
+
+	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
+				   highpc, &postprologue_addr))
+		return;
+
+	pf->addr = postprologue_addr;
+}
+
+static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
+{
+	struct perf_probe_point *pp = &pf->pev->point;
+
+	/* Not uprobe? */
+	if (!pf->pev->uprobes)
+		return;
+
+	/* Compiled with optimization? */
+	if (optimized_target(&pf->cu_die))
+		return;
+
+	/* Don't know entrypc? */
+	if (!pf->addr)
+		return;
+
+	/* Only FUNC and FUNC@SRC are eligible. */
+	if (!pp->function || pp->line || pp->retprobe || pp->lazy_line ||
+	    pp->offset || pp->abs_address)
+		return;
+
+	/* Not interested in func parameter? */
+	if (!perf_probe_with_var(pf->pev))
+		return;
+
+	pr_info("Target program is compiled without optimization. Skipping prologue.\n"
+		"Probe on address 0x%lx to force probing at the function entry.\n\n",
+		pf->addr);
+
+	__skip_prologue(sp_die, pf);
+}
+
 static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
 {
 	struct probe_finder *pf = data;
@@ -954,6 +1117,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
 		if (pp->lazy_line)
 			param->retval = find_probe_point_lazy(sp_die, pf);
 		else {
+			skip_prologue(sp_die, pf);
 			pf->addr += pp->offset;
 			/* TODO: Check the address in this function */
 			param->retval = call_probe_finder(sp_die, pf);
-- 
2.5.5

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Ravi Bangoria]

Hi Masami, Arnaldo,

Any updates on this?

Thanks,
Ravi

On Wednesday 03 August 2016 02:28 PM, Ravi Bangoria wrote:
> Function prologue prepares stack and registers before executing function
> logic. When target program is compiled without optimization, function
> parameter information is only valid after prologue. When we probe entrypc
> of the function, and try to record function parameter, it contains
> garbage value.
>
> For example,
>   $ vim test.c
>     #include <stdio.h>
>
>     void foo(int i)
>     {
>        printf("i: %d\n", i);
>     }
>
>     int main()
>     {
>       foo(42);
>       return 0;
>     }
>
>   $ gcc -g test.c -o test
>   $ objdump -dl test | less
>     foo():
>     /home/ravi/test.c:4
>       400536:       55                      push   %rbp
>       400537:       48 89 e5                mov    %rsp,%rbp
>       40053a:       48 83 ec 10             sub    -bashx10,%rsp
>       40053e:       89 7d fc                mov    %edi,-0x4(%rbp)
>     /home/ravi/test.c:5
>       400541:       8b 45 fc                mov    -0x4(%rbp),%eax
>     ...
>     ...
>     main():
>     /home/ravi/test.c:9
>       400558:       55                      push   %rbp
>       400559:       48 89 e5                mov    %rsp,%rbp
>     /home/ravi/test.c:10
>       40055c:       bf 2a 00 00 00          mov    -bashx2a,%edi
>       400561:       e8 d0 ff ff ff          callq  400536 <foo>
>     /home/ravi/test.c:11
>
>   $ ./perf probe -x ./test 'foo i'
>   $ cat /sys/kernel/debug/tracing/uprobe_events
>      p:probe_test/foo /home/ravi/test:0x0000000000000536 i=-12(%sp):s32
>
>   $ ./perf record -e probe_test:foo ./test
>   $ ./perf script
>      test  5778 [001]  4918.562027: probe_test:foo: (400536) i=0
>
> Here variable 'i' is passed via stack which is pushed on stack at
> 0x40053e. But we are probing at 0x400536.
>
> To resolve this issues, we need to probe on next instruction after
> prologue. gdb and systemtap also does same thing. I've implemented
> this patch based on approach systemtap has used.
>
> After applying patch:
>
>   $ ./perf probe -x ./test 'foo i'
>   $ cat /sys/kernel/debug/tracing/uprobe_events
>     p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32
>
>   $ ./perf record -e probe_test:foo ./test
>   $ ./perf script
>     test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42
>
> No need to skip prologue for optimized case since debug info is correct
> for each instructions for -O2 -g. For more details please visit:
>         https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> Changes in v2:
>   - Skipping prologue only when any ARG is either C variable, $params
>     or $vars.
>   - Probe on line(:1) may not be always possible. Recommend only address
>     to force probe on function entry.
>
>  tools/perf/util/probe-finder.c | 164 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 164 insertions(+)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f2d9ff0..b2bc77c 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
>  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
>  }
>
> +static bool var_has_loclist(Dwarf_Die *die)
> +{
> +	Dwarf_Attribute loc;
> +	int tag = dwarf_tag(die);
> +
> +	if (tag != DW_TAG_formal_parameter &&
> +	    tag != DW_TAG_variable)
> +		return false;
> +
> +	return (dwarf_attr_integrate(die, DW_AT_location, &loc) &&
> +		dwarf_whatform(&loc) == DW_FORM_sec_offset);
> +}
> +
> +/*
> + * For any object in given CU whose DW_AT_location is a location list,
> + * target program is compiled with optimization.
> + */
> +static bool optimized_target(Dwarf_Die *die)
> +{
> +	Dwarf_Die tmp_die;
> +
> +	if (var_has_loclist(die))
> +		return true;
> +
> +	if (!dwarf_child(die, &tmp_die) && optimized_target(&tmp_die))
> +		return true;
> +
> +	if (!dwarf_siblingof(die, &tmp_die) && optimized_target(&tmp_die))
> +		return true;
> +
> +	return false;
> +}
> +
> +static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
> +			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
> +{
> +	unsigned long i;
> +	Dwarf_Addr addr;
> +
> +	for (i = 0; i < nr_lines; i++) {
> +		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
> +			return false;
> +
> +		if (addr == pf_addr) {
> +			*entrypc_idx = i;
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +static bool get_postprologue_addr(unsigned long entrypc_idx,
> +				  Dwarf_Lines *lines,
> +				  unsigned long nr_lines,
> +				  Dwarf_Addr highpc,
> +				  Dwarf_Addr *postprologue_addr)
> +{
> +	unsigned long i;
> +	int entrypc_lno, lno;
> +	Dwarf_Line *line;
> +	Dwarf_Addr addr;
> +	bool p_end;
> +
> +	/* entrypc_lno is actual source line number */
> +	line = dwarf_onesrcline(lines, entrypc_idx);
> +	if (dwarf_lineno(line, &entrypc_lno))
> +		return false;
> +
> +	for (i = entrypc_idx; i < nr_lines; i++) {
> +		line = dwarf_onesrcline(lines, i);
> +
> +		if (dwarf_lineaddr(line, &addr) ||
> +		    dwarf_lineno(line, &lno)    ||
> +		    dwarf_lineprologueend(line, &p_end))
> +			return false;
> +
> +		/* highpc is exclusive. [entrypc,highpc) */
> +		if (addr >= highpc)
> +			break;
> +
> +		/* clang supports prologue-end marker */
> +		if (p_end)
> +			break;
> +
> +		/* Actual next line in source */
> +		if (lno != entrypc_lno)
> +			break;
> +
> +		/*
> +		 * Single source line can have multiple line records.
> +		 * For Example,
> +		 *     void foo() { printf("hello\n"); }
> +		 * contains two line records. One points to declaration and
> +		 * other points to printf() line. Variable 'lno' won't get
> +		 * incremented in this case but 'i' will.
> +		 */
> +		if (i != entrypc_idx)
> +			break;
> +	}
> +
> +	dwarf_lineaddr(line, postprologue_addr);
> +	if (*postprologue_addr >= highpc)
> +		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> +			       postprologue_addr);
> +
> +	return true;
> +}
> +
> +static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> +{
> +	unsigned long nr_lines = 0, entrypc_idx = 0;
> +	Dwarf_Lines *lines = NULL;
> +	Dwarf_Addr postprologue_addr;
> +	Dwarf_Addr highpc;
> +
> +	if (dwarf_highpc(sp_die, &highpc))
> +		return;
> +
> +	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
> +		return;
> +
> +	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
> +		return;
> +
> +	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
> +				   highpc, &postprologue_addr))
> +		return;
> +
> +	pf->addr = postprologue_addr;
> +}
> +
> +static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> +{
> +	struct perf_probe_point *pp = &pf->pev->point;
> +
> +	/* Not uprobe? */
> +	if (!pf->pev->uprobes)
> +		return;
> +
> +	/* Compiled with optimization? */
> +	if (optimized_target(&pf->cu_die))
> +		return;
> +
> +	/* Don't know entrypc? */
> +	if (!pf->addr)
> +		return;
> +
> +	/* Only FUNC and FUNC@SRC are eligible. */
> +	if (!pp->function || pp->line || pp->retprobe || pp->lazy_line ||
> +	    pp->offset || pp->abs_address)
> +		return;
> +
> +	/* Not interested in func parameter? */
> +	if (!perf_probe_with_var(pf->pev))
> +		return;
> +
> +	pr_info("Target program is compiled without optimization. Skipping prologue.\n"
> +		"Probe on address 0x%lx to force probing at the function entry.\n\n",
> +		pf->addr);
> +
> +	__skip_prologue(sp_die, pf);
> +}
> +
>  static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
>  {
>  	struct probe_finder *pf = data;
> @@ -954,6 +1117,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
>  		if (pp->lazy_line)
>  			param->retval = find_probe_point_lazy(sp_die, pf);
>  		else {
> +			skip_prologue(sp_die, pf);
>  			pf->addr += pp->offset;
>  			/* TODO: Check the address in this function */
>  			param->retval = call_probe_finder(sp_die, pf);

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Masami Hiramatsu]

Hi Ravi,

On Sat, 13 Aug 2016 19:15:53 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> Hi Masami, Arnaldo,
> 
> Any updates on this?

Oops, sorry I missed this mail, I was on vacation...


> 
> Thanks,
> Ravi
> 
> On Wednesday 03 August 2016 02:28 PM, Ravi Bangoria wrote:
> > Function prologue prepares stack and registers before executing function
> > logic. When target program is compiled without optimization, function
> > parameter information is only valid after prologue. When we probe entrypc
> > of the function, and try to record function parameter, it contains
> > garbage value.
> >
> > For example,
> >   $ vim test.c
> >     #include <stdio.h>
> >
> >     void foo(int i)
> >     {
> >        printf("i: %d\n", i);
> >     }
> >
> >     int main()
> >     {
> >       foo(42);
> >       return 0;
> >     }
> >
> >   $ gcc -g test.c -o test
> >   $ objdump -dl test | less
> >     foo():
> >     /home/ravi/test.c:4
> >       400536:       55                      push   %rbp
> >       400537:       48 89 e5                mov    %rsp,%rbp
> >       40053a:       48 83 ec 10             sub    -bashx10,%rsp
> >       40053e:       89 7d fc                mov    %edi,-0x4(%rbp)
> >     /home/ravi/test.c:5
> >       400541:       8b 45 fc                mov    -0x4(%rbp),%eax
> >     ...
> >     ...
> >     main():
> >     /home/ravi/test.c:9
> >       400558:       55                      push   %rbp
> >       400559:       48 89 e5                mov    %rsp,%rbp
> >     /home/ravi/test.c:10
> >       40055c:       bf 2a 00 00 00          mov    -bashx2a,%edi
> >       400561:       e8 d0 ff ff ff          callq  400536 <foo>
> >     /home/ravi/test.c:11
> >
> >   $ ./perf probe -x ./test 'foo i'
> >   $ cat /sys/kernel/debug/tracing/uprobe_events
> >      p:probe_test/foo /home/ravi/test:0x0000000000000536 i=-12(%sp):s32
> >
> >   $ ./perf record -e probe_test:foo ./test
> >   $ ./perf script
> >      test  5778 [001]  4918.562027: probe_test:foo: (400536) i=0
> >
> > Here variable 'i' is passed via stack which is pushed on stack at
> > 0x40053e. But we are probing at 0x400536.
> >
> > To resolve this issues, we need to probe on next instruction after
> > prologue. gdb and systemtap also does same thing. I've implemented
> > this patch based on approach systemtap has used.
> >
> > After applying patch:
> >
> >   $ ./perf probe -x ./test 'foo i'
> >   $ cat /sys/kernel/debug/tracing/uprobe_events
> >     p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32
> >
> >   $ ./perf record -e probe_test:foo ./test
> >   $ ./perf script
> >     test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42
> >
> > No need to skip prologue for optimized case since debug info is correct
> > for each instructions for -O2 -g. For more details please visit:
> >         https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6
> >
> > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> > ---
> > Changes in v2:
> >   - Skipping prologue only when any ARG is either C variable, $params
> >     or $vars.
> >   - Probe on line(:1) may not be always possible. Recommend only address
> >     to force probe on function entry.
> >
> >  tools/perf/util/probe-finder.c | 164 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 164 insertions(+)
> >
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index f2d9ff0..b2bc77c 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
> >  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
> >  }
> >
> > +static bool var_has_loclist(Dwarf_Die *die)
> > +{
> > +	Dwarf_Attribute loc;
> > +	int tag = dwarf_tag(die);
> > +
> > +	if (tag != DW_TAG_formal_parameter &&
> > +	    tag != DW_TAG_variable)
> > +		return false;
> > +
> > +	return (dwarf_attr_integrate(die, DW_AT_location, &loc) &&
> > +		dwarf_whatform(&loc) == DW_FORM_sec_offset);
> > +}
> > +
> > +/*
> > + * For any object in given CU whose DW_AT_location is a location list,
> > + * target program is compiled with optimization.
> > + */
> > +static bool optimized_target(Dwarf_Die *die)
> > +{
> > +	Dwarf_Die tmp_die;
> > +
> > +	if (var_has_loclist(die))
> > +		return true;
> > +
> > +	if (!dwarf_child(die, &tmp_die) && optimized_target(&tmp_die))
> > +		return true;
> > +
> > +	if (!dwarf_siblingof(die, &tmp_die) && optimized_target(&tmp_die))
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > +static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
> > +			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
> > +{
> > +	unsigned long i;
> > +	Dwarf_Addr addr;
> > +
> > +	for (i = 0; i < nr_lines; i++) {
> > +		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
> > +			return false;
> > +
> > +		if (addr == pf_addr) {
> > +			*entrypc_idx = i;
> > +			return true;
> > +		}
> > +	}
> > +	return false;
> > +}
> > +
> > +static bool get_postprologue_addr(unsigned long entrypc_idx,
> > +				  Dwarf_Lines *lines,
> > +				  unsigned long nr_lines,
> > +				  Dwarf_Addr highpc,
> > +				  Dwarf_Addr *postprologue_addr)
> > +{
> > +	unsigned long i;
> > +	int entrypc_lno, lno;
> > +	Dwarf_Line *line;
> > +	Dwarf_Addr addr;
> > +	bool p_end;
> > +
> > +	/* entrypc_lno is actual source line number */
> > +	line = dwarf_onesrcline(lines, entrypc_idx);
> > +	if (dwarf_lineno(line, &entrypc_lno))
> > +		return false;
> > +
> > +	for (i = entrypc_idx; i < nr_lines; i++) {
> > +		line = dwarf_onesrcline(lines, i);
> > +
> > +		if (dwarf_lineaddr(line, &addr) ||
> > +		    dwarf_lineno(line, &lno)    ||
> > +		    dwarf_lineprologueend(line, &p_end))
> > +			return false;
> > +
> > +		/* highpc is exclusive. [entrypc,highpc) */
> > +		if (addr >= highpc)
> > +			break;
> > +
> > +		/* clang supports prologue-end marker */
> > +		if (p_end)
> > +			break;
> > +
> > +		/* Actual next line in source */
> > +		if (lno != entrypc_lno)
> > +			break;
> > +
> > +		/*
> > +		 * Single source line can have multiple line records.
> > +		 * For Example,
> > +		 *     void foo() { printf("hello\n"); }
> > +		 * contains two line records. One points to declaration and
> > +		 * other points to printf() line. Variable 'lno' won't get
> > +		 * incremented in this case but 'i' will.
> > +		 */
> > +		if (i != entrypc_idx)
> > +			break;
> > +	}
> > +
> > +	dwarf_lineaddr(line, postprologue_addr);
> > +	if (*postprologue_addr >= highpc)
> > +		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> > +			       postprologue_addr);
> > +
> > +	return true;
> > +}
> > +
> > +static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> > +{
> > +	unsigned long nr_lines = 0, entrypc_idx = 0;
> > +	Dwarf_Lines *lines = NULL;
> > +	Dwarf_Addr postprologue_addr;
> > +	Dwarf_Addr highpc;
> > +
> > +	if (dwarf_highpc(sp_die, &highpc))
> > +		return;
> > +
> > +	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
> > +		return;
> > +
> > +	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
> > +		return;
> > +
> > +	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
> > +				   highpc, &postprologue_addr))
> > +		return;
> > +
> > +	pf->addr = postprologue_addr;
> > +}
> > +
> > +static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> > +{
> > +	struct perf_probe_point *pp = &pf->pev->point;
> > +
> > +	/* Not uprobe? */
> > +	if (!pf->pev->uprobes)
> > +		return;
> > +
> > +	/* Compiled with optimization? */
> > +	if (optimized_target(&pf->cu_die))
> > +		return;
> > +
> > +	/* Don't know entrypc? */
> > +	if (!pf->addr)
> > +		return;
> > +
> > +	/* Only FUNC and FUNC@SRC are eligible. */
> > +	if (!pp->function || pp->line || pp->retprobe || pp->lazy_line ||
> > +	    pp->offset || pp->abs_address)
> > +		return;
> > +
> > +	/* Not interested in func parameter? */
> > +	if (!perf_probe_with_var(pf->pev))
> > +		return;
> > +
> > +	pr_info("Target program is compiled without optimization. Skipping prologue.\n"
> > +		"Probe on address 0x%lx to force probing at the function entry.\n\n",
> > +		pf->addr);
> > +
> > +	__skip_prologue(sp_die, pf);
> > +}
> > +
> >  static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
> >  {
> >  	struct probe_finder *pf = data;
> > @@ -954,6 +1117,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
> >  		if (pp->lazy_line)
> >  			param->retval = find_probe_point_lazy(sp_die, pf);
> >  		else {
> > +			skip_prologue(sp_die, pf);
> >  			pf->addr += pp->offset;
> >  			/* TODO: Check the address in this function */
> >  			param->retval = call_probe_finder(sp_die, pf);
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Naveen N. Rao]

On 2016/08/03 02:28PM, Ravi Bangoria wrote:
> Function prologue prepares stack and registers before executing function
> logic. When target program is compiled without optimization, function
> parameter information is only valid after prologue. When we probe entrypc
> of the function, and try to record function parameter, it contains
> garbage value.
> 
> For example,
>   $ vim test.c
>     #include <stdio.h>
> 
>     void foo(int i)
>     {
>        printf("i: %d\n", i);
>     }
> 
>     int main()
>     {
>       foo(42);
>       return 0;
>     }
> 
>   $ gcc -g test.c -o test
>   $ objdump -dl test | less
>     foo():
>     /home/ravi/test.c:4
>       400536:       55                      push   %rbp
>       400537:       48 89 e5                mov    %rsp,%rbp
>       40053a:       48 83 ec 10             sub    -bashx10,%rsp
>       40053e:       89 7d fc                mov    %edi,-0x4(%rbp)
>     /home/ravi/test.c:5
>       400541:       8b 45 fc                mov    -0x4(%rbp),%eax
>     ...
>     ...
>     main():
>     /home/ravi/test.c:9
>       400558:       55                      push   %rbp
>       400559:       48 89 e5                mov    %rsp,%rbp
>     /home/ravi/test.c:10
>       40055c:       bf 2a 00 00 00          mov    -bashx2a,%edi
>       400561:       e8 d0 ff ff ff          callq  400536 <foo>
>     /home/ravi/test.c:11
> 
>   $ ./perf probe -x ./test 'foo i'
>   $ cat /sys/kernel/debug/tracing/uprobe_events
>      p:probe_test/foo /home/ravi/test:0x0000000000000536 i=-12(%sp):s32
> 
>   $ ./perf record -e probe_test:foo ./test
>   $ ./perf script
>      test  5778 [001]  4918.562027: probe_test:foo: (400536) i=0
> 
> Here variable 'i' is passed via stack which is pushed on stack at
> 0x40053e. But we are probing at 0x400536.
> 
> To resolve this issues, we need to probe on next instruction after
> prologue. gdb and systemtap also does same thing. I've implemented
> this patch based on approach systemtap has used.
> 
> After applying patch:
> 
>   $ ./perf probe -x ./test 'foo i'
>   $ cat /sys/kernel/debug/tracing/uprobe_events
>     p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32
> 
>   $ ./perf record -e probe_test:foo ./test
>   $ ./perf script
>     test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42
> 
> No need to skip prologue for optimized case since debug info is correct
> for each instructions for -O2 -g. For more details please visit:
>         https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> Changes in v2:
>   - Skipping prologue only when any ARG is either C variable, $params
>     or $vars.
>   - Probe on line(:1) may not be always possible. Recommend only address
>     to force probe on function entry.
> 
>  tools/perf/util/probe-finder.c | 164 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 164 insertions(+)

This fixes the uprobe issue reported by Michael Petlan:
https://www.mail-archive.com/linux-perf-users@vger.kernel.org/msg02348.html

Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Masami Hiramatsu]

On Wed,  3 Aug 2016 14:28:45 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> Function prologue prepares stack and registers before executing function
> logic. When target program is compiled without optimization, function
> parameter information is only valid after prologue. When we probe entrypc
> of the function, and try to record function parameter, it contains
> garbage value.
> 
> For example,
>   $ vim test.c
>     #include <stdio.h>
> 
>     void foo(int i)
>     {
>        printf("i: %d\n", i);
>     }
> 
>     int main()
>     {
>       foo(42);
>       return 0;
>     }
> 
>   $ gcc -g test.c -o test
>   $ objdump -dl test | less
>     foo():
>     /home/ravi/test.c:4
>       400536:       55                      push   %rbp
>       400537:       48 89 e5                mov    %rsp,%rbp
>       40053a:       48 83 ec 10             sub    -bashx10,%rsp
>       40053e:       89 7d fc                mov    %edi,-0x4(%rbp)
>     /home/ravi/test.c:5
>       400541:       8b 45 fc                mov    -0x4(%rbp),%eax
>     ...
>     ...
>     main():
>     /home/ravi/test.c:9
>       400558:       55                      push   %rbp
>       400559:       48 89 e5                mov    %rsp,%rbp
>     /home/ravi/test.c:10
>       40055c:       bf 2a 00 00 00          mov    -bashx2a,%edi
>       400561:       e8 d0 ff ff ff          callq  400536 <foo>
>     /home/ravi/test.c:11
> 
>   $ ./perf probe -x ./test 'foo i'
>   $ cat /sys/kernel/debug/tracing/uprobe_events
>      p:probe_test/foo /home/ravi/test:0x0000000000000536 i=-12(%sp):s32
> 
>   $ ./perf record -e probe_test:foo ./test
>   $ ./perf script
>      test  5778 [001]  4918.562027: probe_test:foo: (400536) i=0
> 
> Here variable 'i' is passed via stack which is pushed on stack at
> 0x40053e. But we are probing at 0x400536.
> 
> To resolve this issues, we need to probe on next instruction after
> prologue. gdb and systemtap also does same thing. I've implemented
> this patch based on approach systemtap has used.
> 
> After applying patch:
> 
>   $ ./perf probe -x ./test 'foo i'
>   $ cat /sys/kernel/debug/tracing/uprobe_events
>     p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32
> 
>   $ ./perf record -e probe_test:foo ./test
>   $ ./perf script
>     test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42
> 
> No need to skip prologue for optimized case since debug info is correct
> for each instructions for -O2 -g. For more details please visit:
>         https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Looks good for me:)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Jiri, Michael, please try this. Ravi's patch can fix your problem.

Thank you!


> ---
> Changes in v2:
>   - Skipping prologue only when any ARG is either C variable, $params
>     or $vars.
>   - Probe on line(:1) may not be always possible. Recommend only address
>     to force probe on function entry.
> 
>  tools/perf/util/probe-finder.c | 164 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 164 insertions(+)
> 
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index f2d9ff0..b2bc77c 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
>  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
>  }
>  
> +static bool var_has_loclist(Dwarf_Die *die)
> +{
> +	Dwarf_Attribute loc;
> +	int tag = dwarf_tag(die);
> +
> +	if (tag != DW_TAG_formal_parameter &&
> +	    tag != DW_TAG_variable)
> +		return false;
> +
> +	return (dwarf_attr_integrate(die, DW_AT_location, &loc) &&
> +		dwarf_whatform(&loc) == DW_FORM_sec_offset);
> +}
> +
> +/*
> + * For any object in given CU whose DW_AT_location is a location list,
> + * target program is compiled with optimization.
> + */
> +static bool optimized_target(Dwarf_Die *die)
> +{
> +	Dwarf_Die tmp_die;
> +
> +	if (var_has_loclist(die))
> +		return true;
> +
> +	if (!dwarf_child(die, &tmp_die) && optimized_target(&tmp_die))
> +		return true;
> +
> +	if (!dwarf_siblingof(die, &tmp_die) && optimized_target(&tmp_die))
> +		return true;
> +
> +	return false;
> +}
> +
> +static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
> +			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
> +{
> +	unsigned long i;
> +	Dwarf_Addr addr;
> +
> +	for (i = 0; i < nr_lines; i++) {
> +		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
> +			return false;
> +
> +		if (addr == pf_addr) {
> +			*entrypc_idx = i;
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +static bool get_postprologue_addr(unsigned long entrypc_idx,
> +				  Dwarf_Lines *lines,
> +				  unsigned long nr_lines,
> +				  Dwarf_Addr highpc,
> +				  Dwarf_Addr *postprologue_addr)
> +{
> +	unsigned long i;
> +	int entrypc_lno, lno;
> +	Dwarf_Line *line;
> +	Dwarf_Addr addr;
> +	bool p_end;
> +
> +	/* entrypc_lno is actual source line number */
> +	line = dwarf_onesrcline(lines, entrypc_idx);
> +	if (dwarf_lineno(line, &entrypc_lno))
> +		return false;
> +
> +	for (i = entrypc_idx; i < nr_lines; i++) {
> +		line = dwarf_onesrcline(lines, i);
> +
> +		if (dwarf_lineaddr(line, &addr) ||
> +		    dwarf_lineno(line, &lno)    ||
> +		    dwarf_lineprologueend(line, &p_end))
> +			return false;
> +
> +		/* highpc is exclusive. [entrypc,highpc) */
> +		if (addr >= highpc)
> +			break;
> +
> +		/* clang supports prologue-end marker */
> +		if (p_end)
> +			break;
> +
> +		/* Actual next line in source */
> +		if (lno != entrypc_lno)
> +			break;
> +
> +		/*
> +		 * Single source line can have multiple line records.
> +		 * For Example,
> +		 *     void foo() { printf("hello\n"); }
> +		 * contains two line records. One points to declaration and
> +		 * other points to printf() line. Variable 'lno' won't get
> +		 * incremented in this case but 'i' will.
> +		 */
> +		if (i != entrypc_idx)
> +			break;
> +	}
> +
> +	dwarf_lineaddr(line, postprologue_addr);
> +	if (*postprologue_addr >= highpc)
> +		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> +			       postprologue_addr);
> +
> +	return true;
> +}
> +
> +static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> +{
> +	unsigned long nr_lines = 0, entrypc_idx = 0;
> +	Dwarf_Lines *lines = NULL;
> +	Dwarf_Addr postprologue_addr;
> +	Dwarf_Addr highpc;
> +
> +	if (dwarf_highpc(sp_die, &highpc))
> +		return;
> +
> +	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
> +		return;
> +
> +	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
> +		return;
> +
> +	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
> +				   highpc, &postprologue_addr))
> +		return;
> +
> +	pf->addr = postprologue_addr;
> +}
> +
> +static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> +{
> +	struct perf_probe_point *pp = &pf->pev->point;
> +
> +	/* Not uprobe? */
> +	if (!pf->pev->uprobes)
> +		return;
> +
> +	/* Compiled with optimization? */
> +	if (optimized_target(&pf->cu_die))
> +		return;
> +
> +	/* Don't know entrypc? */
> +	if (!pf->addr)
> +		return;
> +
> +	/* Only FUNC and FUNC@SRC are eligible. */
> +	if (!pp->function || pp->line || pp->retprobe || pp->lazy_line ||
> +	    pp->offset || pp->abs_address)
> +		return;
> +
> +	/* Not interested in func parameter? */
> +	if (!perf_probe_with_var(pf->pev))
> +		return;
> +
> +	pr_info("Target program is compiled without optimization. Skipping prologue.\n"
> +		"Probe on address 0x%lx to force probing at the function entry.\n\n",
> +		pf->addr);
> +
> +	__skip_prologue(sp_die, pf);
> +}
> +
>  static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
>  {
>  	struct probe_finder *pf = data;
> @@ -954,6 +1117,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
>  		if (pp->lazy_line)
>  			param->retval = find_probe_point_lazy(sp_die, pf);
>  		else {
> +			skip_prologue(sp_die, pf);
>  			pf->addr += pp->offset;
>  			/* TODO: Check the address in this function */
>  			param->retval = call_probe_finder(sp_die, pf);
> -- 
> 2.5.5
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Jiri Olsa]

On Thu, Aug 25, 2016 at 09:50:04PM +0900, Masami Hiramatsu wrote:

SNIP

> > After applying patch:
> > 
> >   $ ./perf probe -x ./test 'foo i'
> >   $ cat /sys/kernel/debug/tracing/uprobe_events
> >     p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32
> > 
> >   $ ./perf record -e probe_test:foo ./test
> >   $ ./perf script
> >     test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42
> > 
> > No need to skip prologue for optimized case since debug info is correct
> > for each instructions for -O2 -g. For more details please visit:
> >         https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6
> > 
> > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> 
> Looks good for me:)
> 
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Jiri, Michael, please try this. Ravi's patch can fix your problem.

yes, that fixes the problem for me

thanks a lot,
jirka

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Arnaldo Carvalho de Melo]

Em Thu, Aug 25, 2016 at 03:59:43PM +0200, Jiri Olsa escreveu:
> On Thu, Aug 25, 2016 at 09:50:04PM +0900, Masami Hiramatsu wrote:
> 
> SNIP
> 
> > > After applying patch:
> > > 
> > >   $ ./perf probe -x ./test 'foo i'
> > >   $ cat /sys/kernel/debug/tracing/uprobe_events
> > >     p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32
> > > 
> > >   $ ./perf record -e probe_test:foo ./test
> > >   $ ./perf script
> > >     test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42
> > > 
> > > No need to skip prologue for optimized case since debug info is correct
> > > for each instructions for -O2 -g. For more details please visit:
> > >         https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6
> > > 
> > > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> > 
> > Looks good for me:)
> > 
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > 
> > Jiri, Michael, please try this. Ravi's patch can fix your problem.
> 
> yes, that fixes the problem for me

Ok, so I'll add a Tested-by: Jiri, ok? And push it via perf/urgent.

- Arnaldo

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Jiri Olsa]

On Thu, Aug 25, 2016 at 12:17:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 25, 2016 at 03:59:43PM +0200, Jiri Olsa escreveu:
> > On Thu, Aug 25, 2016 at 09:50:04PM +0900, Masami Hiramatsu wrote:
> > 
> > SNIP
> > 
> > > > After applying patch:
> > > > 
> > > >   $ ./perf probe -x ./test 'foo i'
> > > >   $ cat /sys/kernel/debug/tracing/uprobe_events
> > > >     p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32
> > > > 
> > > >   $ ./perf record -e probe_test:foo ./test
> > > >   $ ./perf script
> > > >     test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42
> > > > 
> > > > No need to skip prologue for optimized case since debug info is correct
> > > > for each instructions for -O2 -g. For more details please visit:
> > > >         https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6
> > > > 
> > > > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> > > 
> > > Looks good for me:)
> > > 
> > > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > 
> > > Jiri, Michael, please try this. Ravi's patch can fix your problem.
> > 
> > yes, that fixes the problem for me
> 
> Ok, so I'll add a Tested-by: Jiri, ok? And push it via perf/urgent.

yep, thanks

jirka

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Arnaldo Carvalho de Melo]

Em Wed, Aug 03, 2016 at 02:28:45PM +0530, Ravi Bangoria escreveu:
> +++ b/tools/perf/util/probe-finder.c
> @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
>  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
>  }
  
> +static bool var_has_loclist(Dwarf_Die *die)

So, the variable 'die' cause the build to fail in multiple systems, I'm
renaming it to 'cu_die', there are some more problems when cross compiling it
to some arches, I'm trying to fix them all now:

[root@jouet ~]# dm
 1 65.668882867 alpine:3.4: Ok
 2 24.327143744 android-ndk:r12b-arm: Ok
 3 76.000455817 archlinux:latest: Ok
 4 40.905282317 centos:5: Ok
 5 28.799284950 centos:6: FAIL
cc1: warnings being treated as errors
util/probe-finder.c: In function 'var_has_loclist':
util/probe-finder.c:904: error: declaration of 'die' shadows a global declaration
util/util.h:137: error: shadowed declaration is here
util/probe-finder.c: In function 'optimized_target':
util/probe-finder.c:921: error: declaration of 'die' shadows a global declaration
util/util.h:137: error: shadowed declaration is here
-----------------------------------------------------------------------------
 6 68.157498673 centos:7: Ok
 7 32.814597820 debian:7: FAIL
util/probe-finder.c: In function 'var_has_loclist':
util/probe-finder.c:904:40: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
In file included from util/probe-finder.c:39:0:
util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
util/probe-finder.c: In function 'optimized_target':
util/probe-finder.c:921:41: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
In file included from util/probe-finder.c:39:0:
util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
-----------------------------------------------------------------------------
 8 69.535114125 debian:8: Ok
 9 71.524733936 debian:experimental: Ok
10 66.956671932 fedora:20: Ok
11 72.127804972 fedora:21: Ok
12 75.767022465 fedora:22: Ok
13 72.244964710 fedora:23: Ok
14 75.556712624 fedora:24: Ok
15 30.825802615 fedora:24-x-ARC-uClibc: Ok
16 76.874903316 fedora:rawhide: Ok
17 76.756388879 mageia:5: Ok
18 71.700493646 opensuse:13.2: Ok
19 69.974649379 opensuse:42.1: Ok
20 75.265305896 opensuse:tumbleweed: Ok
21 28.800283739 ubuntu:12.04.5: FAIL
util/probe-finder.c: In function 'var_has_loclist':
util/probe-finder.c:904:40: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
util/probe-finder.c: In function 'optimized_target':
util/probe-finder.c:921:41: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
-----------------------------------------------------------------------------
22 66.514504455 ubuntu:14.04.4: Ok
23 69.100413367 ubuntu:15.10: Ok
24 64.414190947 ubuntu:16.04: Ok
25 29.481652689 ubuntu:16.04-x-arm: FAIL
util/probe-finder.c: In function '__skip_prologue':
util/probe-finder.c:1022:45: error: passing argument 3 of 'dwarf_getsrclines' from incompatible pointer type [-Werror=incompatible-pointer-types]
  if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
                                             ^
In file included from util/dwarf-aux.h:23:0,
                 from util/probe-finder.h:24,
                 from util/probe-finder.c:41:
/usr/arm-linux-gnueabihf/include/elfutils/libdw.h:592:12: note: expected 'size_t * {aka unsigned int *}' but argument is of type 'long unsigned int *'
 extern int dwarf_getsrclines (Dwarf_Die *cudie, Dwarf_Lines **lines,
            ^
In file included from util/probe-finder.c:37:0:
util/probe-finder.c: In function 'skip_prologue':
util/probe-finder.c:1060:10: error: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'Dwarf_Addr {aka long long unsigned int}' [-Werror=format=]
  pr_info("Target program is compiled without optimization. Skipping prologue.\n"
          ^
util/debug.h:18:21: note: in definition of macro 'pr_fmt'
 #define pr_fmt(fmt) fmt
                     ^
util/probe-finder.c:1060:2: note: in expansion of macro 'pr_info'
  pr_info("Target program is compiled without optimization. Skipping prologue.\n"
  ^
-----------------------------------------------------------------------------
26 51.979875334 ubuntu:16.04-x-arm64: Ok
27 28.808591224 ubuntu:16.04-x-powerpc64: FAIL
  CC       /tmp/build/perf/tests/bp_signal.o
util/probe-finder.c: In function '__skip_prologue':
util/probe-finder.c:1022:45: error: passing argument 3 of 'dwarf_getsrclines' from incompatible pointer type [-Werror=incompatible-pointer-types]
  if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
                                             ^
In file included from util/dwarf-aux.h:23:0,
                 from util/probe-finder.h:24,
                 from util/probe-finder.c:41:
/usr/powerpc-linux-gnu/include/elfutils/libdw.h:592:12: note: expected 'size_t * {aka unsigned int *}' but argument is of type 'long unsigned int *'
 extern int dwarf_getsrclines (Dwarf_Die *cudie, Dwarf_Lines **lines,
            ^
In file included from util/probe-finder.c:37:0:
util/probe-finder.c: In function 'skip_prologue':
util/probe-finder.c:1060:10: error: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'Dwarf_Addr {aka long long unsigned int}' [-Werror=format=]
  pr_info("Target program is compiled without optimization. Skipping prologue.\n"
          ^
util/debug.h:18:21: note: in definition of macro 'pr_fmt'
 #define pr_fmt(fmt) fmt
                     ^
util/probe-finder.c:1060:2: note: in expansion of macro 'pr_info'
-----------------------------------------------------------------------------
28 53.372615706 ubuntu:16.04-x-powerpc64el: Ok
29 71.550573752 ubuntu:16.10: Ok
30 52.727886240 ubuntu:16.10-x-s390: Ok

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Arnaldo Carvalho de Melo]

Em Fri, Aug 26, 2016 at 04:30:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 03, 2016 at 02:28:45PM +0530, Ravi Bangoria escreveu:
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
> >  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
> >  }
>   
> > +static bool var_has_loclist(Dwarf_Die *die)
> 
> So, the variable 'die' cause the build to fail in multiple systems, I'm
> renaming it to 'cu_die', there are some more problems when cross compiling it
> to some arches, I'm trying to fix them all now:

Fixed, please check:

http://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/urgent&id=3866e3762da3291613dfb8b193885a8ed3836669

- Arnaldo

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Masami Hiramatsu]

On Fri, 26 Aug 2016 16:54:52 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Fri, Aug 26, 2016 at 04:30:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Aug 03, 2016 at 02:28:45PM +0530, Ravi Bangoria escreveu:
> > > +++ b/tools/perf/util/probe-finder.c
> > > @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
> > >  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
> > >  }
> >   
> > > +static bool var_has_loclist(Dwarf_Die *die)
> > 
> > So, the variable 'die' cause the build to fail in multiple systems, I'm
> > renaming it to 'cu_die', there are some more problems when cross compiling it
> > to some arches, I'm trying to fix them all now:
> 
> Fixed, please check:
> 
> http://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/urgent&id=3866e3762da3291613dfb8b193885a8ed3836669

Hmm I think var_has_loclist(cu_die) should be vr_die(means Variable DIE) not
cu_die (Compile Unit DIE == object file itself) since user must pass the
DIE for local variable. And revisiting on the code, it seems this has several
generic dwarf related functions, which can be put into dwarf-aux.c/.h as
other APIs. 
But anyway it is a trivial & cleanup thing.

At first we should fix this bug for helping users ASAP. So I'm OK.

Thank you!

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH] perf probe: Move dwarf specific functions to dwarf-aux.c

[Ravi Bangoria]

Move generic dwarf related functions from util/probe-finder.c to
util/dwarf-aux.c. Function names and their prototype are also
changed accordingly. No functionality changes.

Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 tools/perf/util/dwarf-aux.c    | 135 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h    |   5 ++
 tools/perf/util/probe-finder.c | 136 +----------------------------------------
 3 files changed, 142 insertions(+), 134 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index a347b19..8d595b9 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1085,3 +1085,138 @@ int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
 	return -ENOTSUP;
 }
 #endif
+
+static bool die_has_loclist(Dwarf_Die *var_die)
+{
+	Dwarf_Attribute loc;
+	int tag = dwarf_tag(var_die);
+
+	if (tag != DW_TAG_formal_parameter &&
+	    tag != DW_TAG_variable)
+		return false;
+
+	return (dwarf_attr_integrate(var_die, DW_AT_location, &loc) &&
+		dwarf_whatform(&loc) == DW_FORM_sec_offset);
+}
+
+/*
+ * For any object in given CU whose DW_AT_location is a location list,
+ * target program is compiled with optimization.
+ */
+bool die_is_optimized_target(Dwarf_Die *cu_die)
+{
+	Dwarf_Die tmp_die;
+
+	if (die_has_loclist(cu_die))
+		return true;
+
+	if (!dwarf_child(cu_die, &tmp_die) &&
+	    die_is_optimized_target(&tmp_die))
+		return true;
+
+	if (!dwarf_siblingof(cu_die, &tmp_die) &&
+	    die_is_optimized_target(&tmp_die))
+		return true;
+
+	return false;
+}
+
+static bool die_search_idx(Dwarf_Lines *lines, unsigned long nr_lines,
+			   Dwarf_Addr pf_addr, unsigned long *idx)
+{
+	unsigned long i;
+	Dwarf_Addr addr;
+
+	for (i = 0; i < nr_lines; i++) {
+		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
+			return false;
+
+		if (addr == pf_addr) {
+			*idx = i;
+			return true;
+		}
+	}
+	return false;
+}
+
+static bool die_get_postprologue_addr(unsigned long entrypc_idx,
+				      Dwarf_Lines *lines,
+				      unsigned long nr_lines,
+				      Dwarf_Addr highpc,
+				      Dwarf_Addr *postprologue_addr)
+{
+	unsigned long i;
+	int entrypc_lno, lno;
+	Dwarf_Line *line;
+	Dwarf_Addr addr;
+	bool p_end;
+
+	/* entrypc_lno is actual source line number */
+	line = dwarf_onesrcline(lines, entrypc_idx);
+	if (dwarf_lineno(line, &entrypc_lno))
+		return false;
+
+	for (i = entrypc_idx; i < nr_lines; i++) {
+		line = dwarf_onesrcline(lines, i);
+
+		if (dwarf_lineaddr(line, &addr) ||
+		    dwarf_lineno(line, &lno)    ||
+		    dwarf_lineprologueend(line, &p_end))
+			return false;
+
+		/* highpc is exclusive. [entrypc,highpc) */
+		if (addr >= highpc)
+			break;
+
+		/* clang supports prologue-end marker */
+		if (p_end)
+			break;
+
+		/* Actual next line in source */
+		if (lno != entrypc_lno)
+			break;
+
+		/*
+		 * Single source line can have multiple line records.
+		 * For Example,
+		 *     void foo() { printf("hello\n"); }
+		 * contains two line records. One points to declaration and
+		 * other points to printf() line. Variable 'lno' won't get
+		 * incremented in this case but 'i' will.
+		 */
+		if (i != entrypc_idx)
+			break;
+	}
+
+	dwarf_lineaddr(line, postprologue_addr);
+	if (*postprologue_addr >= highpc)
+		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
+			       postprologue_addr);
+
+	return true;
+}
+
+void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
+		       Dwarf_Addr *entrypc)
+{
+	size_t nr_lines = 0;
+	unsigned long entrypc_idx = 0;
+	Dwarf_Lines *lines = NULL;
+	Dwarf_Addr postprologue_addr;
+	Dwarf_Addr highpc;
+
+	if (dwarf_highpc(sp_die, &highpc))
+		return;
+
+	if (dwarf_getsrclines(cu_die, &lines, &nr_lines))
+		return;
+
+	if (!die_search_idx(lines, nr_lines, *entrypc, &entrypc_idx))
+		return;
+
+	if (!die_get_postprologue_addr(entrypc_idx, lines, nr_lines,
+				       highpc, &postprologue_addr))
+		return;
+
+	*entrypc = postprologue_addr;
+}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index dc0ce1a..791884a 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -125,4 +125,9 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
 /* Get the name and type of given variable DIE, stored as "type\tname" */
 int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
 int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
+
+bool die_is_optimized_target(Dwarf_Die *cu_die);
+void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
+		       Dwarf_Addr *entrypc);
+
 #endif
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 945cf7a..72f1152 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -901,138 +901,6 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
 	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
 }
 
-static bool var_has_loclist(Dwarf_Die *cu_die)
-{
-	Dwarf_Attribute loc;
-	int tag = dwarf_tag(cu_die);
-
-	if (tag != DW_TAG_formal_parameter &&
-	    tag != DW_TAG_variable)
-		return false;
-
-	return (dwarf_attr_integrate(cu_die, DW_AT_location, &loc) &&
-		dwarf_whatform(&loc) == DW_FORM_sec_offset);
-}
-
-/*
- * For any object in given CU whose DW_AT_location is a location list,
- * target program is compiled with optimization.
- */
-static bool optimized_target(Dwarf_Die *cu_die)
-{
-	Dwarf_Die tmp_die;
-
-	if (var_has_loclist(cu_die))
-		return true;
-
-	if (!dwarf_child(cu_die, &tmp_die) && optimized_target(&tmp_die))
-		return true;
-
-	if (!dwarf_siblingof(cu_die, &tmp_die) && optimized_target(&tmp_die))
-		return true;
-
-	return false;
-}
-
-static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
-			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
-{
-	unsigned long i;
-	Dwarf_Addr addr;
-
-	for (i = 0; i < nr_lines; i++) {
-		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
-			return false;
-
-		if (addr == pf_addr) {
-			*entrypc_idx = i;
-			return true;
-		}
-	}
-	return false;
-}
-
-static bool get_postprologue_addr(unsigned long entrypc_idx,
-				  Dwarf_Lines *lines,
-				  unsigned long nr_lines,
-				  Dwarf_Addr highpc,
-				  Dwarf_Addr *postprologue_addr)
-{
-	unsigned long i;
-	int entrypc_lno, lno;
-	Dwarf_Line *line;
-	Dwarf_Addr addr;
-	bool p_end;
-
-	/* entrypc_lno is actual source line number */
-	line = dwarf_onesrcline(lines, entrypc_idx);
-	if (dwarf_lineno(line, &entrypc_lno))
-		return false;
-
-	for (i = entrypc_idx; i < nr_lines; i++) {
-		line = dwarf_onesrcline(lines, i);
-
-		if (dwarf_lineaddr(line, &addr) ||
-		    dwarf_lineno(line, &lno)    ||
-		    dwarf_lineprologueend(line, &p_end))
-			return false;
-
-		/* highpc is exclusive. [entrypc,highpc) */
-		if (addr >= highpc)
-			break;
-
-		/* clang supports prologue-end marker */
-		if (p_end)
-			break;
-
-		/* Actual next line in source */
-		if (lno != entrypc_lno)
-			break;
-
-		/*
-		 * Single source line can have multiple line records.
-		 * For Example,
-		 *     void foo() { printf("hello\n"); }
-		 * contains two line records. One points to declaration and
-		 * other points to printf() line. Variable 'lno' won't get
-		 * incremented in this case but 'i' will.
-		 */
-		if (i != entrypc_idx)
-			break;
-	}
-
-	dwarf_lineaddr(line, postprologue_addr);
-	if (*postprologue_addr >= highpc)
-		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
-			       postprologue_addr);
-
-	return true;
-}
-
-static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
-{
-	size_t nr_lines = 0;
-	unsigned long entrypc_idx = 0;
-	Dwarf_Lines *lines = NULL;
-	Dwarf_Addr postprologue_addr;
-	Dwarf_Addr highpc;
-
-	if (dwarf_highpc(sp_die, &highpc))
-		return;
-
-	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
-		return;
-
-	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
-		return;
-
-	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
-				   highpc, &postprologue_addr))
-		return;
-
-	pf->addr = postprologue_addr;
-}
-
 static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 {
 	struct perf_probe_point *pp = &pf->pev->point;
@@ -1042,7 +910,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 		return;
 
 	/* Compiled with optimization? */
-	if (optimized_target(&pf->cu_die))
+	if (die_is_optimized_target(&pf->cu_die))
 		return;
 
 	/* Don't know entrypc? */
@@ -1062,7 +930,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 		"Probe on address 0x%" PRIx64 " to force probing at the function entry.\n\n",
 		pf->addr);
 
-	__skip_prologue(sp_die, pf);
+	die_skip_prologue(sp_die, &pf->cu_die, &pf->addr);
 }
 
 static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
-- 
2.5.5

Re: [PATCH] perf probe: Move dwarf specific functions to dwarf-aux.c

[Masami Hiramatsu]

On Mon, 29 Aug 2016 20:40:01 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> Move generic dwarf related functions from util/probe-finder.c to
> util/dwarf-aux.c. Function names and their prototype are also
> changed accordingly. No functionality changes.

Code looks OK, could you please add usage comments as same as
other functions, and one-line comments for prototypes?

Thanks,

> 
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  tools/perf/util/dwarf-aux.c    | 135 ++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/dwarf-aux.h    |   5 ++
>  tools/perf/util/probe-finder.c | 136 +----------------------------------------
>  3 files changed, 142 insertions(+), 134 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index a347b19..8d595b9 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1085,3 +1085,138 @@ int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
>  	return -ENOTSUP;
>  }
>  #endif
> +
> +static bool die_has_loclist(Dwarf_Die *var_die)
> +{
> +	Dwarf_Attribute loc;
> +	int tag = dwarf_tag(var_die);
> +
> +	if (tag != DW_TAG_formal_parameter &&
> +	    tag != DW_TAG_variable)
> +		return false;
> +
> +	return (dwarf_attr_integrate(var_die, DW_AT_location, &loc) &&
> +		dwarf_whatform(&loc) == DW_FORM_sec_offset);
> +}
> +
> +/*
> + * For any object in given CU whose DW_AT_location is a location list,
> + * target program is compiled with optimization.
> + */
> +bool die_is_optimized_target(Dwarf_Die *cu_die)
> +{
> +	Dwarf_Die tmp_die;
> +
> +	if (die_has_loclist(cu_die))
> +		return true;
> +
> +	if (!dwarf_child(cu_die, &tmp_die) &&
> +	    die_is_optimized_target(&tmp_die))
> +		return true;
> +
> +	if (!dwarf_siblingof(cu_die, &tmp_die) &&
> +	    die_is_optimized_target(&tmp_die))
> +		return true;
> +
> +	return false;
> +}
> +
> +static bool die_search_idx(Dwarf_Lines *lines, unsigned long nr_lines,
> +			   Dwarf_Addr pf_addr, unsigned long *idx)
> +{
> +	unsigned long i;
> +	Dwarf_Addr addr;
> +
> +	for (i = 0; i < nr_lines; i++) {
> +		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
> +			return false;
> +
> +		if (addr == pf_addr) {
> +			*idx = i;
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +static bool die_get_postprologue_addr(unsigned long entrypc_idx,
> +				      Dwarf_Lines *lines,
> +				      unsigned long nr_lines,
> +				      Dwarf_Addr highpc,
> +				      Dwarf_Addr *postprologue_addr)
> +{
> +	unsigned long i;
> +	int entrypc_lno, lno;
> +	Dwarf_Line *line;
> +	Dwarf_Addr addr;
> +	bool p_end;
> +
> +	/* entrypc_lno is actual source line number */
> +	line = dwarf_onesrcline(lines, entrypc_idx);
> +	if (dwarf_lineno(line, &entrypc_lno))
> +		return false;
> +
> +	for (i = entrypc_idx; i < nr_lines; i++) {
> +		line = dwarf_onesrcline(lines, i);
> +
> +		if (dwarf_lineaddr(line, &addr) ||
> +		    dwarf_lineno(line, &lno)    ||
> +		    dwarf_lineprologueend(line, &p_end))
> +			return false;
> +
> +		/* highpc is exclusive. [entrypc,highpc) */
> +		if (addr >= highpc)
> +			break;
> +
> +		/* clang supports prologue-end marker */
> +		if (p_end)
> +			break;
> +
> +		/* Actual next line in source */
> +		if (lno != entrypc_lno)
> +			break;
> +
> +		/*
> +		 * Single source line can have multiple line records.
> +		 * For Example,
> +		 *     void foo() { printf("hello\n"); }
> +		 * contains two line records. One points to declaration and
> +		 * other points to printf() line. Variable 'lno' won't get
> +		 * incremented in this case but 'i' will.
> +		 */
> +		if (i != entrypc_idx)
> +			break;
> +	}
> +
> +	dwarf_lineaddr(line, postprologue_addr);
> +	if (*postprologue_addr >= highpc)
> +		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> +			       postprologue_addr);
> +
> +	return true;
> +}
> +
> +void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
> +		       Dwarf_Addr *entrypc)
> +{
> +	size_t nr_lines = 0;
> +	unsigned long entrypc_idx = 0;
> +	Dwarf_Lines *lines = NULL;
> +	Dwarf_Addr postprologue_addr;
> +	Dwarf_Addr highpc;
> +
> +	if (dwarf_highpc(sp_die, &highpc))
> +		return;
> +
> +	if (dwarf_getsrclines(cu_die, &lines, &nr_lines))
> +		return;
> +
> +	if (!die_search_idx(lines, nr_lines, *entrypc, &entrypc_idx))
> +		return;
> +
> +	if (!die_get_postprologue_addr(entrypc_idx, lines, nr_lines,
> +				       highpc, &postprologue_addr))
> +		return;
> +
> +	*entrypc = postprologue_addr;
> +}
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index dc0ce1a..791884a 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -125,4 +125,9 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
>  /* Get the name and type of given variable DIE, stored as "type\tname" */
>  int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
>  int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
> +
> +bool die_is_optimized_target(Dwarf_Die *cu_die);
> +void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
> +		       Dwarf_Addr *entrypc);
> +
>  #endif
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 945cf7a..72f1152 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -901,138 +901,6 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
>  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
>  }
>  
> -static bool var_has_loclist(Dwarf_Die *cu_die)
> -{
> -	Dwarf_Attribute loc;
> -	int tag = dwarf_tag(cu_die);
> -
> -	if (tag != DW_TAG_formal_parameter &&
> -	    tag != DW_TAG_variable)
> -		return false;
> -
> -	return (dwarf_attr_integrate(cu_die, DW_AT_location, &loc) &&
> -		dwarf_whatform(&loc) == DW_FORM_sec_offset);
> -}
> -
> -/*
> - * For any object in given CU whose DW_AT_location is a location list,
> - * target program is compiled with optimization.
> - */
> -static bool optimized_target(Dwarf_Die *cu_die)
> -{
> -	Dwarf_Die tmp_die;
> -
> -	if (var_has_loclist(cu_die))
> -		return true;
> -
> -	if (!dwarf_child(cu_die, &tmp_die) && optimized_target(&tmp_die))
> -		return true;
> -
> -	if (!dwarf_siblingof(cu_die, &tmp_die) && optimized_target(&tmp_die))
> -		return true;
> -
> -	return false;
> -}
> -
> -static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
> -			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
> -{
> -	unsigned long i;
> -	Dwarf_Addr addr;
> -
> -	for (i = 0; i < nr_lines; i++) {
> -		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
> -			return false;
> -
> -		if (addr == pf_addr) {
> -			*entrypc_idx = i;
> -			return true;
> -		}
> -	}
> -	return false;
> -}
> -
> -static bool get_postprologue_addr(unsigned long entrypc_idx,
> -				  Dwarf_Lines *lines,
> -				  unsigned long nr_lines,
> -				  Dwarf_Addr highpc,
> -				  Dwarf_Addr *postprologue_addr)
> -{
> -	unsigned long i;
> -	int entrypc_lno, lno;
> -	Dwarf_Line *line;
> -	Dwarf_Addr addr;
> -	bool p_end;
> -
> -	/* entrypc_lno is actual source line number */
> -	line = dwarf_onesrcline(lines, entrypc_idx);
> -	if (dwarf_lineno(line, &entrypc_lno))
> -		return false;
> -
> -	for (i = entrypc_idx; i < nr_lines; i++) {
> -		line = dwarf_onesrcline(lines, i);
> -
> -		if (dwarf_lineaddr(line, &addr) ||
> -		    dwarf_lineno(line, &lno)    ||
> -		    dwarf_lineprologueend(line, &p_end))
> -			return false;
> -
> -		/* highpc is exclusive. [entrypc,highpc) */
> -		if (addr >= highpc)
> -			break;
> -
> -		/* clang supports prologue-end marker */
> -		if (p_end)
> -			break;
> -
> -		/* Actual next line in source */
> -		if (lno != entrypc_lno)
> -			break;
> -
> -		/*
> -		 * Single source line can have multiple line records.
> -		 * For Example,
> -		 *     void foo() { printf("hello\n"); }
> -		 * contains two line records. One points to declaration and
> -		 * other points to printf() line. Variable 'lno' won't get
> -		 * incremented in this case but 'i' will.
> -		 */
> -		if (i != entrypc_idx)
> -			break;
> -	}
> -
> -	dwarf_lineaddr(line, postprologue_addr);
> -	if (*postprologue_addr >= highpc)
> -		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> -			       postprologue_addr);
> -
> -	return true;
> -}
> -
> -static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> -{
> -	size_t nr_lines = 0;
> -	unsigned long entrypc_idx = 0;
> -	Dwarf_Lines *lines = NULL;
> -	Dwarf_Addr postprologue_addr;
> -	Dwarf_Addr highpc;
> -
> -	if (dwarf_highpc(sp_die, &highpc))
> -		return;
> -
> -	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
> -		return;
> -
> -	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
> -		return;
> -
> -	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
> -				   highpc, &postprologue_addr))
> -		return;
> -
> -	pf->addr = postprologue_addr;
> -}
> -
>  static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
>  {
>  	struct perf_probe_point *pp = &pf->pev->point;
> @@ -1042,7 +910,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
>  		return;
>  
>  	/* Compiled with optimization? */
> -	if (optimized_target(&pf->cu_die))
> +	if (die_is_optimized_target(&pf->cu_die))
>  		return;
>  
>  	/* Don't know entrypc? */
> @@ -1062,7 +930,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
>  		"Probe on address 0x%" PRIx64 " to force probing at the function entry.\n\n",
>  		pf->addr);
>  
> -	__skip_prologue(sp_die, pf);
> +	die_skip_prologue(sp_die, &pf->cu_die, &pf->addr);
>  }
>  
>  static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
> -- 
> 2.5.5
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH v2] perf probe: Move dwarf specific functions to dwarf-aux.c

[Ravi Bangoria]

Move generic dwarf related functions from util/probe-finder.c to
util/dwarf-aux.c. Functions name and their prototype are also
changed accordingly. No functionality changes.

Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
Changes in v2:
  - Add comments about functions prototype and their usage
  - Replace function parameter names with more generic names

 tools/perf/util/dwarf-aux.c    | 179 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h    |   8 ++
 tools/perf/util/probe-finder.c | 136 +------------------------------
 3 files changed, 189 insertions(+), 134 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index a347b19..faec899 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1085,3 +1085,182 @@ int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
 	return -ENOTSUP;
 }
 #endif
+
+/*
+ * die_has_loclist - Check if DW_AT_location of @vr_die is a location list
+ * @vr_die: a variable DIE
+ */
+static bool die_has_loclist(Dwarf_Die *vr_die)
+{
+	Dwarf_Attribute loc;
+	int tag = dwarf_tag(vr_die);
+
+	if (tag != DW_TAG_formal_parameter &&
+	    tag != DW_TAG_variable)
+		return false;
+
+	return (dwarf_attr_integrate(vr_die, DW_AT_location, &loc) &&
+		dwarf_whatform(&loc) == DW_FORM_sec_offset);
+}
+
+/*
+ * die_is_optimized_target - Check if target program is compiled with
+ * optimization
+ * @cu_die: a CU DIE
+ *
+ * For any object in given CU whose DW_AT_location is a location list,
+ * target program is compiled with optimization. This is applicable to
+ * clang as well.
+ */
+bool die_is_optimized_target(Dwarf_Die *cu_die)
+{
+	Dwarf_Die tmp_die;
+
+	if (die_has_loclist(cu_die))
+		return true;
+
+	if (!dwarf_child(cu_die, &tmp_die) &&
+	    die_is_optimized_target(&tmp_die))
+		return true;
+
+	if (!dwarf_siblingof(cu_die, &tmp_die) &&
+	    die_is_optimized_target(&tmp_die))
+		return true;
+
+	return false;
+}
+
+/*
+ * die_search_idx - Search index of given line address
+ * @lines: Line records of single CU
+ * @nr_lines: Number of @lines
+ * @addr: address we are looking for
+ * @idx: index to be set by this function (return value)
+ *
+ * Search for @addr by looping over every lines of CU. If address
+ * matches, set index of that line in @idx. Note that single source
+ * line can have multiple line records. i.e. single source line can
+ * have multiple index.
+ */
+static bool die_search_idx(Dwarf_Lines *lines, unsigned long nr_lines,
+			   Dwarf_Addr addr, unsigned long *idx)
+{
+	unsigned long i;
+	Dwarf_Addr tmp;
+
+	for (i = 0; i < nr_lines; i++) {
+		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &tmp))
+			return false;
+
+		if (tmp == addr) {
+			*idx = i;
+			return true;
+		}
+	}
+	return false;
+}
+
+/*
+ * die_get_postprologue_addr - Search next address after function prologue
+ * @entrypc_idx: entrypc index
+ * @lines: Line records of single CU
+ * @nr_lines: Number of @lines
+ * @hignpc: high PC address of function
+ * @postprologue_addr: Next address after function prologue (return value)
+ *
+ * Look for prologue-end marker. If there is no explicit marker, return
+ * address of next line record or next source line.
+ */
+static bool die_get_postprologue_addr(unsigned long entrypc_idx,
+				      Dwarf_Lines *lines,
+				      unsigned long nr_lines,
+				      Dwarf_Addr highpc,
+				      Dwarf_Addr *postprologue_addr)
+{
+	unsigned long i;
+	int entrypc_lno, lno;
+	Dwarf_Line *line;
+	Dwarf_Addr addr;
+	bool p_end;
+
+	/* entrypc_lno is actual source line number */
+	line = dwarf_onesrcline(lines, entrypc_idx);
+	if (dwarf_lineno(line, &entrypc_lno))
+		return false;
+
+	for (i = entrypc_idx; i < nr_lines; i++) {
+		line = dwarf_onesrcline(lines, i);
+
+		if (dwarf_lineaddr(line, &addr) ||
+		    dwarf_lineno(line, &lno)    ||
+		    dwarf_lineprologueend(line, &p_end))
+			return false;
+
+		/* highpc is exclusive. [entrypc,highpc) */
+		if (addr >= highpc)
+			break;
+
+		/* clang supports prologue-end marker */
+		if (p_end)
+			break;
+
+		/* Actual next line in source */
+		if (lno != entrypc_lno)
+			break;
+
+		/*
+		 * Single source line can have multiple line records.
+		 * For Example,
+		 *     void foo() { printf("hello\n"); }
+		 * contains two line records. One points to declaration and
+		 * other points to printf() line. Variable 'lno' won't get
+		 * incremented in this case but 'i' will.
+		 */
+		if (i != entrypc_idx)
+			break;
+	}
+
+	dwarf_lineaddr(line, postprologue_addr);
+	if (*postprologue_addr >= highpc)
+		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
+			       postprologue_addr);
+
+	return true;
+}
+
+/*
+ * die_skip_prologue - Use next address after prologue as probe location
+ * @sp_die: a subprogram DIE
+ * @cu_die: a CU DIE
+ * @entrypc: entrypc of the function
+ *
+ * Function prologue prepares stack and registers before executing function
+ * logic. When target program is compiled without optimization, function
+ * parameter information is only valid after prologue. When we probe entrypc
+ * of the function, and try to record function parameter, it contains
+ * garbage value.
+ */
+void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
+		       Dwarf_Addr *entrypc)
+{
+	size_t nr_lines = 0;
+	unsigned long entrypc_idx = 0;
+	Dwarf_Lines *lines = NULL;
+	Dwarf_Addr postprologue_addr;
+	Dwarf_Addr highpc;
+
+	if (dwarf_highpc(sp_die, &highpc))
+		return;
+
+	if (dwarf_getsrclines(cu_die, &lines, &nr_lines))
+		return;
+
+	if (!die_search_idx(lines, nr_lines, *entrypc, &entrypc_idx))
+		return;
+
+	if (!die_get_postprologue_addr(entrypc_idx, lines, nr_lines,
+				       highpc, &postprologue_addr))
+		return;
+
+	*entrypc = postprologue_addr;
+}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index dc0ce1a..8b6d2f8 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -125,4 +125,12 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
 /* Get the name and type of given variable DIE, stored as "type\tname" */
 int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
 int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
+
+/* Check if target program is compiled with optimization */
+bool die_is_optimized_target(Dwarf_Die *cu_die);
+
+/* Use next address after prologue as probe location */
+void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
+		       Dwarf_Addr *entrypc);
+
 #endif
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 945cf7a..72f1152 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -901,138 +901,6 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
 	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
 }
 
-static bool var_has_loclist(Dwarf_Die *cu_die)
-{
-	Dwarf_Attribute loc;
-	int tag = dwarf_tag(cu_die);
-
-	if (tag != DW_TAG_formal_parameter &&
-	    tag != DW_TAG_variable)
-		return false;
-
-	return (dwarf_attr_integrate(cu_die, DW_AT_location, &loc) &&
-		dwarf_whatform(&loc) == DW_FORM_sec_offset);
-}
-
-/*
- * For any object in given CU whose DW_AT_location is a location list,
- * target program is compiled with optimization.
- */
-static bool optimized_target(Dwarf_Die *cu_die)
-{
-	Dwarf_Die tmp_die;
-
-	if (var_has_loclist(cu_die))
-		return true;
-
-	if (!dwarf_child(cu_die, &tmp_die) && optimized_target(&tmp_die))
-		return true;
-
-	if (!dwarf_siblingof(cu_die, &tmp_die) && optimized_target(&tmp_die))
-		return true;
-
-	return false;
-}
-
-static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
-			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
-{
-	unsigned long i;
-	Dwarf_Addr addr;
-
-	for (i = 0; i < nr_lines; i++) {
-		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
-			return false;
-
-		if (addr == pf_addr) {
-			*entrypc_idx = i;
-			return true;
-		}
-	}
-	return false;
-}
-
-static bool get_postprologue_addr(unsigned long entrypc_idx,
-				  Dwarf_Lines *lines,
-				  unsigned long nr_lines,
-				  Dwarf_Addr highpc,
-				  Dwarf_Addr *postprologue_addr)
-{
-	unsigned long i;
-	int entrypc_lno, lno;
-	Dwarf_Line *line;
-	Dwarf_Addr addr;
-	bool p_end;
-
-	/* entrypc_lno is actual source line number */
-	line = dwarf_onesrcline(lines, entrypc_idx);
-	if (dwarf_lineno(line, &entrypc_lno))
-		return false;
-
-	for (i = entrypc_idx; i < nr_lines; i++) {
-		line = dwarf_onesrcline(lines, i);
-
-		if (dwarf_lineaddr(line, &addr) ||
-		    dwarf_lineno(line, &lno)    ||
-		    dwarf_lineprologueend(line, &p_end))
-			return false;
-
-		/* highpc is exclusive. [entrypc,highpc) */
-		if (addr >= highpc)
-			break;
-
-		/* clang supports prologue-end marker */
-		if (p_end)
-			break;
-
-		/* Actual next line in source */
-		if (lno != entrypc_lno)
-			break;
-
-		/*
-		 * Single source line can have multiple line records.
-		 * For Example,
-		 *     void foo() { printf("hello\n"); }
-		 * contains two line records. One points to declaration and
-		 * other points to printf() line. Variable 'lno' won't get
-		 * incremented in this case but 'i' will.
-		 */
-		if (i != entrypc_idx)
-			break;
-	}
-
-	dwarf_lineaddr(line, postprologue_addr);
-	if (*postprologue_addr >= highpc)
-		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
-			       postprologue_addr);
-
-	return true;
-}
-
-static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
-{
-	size_t nr_lines = 0;
-	unsigned long entrypc_idx = 0;
-	Dwarf_Lines *lines = NULL;
-	Dwarf_Addr postprologue_addr;
-	Dwarf_Addr highpc;
-
-	if (dwarf_highpc(sp_die, &highpc))
-		return;
-
-	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
-		return;
-
-	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
-		return;
-
-	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
-				   highpc, &postprologue_addr))
-		return;
-
-	pf->addr = postprologue_addr;
-}
-
 static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 {
 	struct perf_probe_point *pp = &pf->pev->point;
@@ -1042,7 +910,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 		return;
 
 	/* Compiled with optimization? */
-	if (optimized_target(&pf->cu_die))
+	if (die_is_optimized_target(&pf->cu_die))
 		return;
 
 	/* Don't know entrypc? */
@@ -1062,7 +930,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 		"Probe on address 0x%" PRIx64 " to force probing at the function entry.\n\n",
 		pf->addr);
 
-	__skip_prologue(sp_die, pf);
+	die_skip_prologue(sp_die, &pf->cu_die, &pf->addr);
 }
 
 static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
-- 
2.5.5

Re: [PATCH v2] perf probe: Move dwarf specific functions to dwarf-aux.c

[Masami Hiramatsu]

On Tue, 30 Aug 2016 14:09:37 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> Move generic dwarf related functions from util/probe-finder.c to
> util/dwarf-aux.c. Functions name and their prototype are also
> changed accordingly. No functionality changes.

Great! 
Looks good to me :)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> 
> Suggested-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> Changes in v2:
>   - Add comments about functions prototype and their usage
>   - Replace function parameter names with more generic names
> 
>  tools/perf/util/dwarf-aux.c    | 179 +++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/dwarf-aux.h    |   8 ++
>  tools/perf/util/probe-finder.c | 136 +------------------------------
>  3 files changed, 189 insertions(+), 134 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index a347b19..faec899 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1085,3 +1085,182 @@ int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
>  	return -ENOTSUP;
>  }
>  #endif
> +
> +/*
> + * die_has_loclist - Check if DW_AT_location of @vr_die is a location list
> + * @vr_die: a variable DIE
> + */
> +static bool die_has_loclist(Dwarf_Die *vr_die)
> +{
> +	Dwarf_Attribute loc;
> +	int tag = dwarf_tag(vr_die);
> +
> +	if (tag != DW_TAG_formal_parameter &&
> +	    tag != DW_TAG_variable)
> +		return false;
> +
> +	return (dwarf_attr_integrate(vr_die, DW_AT_location, &loc) &&
> +		dwarf_whatform(&loc) == DW_FORM_sec_offset);
> +}
> +
> +/*
> + * die_is_optimized_target - Check if target program is compiled with
> + * optimization
> + * @cu_die: a CU DIE
> + *
> + * For any object in given CU whose DW_AT_location is a location list,
> + * target program is compiled with optimization. This is applicable to
> + * clang as well.
> + */
> +bool die_is_optimized_target(Dwarf_Die *cu_die)
> +{
> +	Dwarf_Die tmp_die;
> +
> +	if (die_has_loclist(cu_die))
> +		return true;
> +
> +	if (!dwarf_child(cu_die, &tmp_die) &&
> +	    die_is_optimized_target(&tmp_die))
> +		return true;
> +
> +	if (!dwarf_siblingof(cu_die, &tmp_die) &&
> +	    die_is_optimized_target(&tmp_die))
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * die_search_idx - Search index of given line address
> + * @lines: Line records of single CU
> + * @nr_lines: Number of @lines
> + * @addr: address we are looking for
> + * @idx: index to be set by this function (return value)
> + *
> + * Search for @addr by looping over every lines of CU. If address
> + * matches, set index of that line in @idx. Note that single source
> + * line can have multiple line records. i.e. single source line can
> + * have multiple index.
> + */
> +static bool die_search_idx(Dwarf_Lines *lines, unsigned long nr_lines,
> +			   Dwarf_Addr addr, unsigned long *idx)
> +{
> +	unsigned long i;
> +	Dwarf_Addr tmp;
> +
> +	for (i = 0; i < nr_lines; i++) {
> +		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &tmp))
> +			return false;
> +
> +		if (tmp == addr) {
> +			*idx = i;
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +/*
> + * die_get_postprologue_addr - Search next address after function prologue
> + * @entrypc_idx: entrypc index
> + * @lines: Line records of single CU
> + * @nr_lines: Number of @lines
> + * @hignpc: high PC address of function
> + * @postprologue_addr: Next address after function prologue (return value)
> + *
> + * Look for prologue-end marker. If there is no explicit marker, return
> + * address of next line record or next source line.
> + */
> +static bool die_get_postprologue_addr(unsigned long entrypc_idx,
> +				      Dwarf_Lines *lines,
> +				      unsigned long nr_lines,
> +				      Dwarf_Addr highpc,
> +				      Dwarf_Addr *postprologue_addr)
> +{
> +	unsigned long i;
> +	int entrypc_lno, lno;
> +	Dwarf_Line *line;
> +	Dwarf_Addr addr;
> +	bool p_end;
> +
> +	/* entrypc_lno is actual source line number */
> +	line = dwarf_onesrcline(lines, entrypc_idx);
> +	if (dwarf_lineno(line, &entrypc_lno))
> +		return false;
> +
> +	for (i = entrypc_idx; i < nr_lines; i++) {
> +		line = dwarf_onesrcline(lines, i);
> +
> +		if (dwarf_lineaddr(line, &addr) ||
> +		    dwarf_lineno(line, &lno)    ||
> +		    dwarf_lineprologueend(line, &p_end))
> +			return false;
> +
> +		/* highpc is exclusive. [entrypc,highpc) */
> +		if (addr >= highpc)
> +			break;
> +
> +		/* clang supports prologue-end marker */
> +		if (p_end)
> +			break;
> +
> +		/* Actual next line in source */
> +		if (lno != entrypc_lno)
> +			break;
> +
> +		/*
> +		 * Single source line can have multiple line records.
> +		 * For Example,
> +		 *     void foo() { printf("hello\n"); }
> +		 * contains two line records. One points to declaration and
> +		 * other points to printf() line. Variable 'lno' won't get
> +		 * incremented in this case but 'i' will.
> +		 */
> +		if (i != entrypc_idx)
> +			break;
> +	}
> +
> +	dwarf_lineaddr(line, postprologue_addr);
> +	if (*postprologue_addr >= highpc)
> +		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> +			       postprologue_addr);
> +
> +	return true;
> +}
> +
> +/*
> + * die_skip_prologue - Use next address after prologue as probe location
> + * @sp_die: a subprogram DIE
> + * @cu_die: a CU DIE
> + * @entrypc: entrypc of the function
> + *
> + * Function prologue prepares stack and registers before executing function
> + * logic. When target program is compiled without optimization, function
> + * parameter information is only valid after prologue. When we probe entrypc
> + * of the function, and try to record function parameter, it contains
> + * garbage value.
> + */
> +void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
> +		       Dwarf_Addr *entrypc)
> +{
> +	size_t nr_lines = 0;
> +	unsigned long entrypc_idx = 0;
> +	Dwarf_Lines *lines = NULL;
> +	Dwarf_Addr postprologue_addr;
> +	Dwarf_Addr highpc;
> +
> +	if (dwarf_highpc(sp_die, &highpc))
> +		return;
> +
> +	if (dwarf_getsrclines(cu_die, &lines, &nr_lines))
> +		return;
> +
> +	if (!die_search_idx(lines, nr_lines, *entrypc, &entrypc_idx))
> +		return;
> +
> +	if (!die_get_postprologue_addr(entrypc_idx, lines, nr_lines,
> +				       highpc, &postprologue_addr))
> +		return;
> +
> +	*entrypc = postprologue_addr;
> +}
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index dc0ce1a..8b6d2f8 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -125,4 +125,12 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
>  /* Get the name and type of given variable DIE, stored as "type\tname" */
>  int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
>  int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
> +
> +/* Check if target program is compiled with optimization */
> +bool die_is_optimized_target(Dwarf_Die *cu_die);
> +
> +/* Use next address after prologue as probe location */
> +void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
> +		       Dwarf_Addr *entrypc);
> +
>  #endif
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 945cf7a..72f1152 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -901,138 +901,6 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
>  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
>  }
>  
> -static bool var_has_loclist(Dwarf_Die *cu_die)
> -{
> -	Dwarf_Attribute loc;
> -	int tag = dwarf_tag(cu_die);
> -
> -	if (tag != DW_TAG_formal_parameter &&
> -	    tag != DW_TAG_variable)
> -		return false;
> -
> -	return (dwarf_attr_integrate(cu_die, DW_AT_location, &loc) &&
> -		dwarf_whatform(&loc) == DW_FORM_sec_offset);
> -}
> -
> -/*
> - * For any object in given CU whose DW_AT_location is a location list,
> - * target program is compiled with optimization.
> - */
> -static bool optimized_target(Dwarf_Die *cu_die)
> -{
> -	Dwarf_Die tmp_die;
> -
> -	if (var_has_loclist(cu_die))
> -		return true;
> -
> -	if (!dwarf_child(cu_die, &tmp_die) && optimized_target(&tmp_die))
> -		return true;
> -
> -	if (!dwarf_siblingof(cu_die, &tmp_die) && optimized_target(&tmp_die))
> -		return true;
> -
> -	return false;
> -}
> -
> -static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
> -			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
> -{
> -	unsigned long i;
> -	Dwarf_Addr addr;
> -
> -	for (i = 0; i < nr_lines; i++) {
> -		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
> -			return false;
> -
> -		if (addr == pf_addr) {
> -			*entrypc_idx = i;
> -			return true;
> -		}
> -	}
> -	return false;
> -}
> -
> -static bool get_postprologue_addr(unsigned long entrypc_idx,
> -				  Dwarf_Lines *lines,
> -				  unsigned long nr_lines,
> -				  Dwarf_Addr highpc,
> -				  Dwarf_Addr *postprologue_addr)
> -{
> -	unsigned long i;
> -	int entrypc_lno, lno;
> -	Dwarf_Line *line;
> -	Dwarf_Addr addr;
> -	bool p_end;
> -
> -	/* entrypc_lno is actual source line number */
> -	line = dwarf_onesrcline(lines, entrypc_idx);
> -	if (dwarf_lineno(line, &entrypc_lno))
> -		return false;
> -
> -	for (i = entrypc_idx; i < nr_lines; i++) {
> -		line = dwarf_onesrcline(lines, i);
> -
> -		if (dwarf_lineaddr(line, &addr) ||
> -		    dwarf_lineno(line, &lno)    ||
> -		    dwarf_lineprologueend(line, &p_end))
> -			return false;
> -
> -		/* highpc is exclusive. [entrypc,highpc) */
> -		if (addr >= highpc)
> -			break;
> -
> -		/* clang supports prologue-end marker */
> -		if (p_end)
> -			break;
> -
> -		/* Actual next line in source */
> -		if (lno != entrypc_lno)
> -			break;
> -
> -		/*
> -		 * Single source line can have multiple line records.
> -		 * For Example,
> -		 *     void foo() { printf("hello\n"); }
> -		 * contains two line records. One points to declaration and
> -		 * other points to printf() line. Variable 'lno' won't get
> -		 * incremented in this case but 'i' will.
> -		 */
> -		if (i != entrypc_idx)
> -			break;
> -	}
> -
> -	dwarf_lineaddr(line, postprologue_addr);
> -	if (*postprologue_addr >= highpc)
> -		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
> -			       postprologue_addr);
> -
> -	return true;
> -}
> -
> -static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
> -{
> -	size_t nr_lines = 0;
> -	unsigned long entrypc_idx = 0;
> -	Dwarf_Lines *lines = NULL;
> -	Dwarf_Addr postprologue_addr;
> -	Dwarf_Addr highpc;
> -
> -	if (dwarf_highpc(sp_die, &highpc))
> -		return;
> -
> -	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
> -		return;
> -
> -	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
> -		return;
> -
> -	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
> -				   highpc, &postprologue_addr))
> -		return;
> -
> -	pf->addr = postprologue_addr;
> -}
> -
>  static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
>  {
>  	struct perf_probe_point *pp = &pf->pev->point;
> @@ -1042,7 +910,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
>  		return;
>  
>  	/* Compiled with optimization? */
> -	if (optimized_target(&pf->cu_die))
> +	if (die_is_optimized_target(&pf->cu_die))
>  		return;
>  
>  	/* Don't know entrypc? */
> @@ -1062,7 +930,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
>  		"Probe on address 0x%" PRIx64 " to force probing at the function entry.\n\n",
>  		pf->addr);
>  
> -	__skip_prologue(sp_die, pf);
> +	die_skip_prologue(sp_die, &pf->cu_die, &pf->addr);
>  }
>  
>  static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
> -- 
> 2.5.5
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[tip:perf/core] perf probe: Move dwarf specific functions to dwarf-aux.c

[tip-bot for Ravi Bangoria]

Commit-ID:  6243b9dc4c991fe8bdc53a0e029908aef3ddb101
Gitweb:     http://git.kernel.org/tip/6243b9dc4c991fe8bdc53a0e029908aef3ddb101
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Tue, 30 Aug 2016 14:09:37 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 1 Sep 2016 12:42:26 -0300

perf probe: Move dwarf specific functions to dwarf-aux.c

Move generic dwarf related functions from util/probe-finder.c to
util/dwarf-aux.c. Functions name and their prototype are also changed
accordingly. No functionality changes.

Suggested-and-Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1472546377-25612-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c    | 179 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h    |   8 ++
 tools/perf/util/probe-finder.c | 136 +------------------------------
 3 files changed, 189 insertions(+), 134 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index a347b19..faec899 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1085,3 +1085,182 @@ int die_get_var_range(Dwarf_Die *sp_die __maybe_unused,
 	return -ENOTSUP;
 }
 #endif
+
+/*
+ * die_has_loclist - Check if DW_AT_location of @vr_die is a location list
+ * @vr_die: a variable DIE
+ */
+static bool die_has_loclist(Dwarf_Die *vr_die)
+{
+	Dwarf_Attribute loc;
+	int tag = dwarf_tag(vr_die);
+
+	if (tag != DW_TAG_formal_parameter &&
+	    tag != DW_TAG_variable)
+		return false;
+
+	return (dwarf_attr_integrate(vr_die, DW_AT_location, &loc) &&
+		dwarf_whatform(&loc) == DW_FORM_sec_offset);
+}
+
+/*
+ * die_is_optimized_target - Check if target program is compiled with
+ * optimization
+ * @cu_die: a CU DIE
+ *
+ * For any object in given CU whose DW_AT_location is a location list,
+ * target program is compiled with optimization. This is applicable to
+ * clang as well.
+ */
+bool die_is_optimized_target(Dwarf_Die *cu_die)
+{
+	Dwarf_Die tmp_die;
+
+	if (die_has_loclist(cu_die))
+		return true;
+
+	if (!dwarf_child(cu_die, &tmp_die) &&
+	    die_is_optimized_target(&tmp_die))
+		return true;
+
+	if (!dwarf_siblingof(cu_die, &tmp_die) &&
+	    die_is_optimized_target(&tmp_die))
+		return true;
+
+	return false;
+}
+
+/*
+ * die_search_idx - Search index of given line address
+ * @lines: Line records of single CU
+ * @nr_lines: Number of @lines
+ * @addr: address we are looking for
+ * @idx: index to be set by this function (return value)
+ *
+ * Search for @addr by looping over every lines of CU. If address
+ * matches, set index of that line in @idx. Note that single source
+ * line can have multiple line records. i.e. single source line can
+ * have multiple index.
+ */
+static bool die_search_idx(Dwarf_Lines *lines, unsigned long nr_lines,
+			   Dwarf_Addr addr, unsigned long *idx)
+{
+	unsigned long i;
+	Dwarf_Addr tmp;
+
+	for (i = 0; i < nr_lines; i++) {
+		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &tmp))
+			return false;
+
+		if (tmp == addr) {
+			*idx = i;
+			return true;
+		}
+	}
+	return false;
+}
+
+/*
+ * die_get_postprologue_addr - Search next address after function prologue
+ * @entrypc_idx: entrypc index
+ * @lines: Line records of single CU
+ * @nr_lines: Number of @lines
+ * @hignpc: high PC address of function
+ * @postprologue_addr: Next address after function prologue (return value)
+ *
+ * Look for prologue-end marker. If there is no explicit marker, return
+ * address of next line record or next source line.
+ */
+static bool die_get_postprologue_addr(unsigned long entrypc_idx,
+				      Dwarf_Lines *lines,
+				      unsigned long nr_lines,
+				      Dwarf_Addr highpc,
+				      Dwarf_Addr *postprologue_addr)
+{
+	unsigned long i;
+	int entrypc_lno, lno;
+	Dwarf_Line *line;
+	Dwarf_Addr addr;
+	bool p_end;
+
+	/* entrypc_lno is actual source line number */
+	line = dwarf_onesrcline(lines, entrypc_idx);
+	if (dwarf_lineno(line, &entrypc_lno))
+		return false;
+
+	for (i = entrypc_idx; i < nr_lines; i++) {
+		line = dwarf_onesrcline(lines, i);
+
+		if (dwarf_lineaddr(line, &addr) ||
+		    dwarf_lineno(line, &lno)    ||
+		    dwarf_lineprologueend(line, &p_end))
+			return false;
+
+		/* highpc is exclusive. [entrypc,highpc) */
+		if (addr >= highpc)
+			break;
+
+		/* clang supports prologue-end marker */
+		if (p_end)
+			break;
+
+		/* Actual next line in source */
+		if (lno != entrypc_lno)
+			break;
+
+		/*
+		 * Single source line can have multiple line records.
+		 * For Example,
+		 *     void foo() { printf("hello\n"); }
+		 * contains two line records. One points to declaration and
+		 * other points to printf() line. Variable 'lno' won't get
+		 * incremented in this case but 'i' will.
+		 */
+		if (i != entrypc_idx)
+			break;
+	}
+
+	dwarf_lineaddr(line, postprologue_addr);
+	if (*postprologue_addr >= highpc)
+		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
+			       postprologue_addr);
+
+	return true;
+}
+
+/*
+ * die_skip_prologue - Use next address after prologue as probe location
+ * @sp_die: a subprogram DIE
+ * @cu_die: a CU DIE
+ * @entrypc: entrypc of the function
+ *
+ * Function prologue prepares stack and registers before executing function
+ * logic. When target program is compiled without optimization, function
+ * parameter information is only valid after prologue. When we probe entrypc
+ * of the function, and try to record function parameter, it contains
+ * garbage value.
+ */
+void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
+		       Dwarf_Addr *entrypc)
+{
+	size_t nr_lines = 0;
+	unsigned long entrypc_idx = 0;
+	Dwarf_Lines *lines = NULL;
+	Dwarf_Addr postprologue_addr;
+	Dwarf_Addr highpc;
+
+	if (dwarf_highpc(sp_die, &highpc))
+		return;
+
+	if (dwarf_getsrclines(cu_die, &lines, &nr_lines))
+		return;
+
+	if (!die_search_idx(lines, nr_lines, *entrypc, &entrypc_idx))
+		return;
+
+	if (!die_get_postprologue_addr(entrypc_idx, lines, nr_lines,
+				       highpc, &postprologue_addr))
+		return;
+
+	*entrypc = postprologue_addr;
+}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index dc0ce1a..8b6d2f8 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -125,4 +125,12 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
 /* Get the name and type of given variable DIE, stored as "type\tname" */
 int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
 int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf);
+
+/* Check if target program is compiled with optimization */
+bool die_is_optimized_target(Dwarf_Die *cu_die);
+
+/* Use next address after prologue as probe location */
+void die_skip_prologue(Dwarf_Die *sp_die, Dwarf_Die *cu_die,
+		       Dwarf_Addr *entrypc);
+
 #endif
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 003ecad..8daca4f 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -907,138 +907,6 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
 	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
 }
 
-static bool var_has_loclist(Dwarf_Die *cu_die)
-{
-	Dwarf_Attribute loc;
-	int tag = dwarf_tag(cu_die);
-
-	if (tag != DW_TAG_formal_parameter &&
-	    tag != DW_TAG_variable)
-		return false;
-
-	return (dwarf_attr_integrate(cu_die, DW_AT_location, &loc) &&
-		dwarf_whatform(&loc) == DW_FORM_sec_offset);
-}
-
-/*
- * For any object in given CU whose DW_AT_location is a location list,
- * target program is compiled with optimization.
- */
-static bool optimized_target(Dwarf_Die *cu_die)
-{
-	Dwarf_Die tmp_die;
-
-	if (var_has_loclist(cu_die))
-		return true;
-
-	if (!dwarf_child(cu_die, &tmp_die) && optimized_target(&tmp_die))
-		return true;
-
-	if (!dwarf_siblingof(cu_die, &tmp_die) && optimized_target(&tmp_die))
-		return true;
-
-	return false;
-}
-
-static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
-			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
-{
-	unsigned long i;
-	Dwarf_Addr addr;
-
-	for (i = 0; i < nr_lines; i++) {
-		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
-			return false;
-
-		if (addr == pf_addr) {
-			*entrypc_idx = i;
-			return true;
-		}
-	}
-	return false;
-}
-
-static bool get_postprologue_addr(unsigned long entrypc_idx,
-				  Dwarf_Lines *lines,
-				  unsigned long nr_lines,
-				  Dwarf_Addr highpc,
-				  Dwarf_Addr *postprologue_addr)
-{
-	unsigned long i;
-	int entrypc_lno, lno;
-	Dwarf_Line *line;
-	Dwarf_Addr addr;
-	bool p_end;
-
-	/* entrypc_lno is actual source line number */
-	line = dwarf_onesrcline(lines, entrypc_idx);
-	if (dwarf_lineno(line, &entrypc_lno))
-		return false;
-
-	for (i = entrypc_idx; i < nr_lines; i++) {
-		line = dwarf_onesrcline(lines, i);
-
-		if (dwarf_lineaddr(line, &addr) ||
-		    dwarf_lineno(line, &lno)    ||
-		    dwarf_lineprologueend(line, &p_end))
-			return false;
-
-		/* highpc is exclusive. [entrypc,highpc) */
-		if (addr >= highpc)
-			break;
-
-		/* clang supports prologue-end marker */
-		if (p_end)
-			break;
-
-		/* Actual next line in source */
-		if (lno != entrypc_lno)
-			break;
-
-		/*
-		 * Single source line can have multiple line records.
-		 * For Example,
-		 *     void foo() { printf("hello\n"); }
-		 * contains two line records. One points to declaration and
-		 * other points to printf() line. Variable 'lno' won't get
-		 * incremented in this case but 'i' will.
-		 */
-		if (i != entrypc_idx)
-			break;
-	}
-
-	dwarf_lineaddr(line, postprologue_addr);
-	if (*postprologue_addr >= highpc)
-		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
-			       postprologue_addr);
-
-	return true;
-}
-
-static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
-{
-	size_t nr_lines = 0;
-	unsigned long entrypc_idx = 0;
-	Dwarf_Lines *lines = NULL;
-	Dwarf_Addr postprologue_addr;
-	Dwarf_Addr highpc;
-
-	if (dwarf_highpc(sp_die, &highpc))
-		return;
-
-	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
-		return;
-
-	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
-		return;
-
-	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
-				   highpc, &postprologue_addr))
-		return;
-
-	pf->addr = postprologue_addr;
-}
-
 static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 {
 	struct perf_probe_point *pp = &pf->pev->point;
@@ -1048,7 +916,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 		return;
 
 	/* Compiled with optimization? */
-	if (optimized_target(&pf->cu_die))
+	if (die_is_optimized_target(&pf->cu_die))
 		return;
 
 	/* Don't know entrypc? */
@@ -1068,7 +936,7 @@ static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
 		"Probe on address 0x%" PRIx64 " to force probing at the function entry.\n\n",
 		pf->addr);
 
-	__skip_prologue(sp_die, pf);
+	die_skip_prologue(sp_die, &pf->cu_die, &pf->addr);
 }
 
 static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Ravi Bangoria]

On Saturday 27 August 2016 01:24 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Aug 26, 2016 at 04:30:27PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Wed, Aug 03, 2016 at 02:28:45PM +0530, Ravi Bangoria escreveu:
>>> +++ b/tools/perf/util/probe-finder.c
>>> @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
>>>  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
>>>  }
>>   
>>> +static bool var_has_loclist(Dwarf_Die *die)
>> So, the variable 'die' cause the build to fail in multiple systems, I'm
>> renaming it to 'cu_die', there are some more problems when cross compiling it
>> to some arches, I'm trying to fix them all now:
> Fixed, please check:
>
> http://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/urgent&id=3866e3762da3291613dfb8b193885a8ed3836669

Thanks Arnaldo,

I've tested it. Looks fine to me.

-Ravi

> - Arnaldo
>

Re: [PATCH v2 2/2] perf uprobe: Skip prologue if program compiled without optimization

[Ravi Bangoria]

On Saturday 27 August 2016 01:00 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Aug 03, 2016 at 02:28:45PM +0530, Ravi Bangoria escreveu:
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -892,6 +892,169 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
>>  	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
>>  }
>   
>> +static bool var_has_loclist(Dwarf_Die *die)
> So, the variable 'die' cause the build to fail in multiple systems, I'm
> renaming it to 'cu_die', there are some more problems when cross compiling it
> to some arches, I'm trying to fix them all now:

Sorry for this. I started using "make -C tools/perf build-test" but I don't have
infrastructure handy to test for cross arch.

If possible, can you please provide that to me. Otherwise I've to create it from
scratch.

-Ravi

> [root@jouet ~]# dm
>  1 65.668882867 alpine:3.4: Ok
>  2 24.327143744 android-ndk:r12b-arm: Ok
>  3 76.000455817 archlinux:latest: Ok
>  4 40.905282317 centos:5: Ok
>  5 28.799284950 centos:6: FAIL
> cc1: warnings being treated as errors
> util/probe-finder.c: In function 'var_has_loclist':
> util/probe-finder.c:904: error: declaration of 'die' shadows a global declaration
> util/util.h:137: error: shadowed declaration is here
> util/probe-finder.c: In function 'optimized_target':
> util/probe-finder.c:921: error: declaration of 'die' shadows a global declaration
> util/util.h:137: error: shadowed declaration is here
> -----------------------------------------------------------------------------
>  6 68.157498673 centos:7: Ok
>  7 32.814597820 debian:7: FAIL
> util/probe-finder.c: In function 'var_has_loclist':
> util/probe-finder.c:904:40: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
> In file included from util/probe-finder.c:39:0:
> util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
> util/probe-finder.c: In function 'optimized_target':
> util/probe-finder.c:921:41: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
> In file included from util/probe-finder.c:39:0:
> util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
> -----------------------------------------------------------------------------
>  8 69.535114125 debian:8: Ok
>  9 71.524733936 debian:experimental: Ok
> 10 66.956671932 fedora:20: Ok
> 11 72.127804972 fedora:21: Ok
> 12 75.767022465 fedora:22: Ok
> 13 72.244964710 fedora:23: Ok
> 14 75.556712624 fedora:24: Ok
> 15 30.825802615 fedora:24-x-ARC-uClibc: Ok
> 16 76.874903316 fedora:rawhide: Ok
> 17 76.756388879 mageia:5: Ok
> 18 71.700493646 opensuse:13.2: Ok
> 19 69.974649379 opensuse:42.1: Ok
> 20 75.265305896 opensuse:tumbleweed: Ok
> 21 28.800283739 ubuntu:12.04.5: FAIL
> util/probe-finder.c: In function 'var_has_loclist':
> util/probe-finder.c:904:40: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
> util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
> util/probe-finder.c: In function 'optimized_target':
> util/probe-finder.c:921:41: error: declaration of 'die' shadows a global declaration [-Werror=shadow]
> util/util.h:137:6: error: shadowed declaration is here [-Werror=shadow]
> -----------------------------------------------------------------------------
> 22 66.514504455 ubuntu:14.04.4: Ok
> 23 69.100413367 ubuntu:15.10: Ok
> 24 64.414190947 ubuntu:16.04: Ok
> 25 29.481652689 ubuntu:16.04-x-arm: FAIL
> util/probe-finder.c: In function '__skip_prologue':
> util/probe-finder.c:1022:45: error: passing argument 3 of 'dwarf_getsrclines' from incompatible pointer type [-Werror=incompatible-pointer-types]
>   if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
>                                              ^
> In file included from util/dwarf-aux.h:23:0,
>                  from util/probe-finder.h:24,
>                  from util/probe-finder.c:41:
> /usr/arm-linux-gnueabihf/include/elfutils/libdw.h:592:12: note: expected 'size_t * {aka unsigned int *}' but argument is of type 'long unsigned int *'
>  extern int dwarf_getsrclines (Dwarf_Die *cudie, Dwarf_Lines **lines,
>             ^
> In file included from util/probe-finder.c:37:0:
> util/probe-finder.c: In function 'skip_prologue':
> util/probe-finder.c:1060:10: error: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'Dwarf_Addr {aka long long unsigned int}' [-Werror=format=]
>   pr_info("Target program is compiled without optimization. Skipping prologue.\n"
>           ^
> util/debug.h:18:21: note: in definition of macro 'pr_fmt'
>  #define pr_fmt(fmt) fmt
>                      ^
> util/probe-finder.c:1060:2: note: in expansion of macro 'pr_info'
>   pr_info("Target program is compiled without optimization. Skipping prologue.\n"
>   ^
> -----------------------------------------------------------------------------
> 26 51.979875334 ubuntu:16.04-x-arm64: Ok
> 27 28.808591224 ubuntu:16.04-x-powerpc64: FAIL
>   CC       /tmp/build/perf/tests/bp_signal.o
> util/probe-finder.c: In function '__skip_prologue':
> util/probe-finder.c:1022:45: error: passing argument 3 of 'dwarf_getsrclines' from incompatible pointer type [-Werror=incompatible-pointer-types]
>   if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
>                                              ^
> In file included from util/dwarf-aux.h:23:0,
>                  from util/probe-finder.h:24,
>                  from util/probe-finder.c:41:
> /usr/powerpc-linux-gnu/include/elfutils/libdw.h:592:12: note: expected 'size_t * {aka unsigned int *}' but argument is of type 'long unsigned int *'
>  extern int dwarf_getsrclines (Dwarf_Die *cudie, Dwarf_Lines **lines,
>             ^
> In file included from util/probe-finder.c:37:0:
> util/probe-finder.c: In function 'skip_prologue':
> util/probe-finder.c:1060:10: error: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'Dwarf_Addr {aka long long unsigned int}' [-Werror=format=]
>   pr_info("Target program is compiled without optimization. Skipping prologue.\n"
>           ^
> util/debug.h:18:21: note: in definition of macro 'pr_fmt'
>  #define pr_fmt(fmt) fmt
>                      ^
> util/probe-finder.c:1060:2: note: in expansion of macro 'pr_info'
> -----------------------------------------------------------------------------
> 28 53.372615706 ubuntu:16.04-x-powerpc64el: Ok
> 29 71.550573752 ubuntu:16.10: Ok
> 30 52.727886240 ubuntu:16.10-x-s390: Ok
>

[tip:perf/core] perf uprobe: Skip prologue if program compiled without optimization

[tip-bot for Ravi Bangoria]

Commit-ID:  e47392bf9c0613a058cd20ee89d8ce9d957d4b24
Gitweb:     http://git.kernel.org/tip/e47392bf9c0613a058cd20ee89d8ce9d957d4b24
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Wed, 3 Aug 2016 14:28:45 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 1 Sep 2016 12:42:25 -0300

perf uprobe: Skip prologue if program compiled without optimization

The function prologue prepares stack and registers before executing
function logic.

When target program is compiled without optimization, function parameter
information is only valid after the prologue.

When we probe entrypc of the function, and try to record a function
parameter, it contains a garbage value.

For example:

  $ vim test.c
    #include <stdio.h>

    void foo(int i)
    {
       printf("i: %d\n", i);
    }

    int main()
    {
      foo(42);
      return 0;
    }

  $ gcc -g test.c -o test
  $ objdump -dl test | less
    foo():
    /home/ravi/test.c:4
      400536:       55                      push   %rbp
      400537:       48 89 e5                mov    %rsp,%rbp
      40053a:       48 83 ec 10             sub    -bashx10,%rsp
      40053e:       89 7d fc                mov    %edi,-0x4(%rbp)
    /home/ravi/test.c:5
      400541:       8b 45 fc                mov    -0x4(%rbp),%eax
    ...
    ...
    main():
    /home/ravi/test.c:9
      400558:       55                      push   %rbp
      400559:       48 89 e5                mov    %rsp,%rbp
    /home/ravi/test.c:10
      40055c:       bf 2a 00 00 00          mov    -bashx2a,%edi
      400561:       e8 d0 ff ff ff          callq  400536 <foo>

  $ perf probe -x ./test 'foo i'
  $ cat /sys/kernel/debug/tracing/uprobe_events
     p:probe_test/foo /home/ravi/test:0x0000000000000536 i=-12(%sp):s32

  $ perf record -e probe_test:foo ./test
  $ perf script
     test  5778 [001]  4918.562027: probe_test:foo: (400536) i=0

Here variable 'i' is passed via stack which is pushed on stack at
0x40053e. But we are probing at 0x400536.

To resolve this issues, we need to probe on next instruction after
prologue.  gdb and systemtap also does same thing. I've implemented this
patch based on approach systemtap has used.

After applying patch:

  $ perf probe -x ./test 'foo i'
  $ cat /sys/kernel/debug/tracing/uprobe_events
    p:probe_test/foo /home/ravi/test:0x0000000000000541 i=-4(%bp):s32

  $ perf record -e probe_test:foo ./test
  $ perf script
    test  6300 [001]  5877.879327: probe_test:foo: (400541) i=42

No need to skip prologue for optimized case since debug info is correct
for each instructions for -O2 -g. For more details please visit:

        https://bugzilla.redhat.com/show_bug.cgi?id=612253#c6

Changes in v2:

- Skipping prologue only when any ARG is either C variable, $params or
  $vars.

- Probe on line(:1) may not be always possible. Recommend only address
  to force probe on function entry.

Committer notes:

Testing it with 'perf trace':

  # perf probe -x ./test foo i
  Added new event:
    probe_test:foo       (on foo in /home/acme/c/test with i)

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

	  perf record -e probe_test:foo -aR sleep 1

  # cat /sys/kernel/debug/tracing/uprobe_events
  p:probe_test/foo /home/acme/c/test:0x0000000000000526 i=-12(%sp):s32
  # trace --no-sys --event probe_*:* ./test
  i: 42
     0.000 probe_test:foo:(400526) i=0)
  #

After the patch:

  # perf probe -d *:*
  Removed event: probe_test:foo
  # perf probe -x ./test foo i
  Target program is compiled without optimization. Skipping prologue.
  Probe on address 0x400526 to force probing at the function entry.

  Added new event:
    probe_test:foo       (on foo in /home/acme/c/test with i)

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

	perf record -e probe_test:foo -aR sleep 1

  # cat /sys/kernel/debug/tracing/uprobe_events
  p:probe_test/foo /home/acme/c/test:0x0000000000000531 i=-4(%bp):s32
  # trace --no-sys --event probe_*:* ./test
  i: 42
     0.000 probe_test:foo:(400531) i=42)
  #

Reported-by: Michael Petlan <mpetlan@redhat.com>
Report-Link: https://www.mail-archive.com/linux-perf-users@vger.kernel.org/msg02348.html
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1299021
Link: http://lkml.kernel.org/r/1470214725-5023-2-git-send-email-ravi.bangoria@linux.vnet.ibm.com
[ Rename 'die' to 'cu_die' to avoid shadowing a die() definition on at least centos 5, Debian 7 and ubuntu:12.04.5]
[ Use PRIx64 instead of lx to format a Dwarf_Addr, aka long long unsigned int, fixing the build on 32-bit systems ]
[ dwarf_getsrclines() expects a size_t * argument ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 165 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 165 insertions(+)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 508b61c..003ecad 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -907,6 +907,170 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
 	return die_walk_lines(sp_die, probe_point_lazy_walker, pf);
 }
 
+static bool var_has_loclist(Dwarf_Die *cu_die)
+{
+	Dwarf_Attribute loc;
+	int tag = dwarf_tag(cu_die);
+
+	if (tag != DW_TAG_formal_parameter &&
+	    tag != DW_TAG_variable)
+		return false;
+
+	return (dwarf_attr_integrate(cu_die, DW_AT_location, &loc) &&
+		dwarf_whatform(&loc) == DW_FORM_sec_offset);
+}
+
+/*
+ * For any object in given CU whose DW_AT_location is a location list,
+ * target program is compiled with optimization.
+ */
+static bool optimized_target(Dwarf_Die *cu_die)
+{
+	Dwarf_Die tmp_die;
+
+	if (var_has_loclist(cu_die))
+		return true;
+
+	if (!dwarf_child(cu_die, &tmp_die) && optimized_target(&tmp_die))
+		return true;
+
+	if (!dwarf_siblingof(cu_die, &tmp_die) && optimized_target(&tmp_die))
+		return true;
+
+	return false;
+}
+
+static bool get_entrypc_idx(Dwarf_Lines *lines, unsigned long nr_lines,
+			    Dwarf_Addr pf_addr, unsigned long *entrypc_idx)
+{
+	unsigned long i;
+	Dwarf_Addr addr;
+
+	for (i = 0; i < nr_lines; i++) {
+		if (dwarf_lineaddr(dwarf_onesrcline(lines, i), &addr))
+			return false;
+
+		if (addr == pf_addr) {
+			*entrypc_idx = i;
+			return true;
+		}
+	}
+	return false;
+}
+
+static bool get_postprologue_addr(unsigned long entrypc_idx,
+				  Dwarf_Lines *lines,
+				  unsigned long nr_lines,
+				  Dwarf_Addr highpc,
+				  Dwarf_Addr *postprologue_addr)
+{
+	unsigned long i;
+	int entrypc_lno, lno;
+	Dwarf_Line *line;
+	Dwarf_Addr addr;
+	bool p_end;
+
+	/* entrypc_lno is actual source line number */
+	line = dwarf_onesrcline(lines, entrypc_idx);
+	if (dwarf_lineno(line, &entrypc_lno))
+		return false;
+
+	for (i = entrypc_idx; i < nr_lines; i++) {
+		line = dwarf_onesrcline(lines, i);
+
+		if (dwarf_lineaddr(line, &addr) ||
+		    dwarf_lineno(line, &lno)    ||
+		    dwarf_lineprologueend(line, &p_end))
+			return false;
+
+		/* highpc is exclusive. [entrypc,highpc) */
+		if (addr >= highpc)
+			break;
+
+		/* clang supports prologue-end marker */
+		if (p_end)
+			break;
+
+		/* Actual next line in source */
+		if (lno != entrypc_lno)
+			break;
+
+		/*
+		 * Single source line can have multiple line records.
+		 * For Example,
+		 *     void foo() { printf("hello\n"); }
+		 * contains two line records. One points to declaration and
+		 * other points to printf() line. Variable 'lno' won't get
+		 * incremented in this case but 'i' will.
+		 */
+		if (i != entrypc_idx)
+			break;
+	}
+
+	dwarf_lineaddr(line, postprologue_addr);
+	if (*postprologue_addr >= highpc)
+		dwarf_lineaddr(dwarf_onesrcline(lines, i - 1),
+			       postprologue_addr);
+
+	return true;
+}
+
+static void __skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
+{
+	size_t nr_lines = 0;
+	unsigned long entrypc_idx = 0;
+	Dwarf_Lines *lines = NULL;
+	Dwarf_Addr postprologue_addr;
+	Dwarf_Addr highpc;
+
+	if (dwarf_highpc(sp_die, &highpc))
+		return;
+
+	if (dwarf_getsrclines(&pf->cu_die, &lines, &nr_lines))
+		return;
+
+	if (!get_entrypc_idx(lines, nr_lines, pf->addr, &entrypc_idx))
+		return;
+
+	if (!get_postprologue_addr(entrypc_idx, lines, nr_lines,
+				   highpc, &postprologue_addr))
+		return;
+
+	pf->addr = postprologue_addr;
+}
+
+static void skip_prologue(Dwarf_Die *sp_die, struct probe_finder *pf)
+{
+	struct perf_probe_point *pp = &pf->pev->point;
+
+	/* Not uprobe? */
+	if (!pf->pev->uprobes)
+		return;
+
+	/* Compiled with optimization? */
+	if (optimized_target(&pf->cu_die))
+		return;
+
+	/* Don't know entrypc? */
+	if (!pf->addr)
+		return;
+
+	/* Only FUNC and FUNC@SRC are eligible. */
+	if (!pp->function || pp->line || pp->retprobe || pp->lazy_line ||
+	    pp->offset || pp->abs_address)
+		return;
+
+	/* Not interested in func parameter? */
+	if (!perf_probe_with_var(pf->pev))
+		return;
+
+	pr_info("Target program is compiled without optimization. Skipping prologue.\n"
+		"Probe on address 0x%" PRIx64 " to force probing at the function entry.\n\n",
+		pf->addr);
+
+	__skip_prologue(sp_die, pf);
+}
+
 static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
 {
 	struct probe_finder *pf = data;
@@ -969,6 +1133,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
 		if (pp->lazy_line)
 			param->retval = find_probe_point_lazy(sp_die, pf);
 		else {
+			skip_prologue(sp_die, pf);
 			pf->addr += pp->offset;
 			/* TODO: Check the address in this function */
 			param->retval = call_probe_finder(sp_die, pf);

Re: [PATCH 1/2] perf probe: Helper function to check if probe with variable

[Masami Hiramatsu]

On Wed,  3 Aug 2016 14:28:44 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> Introduce helper function instead of inline code and replace hardcoded
> strings "$vars" and "$params" with their corresponding macros.
> 
> perf_probe_with_var is not declared as static since it will be called
> from different file in subsequent patch.
> 

Looks good to me :)

> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> ---
>  tools/perf/util/probe-event.c | 22 +++++++++++++++-------
>  tools/perf/util/probe-event.h |  2 ++
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 953dc1a..bc9317e 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1592,19 +1592,27 @@ out:
>  	return ret;
>  }
>  
> +/* Returns true if *any* ARG is either C variable, $params or $vars. */
> +bool perf_probe_with_var(struct perf_probe_event *pev)
> +{
> +	int i = 0;
> +
> +	for (i = 0; i < pev->nargs; i++)
> +		if (is_c_varname(pev->args[i].var)              ||
> +		    !strcmp(pev->args[i].var, PROBE_ARG_PARAMS) ||
> +		    !strcmp(pev->args[i].var, PROBE_ARG_VARS))
> +			return true;
> +	return false;
> +}
> +
>  /* Return true if this perf_probe_event requires debuginfo */
>  bool perf_probe_event_need_dwarf(struct perf_probe_event *pev)
>  {
> -	int i;
> -
>  	if (pev->point.file || pev->point.line || pev->point.lazy_line)
>  		return true;
>  
> -	for (i = 0; i < pev->nargs; i++)
> -		if (is_c_varname(pev->args[i].var) ||
> -		    !strcmp(pev->args[i].var, "$params") ||
> -		    !strcmp(pev->args[i].var, "$vars"))
> -			return true;
> +	if (perf_probe_with_var(pev))
> +		return true;
>  
>  	return false;
>  }
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index e18ea9f..4d1139b 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -128,6 +128,8 @@ char *synthesize_perf_probe_point(struct perf_probe_point *pp);
>  int perf_probe_event__copy(struct perf_probe_event *dst,
>  			   struct perf_probe_event *src);
>  
> +bool perf_probe_with_var(struct perf_probe_event *pev);
> +
>  /* Check the perf_probe_event needs debuginfo */
>  bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
>  
> -- 
> 2.5.5
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[tip:perf/core] perf probe: Add helper function to check if probe with variable

[tip-bot for Ravi Bangoria]

Commit-ID:  b3f33f930606a55b547ac4ef5a32df2c23a44ec1
Gitweb:     http://git.kernel.org/tip/b3f33f930606a55b547ac4ef5a32df2c23a44ec1
Author:     Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
AuthorDate: Wed, 3 Aug 2016 14:28:44 +0530
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 1 Sep 2016 12:42:25 -0300

perf probe: Add helper function to check if probe with variable

Introduce helper function instead of inline code and replace hardcoded
strings "$vars" and "$params" with their corresponding macros.

perf_probe_with_var() is not declared as static since it will be called
from different file in subsequent patch.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1470214725-5023-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 22 +++++++++++++++-------
 tools/perf/util/probe-event.h |  2 ++
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8a1e9e6..a543e9c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1618,19 +1618,27 @@ out:
 	return ret;
 }
 
+/* Returns true if *any* ARG is either C variable, $params or $vars. */
+bool perf_probe_with_var(struct perf_probe_event *pev)
+{
+	int i = 0;
+
+	for (i = 0; i < pev->nargs; i++)
+		if (is_c_varname(pev->args[i].var)              ||
+		    !strcmp(pev->args[i].var, PROBE_ARG_PARAMS) ||
+		    !strcmp(pev->args[i].var, PROBE_ARG_VARS))
+			return true;
+	return false;
+}
+
 /* Return true if this perf_probe_event requires debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev)
 {
-	int i;
-
 	if (pev->point.file || pev->point.line || pev->point.lazy_line)
 		return true;
 
-	for (i = 0; i < pev->nargs; i++)
-		if (is_c_varname(pev->args[i].var) ||
-		    !strcmp(pev->args[i].var, "$params") ||
-		    !strcmp(pev->args[i].var, "$vars"))
-			return true;
+	if (perf_probe_with_var(pev))
+		return true;
 
 	return false;
 }
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 6209408..8091d15 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -128,6 +128,8 @@ char *synthesize_perf_probe_point(struct perf_probe_point *pp);
 int perf_probe_event__copy(struct perf_probe_event *dst,
 			   struct perf_probe_event *src);
 
+bool perf_probe_with_var(struct perf_probe_event *pev);
+
 /* Check the perf_probe_event needs debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);