Received-SPF: pass (google.com: domain of linux-kernel+bounces-132284-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <3485d1b1-7824-4e62-82b4-ed52360122dd@gmail.com>
Date: Thu, 4 Apr 2024 21:09:51 -0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] perf srcline: Implement addr2line using libdw
To: Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
 Arnaldo Carvalho de Melo <acme@kernel.org>,
 Mark Rutland <mark.rutland@arm.com>,
 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
 Jiri Olsa <jolsa@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>,
 linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240401160805.51277-1-yakoyoku@gmail.com>
 <CAP-5=fXu4uM=cU8Q=1JP19sELfarNE9BtBmbFW0Uyq2e_HJ6QA@mail.gmail.com>
 <CAM9d7cjgfQpo6dgV4f5Wm4shVpknzP4_m36AkTwaUaF4jLrV1w@mail.gmail.com>
Content-Language: en-US
From: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
In-Reply-To: <CAM9d7cjgfQpo6dgV4f5Wm4shVpknzP4_m36AkTwaUaF4jLrV1w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 4/1/24 8:31 PM, Namhyung Kim wrote:
> [...]
> 
> Thanks for doing this!  Yep, we could unset the env temporarily.
> 
> As a general comment, perf has some helper functions for libdw
> in util/dwarf-aux.[ch].  Please take a look and use/update them.

Whoops! I haven't seen that file, thanks for mentioning it.

> [...]
> 
> Probably we can use cu_find_lineinfo().

I could use that, I'll see.

> [...]
> 
>>> +static struct a2l_data *addr2line_init(const char *path)
> 
> debuginfo__new()?

`libdw_a2l_new` can be another option too.

> [...]
> 
>>> +static int get_inline_function(struct dso *dso, struct inline_node *node,
>>> +                              struct symbol *sym)
>>> +{
>>> +       struct a2l_data *a2l = dso->a2l;
>>> +       Dwarf_Addr addr = a2l->addr + a2l->bias;
>>> +       Dwarf_Addr bias = 0;
>>> +       Dwarf_Die *cudie = dwfl_module_addrdie(a2l->mod, addr, &bias);
>>> +
>>> +       Dwarf_Die *scopes = NULL;
>>> +       int nscopes = dwarf_getscopes(cudie, addr - bias, &scopes);
> 
> It's not clear to me how this dwarf_getscopes() and later
> dwarf_getscopes_die() work together.  Can you please add some
> comment?  Also we have die_get_scopes() and I think it's simpler.

I think the first is to get the scopes at an address and the second is
for the scopes with the addr offset at that DIE. Off the top of my head.

> [...]
> 
> Why not get the function name from the abstract origin?

I'm not getting it, where's that abstract origin?

> [...]