From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga04-in.huawei.com (szxga04-in.huawei.com [45.249.212.190]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2CEE8208964; Tue, 12 Nov 2024 03:09:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=45.249.212.190 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731380970; cv=none; b=b0rCcfHTkUvIBFZC9ice7quzgRYS2ZhpBQWHf9BtYswG1MilnBJBpBwvutlvihxRiugIWwedaTaRgTW38Kg4V5TPCRp1gOXe6te6H3JzyIcM0JmxywfuJfBhec5kt4gFfnveYe5OzF8B5WwvmNxyVayTseDT3mIFKZYaAymJXXI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731380970; c=relaxed/simple; bh=Wc43L/Ps0VapgJQUE+gizw0rq0nrsbKRoVI9lMepUTY=; h=Subject:To:CC:References:From:Message-ID:Date:MIME-Version: In-Reply-To:Content-Type; b=hWOxMEesx9ps5JWp3efGKEBKKLO3qxw+E/7PNIBdzMZIYb+iA7RPWH4LB8nMaQgAeuutNYGLL9DYUvWL8D+QlUz2mwf9OTpS7cjA8rYVj4EMywugH8o+HgXRbXAD+bJvmIsmZkgYdrIbSuvEzWjVhanqJWL9TrGZXzUJcW6slIs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; arc=none smtp.client-ip=45.249.212.190 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Received: from mail.maildlp.com (unknown [172.19.88.214]) by szxga04-in.huawei.com (SkyGuard) with ESMTP id 4XnWZL3tZsz20t1m; Tue, 12 Nov 2024 11:08:10 +0800 (CST) Received: from kwepemf500004.china.huawei.com (unknown [7.202.181.242]) by mail.maildlp.com (Postfix) with ESMTPS id 9CB8F1A016C; Tue, 12 Nov 2024 11:09:23 +0800 (CST) Received: from [10.67.110.237] (10.67.110.237) by kwepemf500004.china.huawei.com (7.202.181.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Tue, 12 Nov 2024 11:09:22 +0800 Subject: Re: [PATCH 2/2] perf probe: Fix the incorrect line number display in 'perf probe -l' To: "Masami Hiramatsu (Google)" CC: , , , , , , , , , , , , , References: <20241108181909.3515716-1-lihuafei1@huawei.com> <20241108181909.3515716-2-lihuafei1@huawei.com> <20241111170549.e4d1ba7b65aee3d890889277@kernel.org> <20241111210559.055c990dc94e95666f6464a2@kernel.org> From: Li Huafei Message-ID: <4751058c-62e3-4b99-9568-283fdf52b055@huawei.com> Date: Tue, 12 Nov 2024 11:09:22 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: <20241111210559.055c990dc94e95666f6464a2@kernel.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemf500004.china.huawei.com (7.202.181.242) Hi Masami, On 2024/11/11 20:05, Masami Hiramatsu (Google) wrote: > Hi Li, > > On Mon, 11 Nov 2024 17:05:49 +0900 > Masami Hiramatsu (Google) wrote: > >> Currently debuginfo__find_probe_point() does >> >> (1) Get the line and file from CU's lineinfo >> (2) Get the real function(function instance) of the address >> (use this function's decl_line/decl_file as basement) >> (2-1) Search the inlined function scope in the real function >> for the given address. >> (2-2) if there is inlined function, update basement line/file. >> (2-3) verify the filename is same as basement filename. >> (3) calculate the relative line number from basement. >> >> The problem is in (1). Since we have no basement file/line info, >> we can not verify that the file/line info from CU's lineinfo. >> As Li shown above, the lineinfo may have several different lines >> for one address. We need to find most appropriate one based on >> the basement file/line. >> >> Thus what we need are >> >> - Introduce cu_find_lineinfo_at() which gives basement file/line >> information so that it can choose correct one. (hopefully) >> - Swap the order of (1) and (2*) so that we can pass the basement >> file/line when searching lineinfo. (Also, (2-3) should be right >> before (3)) >> > > Can you check below change fixes your issue? > Thank you for the detailed explanation in your previous email. I tested your patch, and the results are as follows: # perf probe -l probe:schedule (on schedule:5@kernel/sched/core.c) Is this not as expected? Should our expectation be: probe:schedule (on schedule:6780@kernel/sched/core.c) Thanks, Huafei > Thank you, > >>>From 3027c4d3c8be874a7c84a98e73fe0837fd135129 Mon Sep 17 00:00:00 2001 > From: "Masami Hiramatsu (Google)" > Date: Mon, 11 Nov 2024 20:56:00 +0900 > Subject: [PATCH] perf-probe: Fix --line option to show correct offset line > number from function > > Fix --line option to show correct offset if the DWARF line info of the > probe address has the statement lines in differnt functions. > > Signed-off-by: Masami Hiramatsu (Google) > --- > tools/perf/util/dwarf-aux.c | 78 ++++++++++++++++++++++++++++------ > tools/perf/util/dwarf-aux.h | 7 +++ > tools/perf/util/probe-finder.c | 44 +++++++++++++------ > 3 files changed, 105 insertions(+), 24 deletions(-) > > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c > index 92eb9c8dc3e5..0bffec6962b8 100644 > --- a/tools/perf/util/dwarf-aux.c > +++ b/tools/perf/util/dwarf-aux.c > @@ -60,14 +60,33 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die) > return dwarf_formstring(&attr); > } > > +static Dwarf_Line *get_next_statement_line(Dwarf_Lines *lines, size_t *idx, Dwarf_Addr addr) > +{ > + bool is_statement = false; > + Dwarf_Line *line; > + Dwarf_Addr laddr; > + size_t l = *idx; > + > + while (!is_statement) { > + line = dwarf_onesrcline(lines, ++l); > + if (!line || dwarf_lineaddr(line, &laddr) != 0 || > + dwarf_linebeginstatement(line, &is_statement) != 0) > + return NULL; > + if (laddr > addr) > + return NULL; > + } > + *idx = l; > + return line; > +} > + > /* Unlike dwarf_getsrc_die(), cu_getsrc_die() only returns statement line */ > -static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr) > +static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr, > + Dwarf_Lines **lines_p, size_t *idx) > { > Dwarf_Addr laddr; > Dwarf_Lines *lines; > Dwarf_Line *line; > size_t nlines, l, u, n; > - bool flag; > > if (dwarf_getsrclines(cu_die, &lines, &nlines) != 0 || > nlines == 0) > @@ -91,16 +110,14 @@ static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr) > if (!line || dwarf_lineaddr(line, &laddr) != 0) > return NULL; > } while (laddr == addr); > - l++; > /* Going forward to find the statement line */ > - do { > - line = dwarf_onesrcline(lines, l++); > - if (!line || dwarf_lineaddr(line, &laddr) != 0 || > - dwarf_linebeginstatement(line, &flag) != 0) > - return NULL; > - if (laddr > addr) > - return NULL; > - } while (!flag); > + line = get_next_statement_line(lines, &l, addr); > + if (!line) > + return NULL; > + if (lines_p) > + *lines_p = lines; > + if (idx) > + *idx = l; > > return line; > } > @@ -129,7 +146,7 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr, > goto out; > } > > - line = cu_getsrc_die(cu_die, addr); > + line = cu_getsrc_die(cu_die, addr, NULL, NULL); > if (line && dwarf_lineno(line, lineno) == 0) { > *fname = dwarf_linesrc(line, NULL, NULL); > if (!*fname) > @@ -141,6 +158,43 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr, > return (*lineno && *fname) ? *lineno : -ENOENT; > } > > +/** > + * cu_find_lineinfo_after - Get a line number after file:line for given address > + * @cu_die: a CU DIE > + * @addr: An address > + * @lineno: a pointer which returns the line number > + * @fname: the filename where searching the line > + * @baseline: the basement line number > + * > + * Find a line number after @baseline in @fname for @addr in @cu_die. > + * Return the found line number, or -ENOENT if not found. > + */ > +int cu_find_lineinfo_after(Dwarf_Die *cu_die, Dwarf_Addr addr, > + int *lineno, const char *fname, int baseline) > +{ > + const char *line_fname; > + Dwarf_Lines *lines; > + Dwarf_Line *line; > + size_t idx = 0; > + > + if (cu_find_lineinfo(cu_die, addr, &line_fname, lineno) < 0) > + return -ENOENT; > + > + if (!strcmp(line_fname, fname) && baseline <= *lineno) > + return *lineno; > + > + line = cu_getsrc_die(cu_die, addr, &lines, &idx); > + > + while (line && dwarf_lineno(line, lineno) == 0) { > + line_fname = dwarf_linesrc(line, NULL, NULL); > + if (line_fname && !strcmp(line_fname, fname) && baseline <= *lineno) > + return *lineno; > + line = get_next_statement_line(lines, &idx, addr); > + } > + > + return -ENOENT; > +} > + > static int __die_find_inline_cb(Dwarf_Die *die_mem, void *data); > > /** > diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h > index bd7505812569..19edf21e2f78 100644 > --- a/tools/perf/util/dwarf-aux.h > +++ b/tools/perf/util/dwarf-aux.h > @@ -23,6 +23,13 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die); > int cu_find_lineinfo(Dwarf_Die *cudie, Dwarf_Addr addr, > const char **fname, int *lineno); > > +/* > + * Get the most likely line number for given address in given filename > + * and basement line number. > + */ > +int cu_find_lineinfo_after(Dwarf_Die *cudie, Dwarf_Addr addr, > + int *lineno, const char *fname, int baseline); > + > /* Walk on functions at given address */ > int cu_walk_functions_at(Dwarf_Die *cu_die, Dwarf_Addr addr, > int (*callback)(Dwarf_Die *, void *), void *data); > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c > index 13ff45d3d6a4..efcacb5568e5 100644 > --- a/tools/perf/util/probe-finder.c > +++ b/tools/perf/util/probe-finder.c > @@ -1578,7 +1578,8 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr, > { > Dwarf_Die cudie, spdie, indie; > Dwarf_Addr _addr = 0, baseaddr = 0; > - const char *fname = NULL, *func = NULL, *basefunc = NULL, *tmp; > + const char *fname = NULL, *func = NULL, *basefunc = NULL; > + const char *basefname = NULL, *tmp; > int baseline = 0, lineno = 0, ret = 0; > > /* We always need to relocate the address for aranges */ > @@ -1592,11 +1593,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr, > goto end; > } > > - /* Find a corresponding line (filename and lineno) */ > - cu_find_lineinfo(&cudie, (Dwarf_Addr)addr, &fname, &lineno); > - /* Don't care whether it failed or not */ > - > - /* Find a corresponding function (name, baseline and baseaddr) */ > + /* Find the basement function (name, baseline and baseaddr) */ > if (die_find_realfunc(&cudie, (Dwarf_Addr)addr, &spdie)) { > /* Get function entry information */ > func = basefunc = dwarf_diename(&spdie); > @@ -1607,10 +1604,16 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr, > goto post; > } > > - fname = die_get_decl_file(&spdie); > + basefname = die_get_decl_file(&spdie); > + if (!basefname) { > + lineno = 0; > + goto post; > + } > + > if (addr == baseaddr) { > /* Function entry - Relative line number is 0 */ > lineno = baseline; > + fname = basefname; > goto post; > } > > @@ -1627,7 +1630,9 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr, > */ > lineno = die_get_call_lineno(&indie); > fname = die_get_call_file(&indie); > - break; > + if (!fname || strcmp(fname, basefname)) > + lineno = 0; > + goto post; > } else { > /* > * addr is in an inline function body. > @@ -1636,20 +1641,35 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr, > * be the entry line of the inline function. > */ > tmp = dwarf_diename(&indie); > - if (!tmp || > - dwarf_decl_line(&indie, &baseline) != 0) > - break; > + basefname = die_get_decl_file(&indie); > + if (!tmp || !basefname || > + dwarf_decl_line(&indie, &baseline) != 0) { > + lineno = 0; > + goto post; > + } > func = tmp; > spdie = indie; > } > } > + } > + > + if (!lineno) { > + /* Find a corresponding line (filename and lineno) */ > + if (cu_find_lineinfo_after(&cudie, (Dwarf_Addr)addr, &lineno, > + basefname, baseline) < 0) > + lineno = 0; > + else > + fname = basefname; > + } > + > +post: > + if (lineno) { > /* Verify the lineno and baseline are in a same file */ > tmp = die_get_decl_file(&spdie); > if (!tmp || (fname && strcmp(tmp, fname) != 0)) > lineno = 0; > } > > -post: > /* Make a relative line number or an offset */ > if (lineno) > ppt->line = lineno - baseline; >