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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADyE4xEeam__EzZ1a6+ceR40ukXcuPBVGVT0wDy0Nt+5Mi+x0A@mail.gmail.com>
Date:	Mon, 11 Jul 2011 02:22:44 +0900
From:	Masami Hiramatsu <masami.hiramatsu@...il.com>
To:	Pekka Enberg <penberg@...nel.org>
Cc:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	linux-kernel@...r.kernel.org, yrl.pp-manager.tt@...achi.com,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
Subject: Re: [PATCH -tip 4/5] [BUGFIX] perf probe: Fix the order of searching
 scopes for variables

2011/7/11 Pekka Enberg <penberg@...nel.org>:
> On Sun, Jul 10, 2011 at 3:08 PM, Masami Hiramatsu
> <masami.hiramatsu.pt@...achi.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@...achi.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@...il.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/