Re: [PATCH -tip 4/5] [BUGFIX] perf probe: Fix the order of searchingscopes 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@xxxxxxxxxxx> 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@xxxxxxxxx>
>> Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
>> Cc: Paul Mackerras <paulus@xxxxxxxxx>
>> Cc: Ingo Molnar <mingo@xxxxxxx>
>> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxxxxxxxxxx>
>> ---
>>
>>  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@xxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/