[PATCH -tip 0/5]perf probe bugfixes

[PATCH -tip 0/5]perf probe bugfixes

[Masami Hiramatsu]

Hi Ingo,

Here are several bugfixes of "perf probe" which I've found
recently. I think these should go into perf/urgent branch
for safe.

Thank you,

---

Masami Hiramatsu (5):
      [BUGFIX] perf probe: Warn when more than one line are given
      [BUGFIX] perf probe: Fix the order of searching scopes for variables
      [BUGFIX] perf probe: Fix to walk all inline instances
      [BUGFIX] perf probe: Fix line walker to check CU correctly
      [BUGFIX] perf-probe: Fix a memory leak for scopes array


 tools/perf/builtin-probe.c     |   14 +++++++++++---
 tools/perf/util/probe-finder.c |   36 +++++++++++++++++++++---------------
 2 files changed, 32 insertions(+), 18 deletions(-)

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

[PATCH -tip 1/5] [BUGFIX] perf-probe: Fix a memory leak for scopes array

[Masami Hiramatsu]

Fix a memory leak for scopes array when it finds a
variable in the global scope.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
---

 tools/perf/util/probe-finder.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 3b9d0b8..1a35637 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1177,6 +1177,7 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
 	else {
 		/* Search upper class */
 		nscopes = dwarf_getscopes_die(sp_die, &scopes);
+		ret = -ENOENT;
 		while (nscopes-- > 1) {
 			pr_debug("Searching variables in %s\n",
 				 dwarf_diename(&scopes[nscopes]));
@@ -1185,14 +1186,12 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
 						 pf->pvar->var, 0,
 						 &vr_die)) {
 				ret = convert_variable(&vr_die, pf);
-				goto found;
+				break;
 			}
 		}
 		if (scopes)
 			free(scopes);
-		ret = -ENOENT;
 	}
-found:
 	if (ret < 0)
 		pr_warning("Failed to find '%s' in this function.\n",
 			   pf->pvar->var);

[PATCH -tip 2/5] [BUGFIX] perf probe: Fix line walker to check CU correctly

[Masami Hiramatsu]

Fix line walker to check whether a given DIE is CU or not.
Actually this function accepts CU, subprogram and
inlined_subroutine DIEs.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
---

 tools/perf/util/probe-finder.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 1a35637..52b87f3 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -599,8 +599,9 @@ static int __die_walk_culines_cb(Dwarf_Die *sp_die, void *data)
 }
 
 /*
- * Walk on lines inside given PDIE. If the PDIE is subprogram, walk only on
- * the lines inside the subprogram, otherwise PDIE must be a CU DIE.
+ * Walk on lines inside given PDIE. If the PDIE is subprogram or
+ * inlined_subprogram, walk only on the lines inside the DIE,
+ * otherwise PDIE must be a CU DIE.
  */
 static int die_walk_lines(Dwarf_Die *pdie, line_walk_handler_t handler,
 			  void *data)
@@ -614,12 +615,12 @@ static int die_walk_lines(Dwarf_Die *pdie, line_walk_handler_t handler,
 	size_t nlines, i;
 
 	/* Get the CU die */
-	if (dwarf_tag(pdie) == DW_TAG_subprogram)
+	if (dwarf_tag(pdie) != DW_TAG_compile_unit)
 		cu_die = dwarf_diecu(pdie, &die_mem, NULL, NULL);
 	else
 		cu_die = pdie;
 	if (!cu_die) {
-		pr_debug2("Failed to get CU from subprogram\n");
+		pr_debug2("Failed to get CU from given DIE\n");
 		return -EINVAL;
 	}
 

[PATCH -tip 3/5] [BUGFIX] perf probe: Fix to walk all inline instances

[Masami Hiramatsu]

Fix line-range collector to walk all instances of inlined
function, because some execution paths can be optimized out
depends on the function argument of instances.

E.g.)
inline_func (arg) {
	if (arg)
		do_something;
	else
		do_another;
}

func_A() {
	inline_func(1)
}

func_B() {
	inline_func(0)
}

In this case, func_A may have only do_something code and
func_B may have only do_another.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
---

 tools/perf/util/probe-finder.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 52b87f3..222ba66 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1932,7 +1932,13 @@ static int line_range_inline_cb(Dwarf_Die *in_die, void *data)
 	struct dwarf_callback_param *param = data;
 
 	param->retval = find_line_range_by_line(in_die, param->data);
-	return DWARF_CB_ABORT;	/* No need to find other instances */
+
+	/*
+	 * We have to check all instances of inlined function, because
+	 * some execution paths can be optimized out depends on the
+	 * function argument of instances
+	 */
+	return DWARF_CB_OK;
 }
 
 /* Search function from function name */

[PATCH -tip 4/5] [BUGFIX] perf probe: Fix the order of searching scopes for variables

[Masami Hiramatsu]

Fix variable searching routine to search from inner scope
to outer scope.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
---

 tools/perf/util/probe-finder.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 222ba66..cf7d48d 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1135,7 +1135,7 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
 {
 	Dwarf_Die vr_die, *scopes;
 	char buf[32], *ptr;
-	int ret, nscopes;
+	int ret, nscopes, i;
 
 	if (!is_c_varname(pf->pvar->var)) {
 		/* Copy raw parameters */
@@ -1179,11 +1179,11 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
 		/* Search upper class */
 		nscopes = dwarf_getscopes_die(sp_die, &scopes);
 		ret = -ENOENT;
-		while (nscopes-- > 1) {
+		for (i = 1; i < nscopes; i++) {	/* scopes[0] is sp_die */
 			pr_debug("Searching variables in %s\n",
-				 dwarf_diename(&scopes[nscopes]));
+				 dwarf_diename(&scopes[i]));
 			/* We should check this scope, so give dummy address */
-			if (die_find_variable_at(&scopes[nscopes],
+			if (die_find_variable_at(&scopes[i],
 						 pf->pvar->var, 0,
 						 &vr_die)) {
 				ret = convert_variable(&vr_die, pf);
@@ -1693,7 +1693,7 @@ static int add_available_vars(Dwarf_Die *sp_die, struct probe_finder *pf)
 			container_of(pf, struct available_var_finder, pf);
 	struct variable_list *vl;
 	Dwarf_Die die_mem, *scopes = NULL;
-	int ret, nscopes;
+	int ret, nscopes, i;
 
 	/* Check number of tevs */
 	if (af->nvls == af->max_vls) {
@@ -1723,8 +1723,8 @@ static int add_available_vars(Dwarf_Die *sp_die, struct probe_finder *pf)
 	/* Don't need to search child DIE for externs. */
 	af->child = false;
 	nscopes = dwarf_getscopes_die(sp_die, &scopes);
-	while (nscopes-- > 1)
-		die_find_child(&scopes[nscopes], collect_variables_cb,
+	for (i = 1; i < nscopes; i++)	/* scopes[0] is sp_die */
+		die_find_child(&scopes[i], collect_variables_cb,
 			       (void *)af, &die_mem);
 	if (scopes)
 		free(scopes);

[PATCH -tip 5/5] [BUGFIX] perf probe: Warn when more than one line are given

[Masami Hiramatsu]

Check multiple --lines option and print warning which
warns only the first specified --line option is valid.

Changes from previous version:
- Accept only the first option instead of the last.
- Fix warning message according to David's comment.
- Mark as a bugfix.


Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: David Ahern <dsahern@gmail.com>
---

 tools/perf/builtin-probe.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 2c0e64d..8cb0ac8 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -134,10 +134,18 @@ static int opt_show_lines(const struct option *opt __used,
 {
 	int ret = 0;
 
-	if (str)
-		ret = parse_line_range_desc(str, &params.line_range);
-	INIT_LIST_HEAD(&params.line_range.line_list);
+	if (!str)
+		return 0;
+
+	if (params.show_lines) {
+		pr_warning("Warning: more than one --line options are"
+			   " detected. Only the first one is valid.\n");
+		return 0;
+	}
+
 	params.show_lines = true;
+	ret = parse_line_range_desc(str, &params.line_range);
+	INIT_LIST_HEAD(&params.line_range.line_list);
 
 	return ret;
 }

Re: [PATCH -tip 1/5] [BUGFIX] perf-probe: Fix a memory leak for scopes array

[Pekka Enberg]

On Sun, Jul 10, 2011 at 1:00 PM, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
> Fix a memory leak for scopes array when it finds a
> variable in the global scope.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>

Reviewed-by: Pekka Enberg <penberg@kernel.org>

Re: [PATCH -tip 4/5] [BUGFIX] perf probe: Fix the order of searching scopes for variables

[Pekka Enberg]

On Sun, Jul 10, 2011 at 1:00 PM, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
> Fix variable searching routine to search from inner scope
> to outer scope.

Admittedly, I don't really know this code at all. However, the
changelog is somewhat vague. Why do we need this change? What problem
does it fix? Why does the order in which we iterate 'scopes' matter?

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@gmail.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> ---
>
>  tools/perf/util/probe-finder.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 222ba66..cf7d48d 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1135,7 +1135,7 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
>  {
>        Dwarf_Die vr_die, *scopes;
>        char buf[32], *ptr;
> -       int ret, nscopes;
> +       int ret, nscopes, i;
>
>        if (!is_c_varname(pf->pvar->var)) {
>                /* Copy raw parameters */
> @@ -1179,11 +1179,11 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
>                /* Search upper class */
>                nscopes = dwarf_getscopes_die(sp_die, &scopes);
>                ret = -ENOENT;
> -               while (nscopes-- > 1) {
> +               for (i = 1; i < nscopes; i++) { /* scopes[0] is sp_die */
>                        pr_debug("Searching variables in %s\n",
> -                                dwarf_diename(&scopes[nscopes]));
> +                                dwarf_diename(&scopes[i]));
>                        /* We should check this scope, so give dummy address */
> -                       if (die_find_variable_at(&scopes[nscopes],
> +                       if (die_find_variable_at(&scopes[i],
>                                                 pf->pvar->var, 0,
>                                                 &vr_die)) {
>                                ret = convert_variable(&vr_die, pf);
> @@ -1693,7 +1693,7 @@ static int add_available_vars(Dwarf_Die *sp_die, struct probe_finder *pf)
>                        container_of(pf, struct available_var_finder, pf);
>        struct variable_list *vl;
>        Dwarf_Die die_mem, *scopes = NULL;
> -       int ret, nscopes;
> +       int ret, nscopes, i;
>
>        /* Check number of tevs */
>        if (af->nvls == af->max_vls) {
> @@ -1723,8 +1723,8 @@ static int add_available_vars(Dwarf_Die *sp_die, struct probe_finder *pf)
>        /* Don't need to search child DIE for externs. */
>        af->child = false;
>        nscopes = dwarf_getscopes_die(sp_die, &scopes);
> -       while (nscopes-- > 1)
> -               die_find_child(&scopes[nscopes], collect_variables_cb,
> +       for (i = 1; i < nscopes; i++)   /* scopes[0] is sp_die */
> +               die_find_child(&scopes[i], collect_variables_cb,
>                               (void *)af, &die_mem);
>        if (scopes)
>                free(scopes);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH -tip 4/5] [BUGFIX] perf probe: Fix the order of searching scopes for variables

[Masami Hiramatsu]

Hi Pekka,

(2011/07/10 20:05), Pekka Enberg wrote:
> On Sun, Jul 10, 2011 at 1:00 PM, Masami Hiramatsu
> <masami.hiramatsu.pt@hitachi.com> wrote:
>> Fix variable searching routine to search from inner scope
>> to outer scope.
> 
> Admittedly, I don't really know this code at all. However, the
> changelog is somewhat vague. Why do we need this change? What problem
> does it fix? Why does the order in which we iterate 'scopes' matter?

Hmm, I see.

Debuginfo expresses functions and nested inlined functions as
DIE(Dwarf Information Entry) tree, and "scope" means each
level of the DIE.

Outer
^
|
  CU DIE [A](compile unit: it's an object file)
   +-global variable DIEs
   +-inline function declaration DIEs
   +-function [B] DIE
      +-local variable DIEs
      +-inline function instance [C] DIE
         +-inline local variable DIEs
         +-(nested)inline function instance [D] DIE
            +...
|
v
Inner

Imagine that if we are at [C],

nscopes = dwarf_getscopes_die([C], &scopes)

this returns scopes[] DIEs containing [C] DIE, and nscopes = 3
On that array, scopes[0] is [C], scopes[1] is [B], and scopes[2]
is [A].

And, the below loop

 while (nscopes-- > 1)
	die_find_variable_at(&scopes[nscopes], ...

starts to find variables from the outermost scope [A] and
find a global variable first.　This changes searching order
from inner scope ([B]) to outer ([A]) to search global vars
last.


Thank you!


> 
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@gmail.com>
>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
>> ---
>>
>>  tools/perf/util/probe-finder.c |   14 +++++++-------
>>  1 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
>> index 222ba66..cf7d48d 100644
>> --- a/tools/perf/util/probe-finder.c
>> +++ b/tools/perf/util/probe-finder.c
>> @@ -1135,7 +1135,7 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
>>  {
>>        Dwarf_Die vr_die, *scopes;
>>        char buf[32], *ptr;
>> -       int ret, nscopes;
>> +       int ret, nscopes, i;
>>
>>        if (!is_c_varname(pf->pvar->var)) {
>>                /* Copy raw parameters */
>> @@ -1179,11 +1179,11 @@ static int find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
>>                /* Search upper class */
>>                nscopes = dwarf_getscopes_die(sp_die, &scopes);
>>                ret = -ENOENT;
>> -               while (nscopes-- > 1) {
>> +               for (i = 1; i < nscopes; i++) { /* scopes[0] is sp_die */
>>                        pr_debug("Searching variables in %s\n",
>> -                                dwarf_diename(&scopes[nscopes]));
>> +                                dwarf_diename(&scopes[i]));
>>                        /* We should check this scope, so give dummy address */
>> -                       if (die_find_variable_at(&scopes[nscopes],
>> +                       if (die_find_variable_at(&scopes[i],
>>                                                 pf->pvar->var, 0,
>>                                                 &vr_die)) {
>>                                ret = convert_variable(&vr_die, pf);
>> @@ -1693,7 +1693,7 @@ static int add_available_vars(Dwarf_Die *sp_die, struct probe_finder *pf)
>>                        container_of(pf, struct available_var_finder, pf);
>>        struct variable_list *vl;
>>        Dwarf_Die die_mem, *scopes = NULL;
>> -       int ret, nscopes;
>> +       int ret, nscopes, i;
>>
>>        /* Check number of tevs */
>>        if (af->nvls == af->max_vls) {
>> @@ -1723,8 +1723,8 @@ static int add_available_vars(Dwarf_Die *sp_die, struct probe_finder *pf)
>>        /* Don't need to search child DIE for externs. */
>>        af->child = false;
>>        nscopes = dwarf_getscopes_die(sp_die, &scopes);
>> -       while (nscopes-- > 1)
>> -               die_find_child(&scopes[nscopes], collect_variables_cb,
>> +       for (i = 1; i < nscopes; i++)   /* scopes[0] is sp_die */
>> +               die_find_child(&scopes[i], collect_variables_cb,
>>                               (void *)af, &die_mem);
>>        if (scopes)
>>                free(scopes);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 


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

Re: [PATCH -tip 4/5] [BUGFIX] perf probe: Fix the order of searching scopes for variables

[Pekka Enberg]

On Sun, Jul 10, 2011 at 3:08 PM, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
> Hi Pekka,
>
> (2011/07/10 20:05), Pekka Enberg wrote:
>> On Sun, Jul 10, 2011 at 1:00 PM, Masami Hiramatsu
>> <masami.hiramatsu.pt@hitachi.com> wrote:
>>> Fix variable searching routine to search from inner scope
>>> to outer scope.
>>
>> Admittedly, I don't really know this code at all. However, the
>> changelog is somewhat vague. Why do we need this change? What problem
>> does it fix? Why does the order in which we iterate 'scopes' matter?
>
> Hmm, I see.
>
> Debuginfo expresses functions and nested inlined functions as
> DIE(Dwarf Information Entry) tree, and "scope" means each
> level of the DIE.
>
> Outer
> ^
> |
>  CU DIE [A](compile unit: it's an object file)
>   +-global variable DIEs
>   +-inline function declaration DIEs
>   +-function [B] DIE
>      +-local variable DIEs
>      +-inline function instance [C] DIE
>         +-inline local variable DIEs
>         +-(nested)inline function instance [D] DIE
>            +...
> |
> v
> Inner
>
> Imagine that if we are at [C],
>
> nscopes = dwarf_getscopes_die([C], &scopes)
>
> this returns scopes[] DIEs containing [C] DIE, and nscopes = 3
> On that array, scopes[0] is [C], scopes[1] is [B], and scopes[2]
> is [A].
>
> And, the below loop
>
>  while (nscopes-- > 1)
>        die_find_variable_at(&scopes[nscopes], ...
>
> starts to find variables from the outermost scope [A] and
> find a global variable first.　This changes searching order
> from inner scope ([B]) to outer ([A]) to search global vars
> last.

Thanks for the explanation! What kind of problems does this cause for
'perf probe' users?

Re: [PATCH -tip 4/5] [BUGFIX] perf probe: Fix the order of searching scopes for variables

[Masami Hiramatsu]

2011/7/11 Pekka Enberg <penberg@kernel.org>:
> On Sun, Jul 10, 2011 at 3:08 PM, Masami Hiramatsu
> <masami.hiramatsu.pt@hitachi.com> wrote:
>> Hi Pekka,
>>
>> (2011/07/10 20:05), Pekka Enberg wrote:
>>> On Sun, Jul 10, 2011 at 1:00 PM, Masami Hiramatsu
>>> <masami.hiramatsu.pt@hitachi.com> wrote:
>>>> Fix variable searching routine to search from inner scope
>>>> to outer scope.
>>>
>>> Admittedly, I don't really know this code at all. However, the
>>> changelog is somewhat vague. Why do we need this change? What problem
>>> does it fix? Why does the order in which we iterate 'scopes' matter?
>>
>> Hmm, I see.
>>
>> Debuginfo expresses functions and nested inlined functions as
>> DIE(Dwarf Information Entry) tree, and "scope" means each
>> level of the DIE.
>>
>> Outer
>> ^
>> |
>>  CU DIE [A](compile unit: it's an object file)
>>   +-global variable DIEs
>>   +-inline function declaration DIEs
>>   +-function [B] DIE
>>      +-local variable DIEs
>>      +-inline function instance [C] DIE
>>         +-inline local variable DIEs
>>         +-(nested)inline function instance [D] DIE
>>            +...
>> |
>> v
>> Inner
>>
>> Imagine that if we are at [C],
>>
>> nscopes = dwarf_getscopes_die([C], &scopes)
>>
>> this returns scopes[] DIEs containing [C] DIE, and nscopes = 3
>> On that array, scopes[0] is [C], scopes[1] is [B], and scopes[2]
>> is [A].
>>
>> And, the below loop
>>
>>  while (nscopes-- > 1)
>>        die_find_variable_at(&scopes[nscopes], ...
>>
>> starts to find variables from the outermost scope [A] and
>> find a global variable first.　This changes searching order
>> from inner scope ([B]) to outer ([A]) to search global vars
>> last.
>
> Thanks for the explanation! What kind of problems does this cause for
> 'perf probe' users?
>

Hmm, wait, I must say "drop this! Ingo" since I've found that
both of original routine and this fix will not fix a problem.

OK, here is the problem I have - if there are some same-name variables
in different scopes, which one should be referred first? for example,
imagine the below situation;

int var;

void inline infunc(int i)
{
    i++;   <----here, a user put a probe with 'var' argument.
}

void func(void)
{
   int var = 10;
   infunc(var);
}


This patch searches in the  inline-local scope (infunc), after that, it searches
from inner scope (func) to outer scope (global). In this case, it picks up the
function local variable if it found, before searching global
variables. This is same
logic as libdw does. But this looks insane in C manner. So, I'd like
to drop this.

On the other hand, original one searches in the inline-local scope
(infunc), after
that, searches in the outermost (global) scope, and then inner scope (func).
This will pick the global variable first if it failed to find in
inline-local scopes.
This seems sane.

... But wait, if there is no global 'var', what happened? In that
case, it will also
return the 'var' in 'func' scope. This also might not be good. User will expect
"there is no var" error in that case, but perf-probe will have no error and
define an event with unexpected variable as an argument.

So I think the correct fix is not to use several scopes, but search var only
in the outermost scope. I'm not so sure about scopes, there might be
several other scopes (not only functions, maybe namespace or something?)
I've just tried to mimic the libdw logic in this patch.

Anyway, thanks Pekka! for giving me a chance to find it. ;-)

Thank you again,

-- 
Masami Hiramatsu
mailto:masami.hiramatsu@gmail.com