* [PATCH 1/3] tools/perf: Fix to avoid crashing with a broken DWARF file
2022-11-01 13:14 [PATCH 0/3] tools/perf: Fix perf probe crash by broken DWARF file Masami Hiramatsu (Google)
@ 2022-11-01 13:14 ` Masami Hiramatsu (Google)
2022-11-01 13:14 ` [PATCH 2/3] tools/perf: Fix to use dwarf_attr_integrate for generic attr accessor Masami Hiramatsu (Google)
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2022-11-01 13:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel,
Masami Hiramatsu, Steven Rostedt
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since Clang generates a wrong DWARF5 format, dwarf_decl_file() can
return NULL. In that case the perf probe will crash by SIGSEGV.
This adds checks of the return value of dwarf_decl_file() to avoid
such SEGV on a broken DWARF file.
Without this, perf probe crashes like below;
$ ./perf probe -k $BIN_PATH/vmlinux -s $SRC_PATH -L vfs_read:10
Segmentation fault
With this, perf probe just warns it;
$ ./perf probe -k $BIN_PATH/vmlinux -s $SRC_PATH -L vfs_read:10
Debuginfo analysis failed.
Error: Failed to show lines.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
tools/perf/util/dwarf-aux.c | 7 ++++++-
tools/perf/util/probe-finder.c | 29 +++++++++++++++++++++--------
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 609ca1671501..406b7bdc851a 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -137,7 +137,7 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr,
}
out:
- return *lineno ?: -ENOENT;
+ return (*lineno && *fname) ? *lineno : -ENOENT;
}
static int __die_find_inline_cb(Dwarf_Die *die_mem, void *data);
@@ -874,6 +874,11 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
cu_die = dwarf_diecu(rt_die, &die_mem, NULL, NULL);
dwarf_decl_line(rt_die, &decl);
decf = dwarf_decl_file(rt_die);
+ if (!decf) {
+ pr_debug2("Failed to get the declared file name of %s\n",
+ dwarf_diename(rt_die));
+ return -EINVAL;
+ }
} else
cu_die = rt_die;
if (!cu_die) {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 50d861a80f57..1aa8fcc41c76 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1063,6 +1063,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
struct dwarf_callback_param *param = data;
struct probe_finder *pf = param->data;
struct perf_probe_point *pp = &pf->pev->point;
+ const char *fname;
/* Check tag and diename */
if (!die_is_func_def(sp_die) ||
@@ -1070,12 +1071,17 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
return DWARF_CB_OK;
/* Check declared file */
- if (pp->file && strtailcmp(pp->file, dwarf_decl_file(sp_die)))
+ fname = dwarf_decl_file(sp_die);
+ if (!fname) {
+ pr_warning("A function DIE doesn't have decl_line. Maybe broken DWARF?\n");
+ return DWARF_CB_OK;
+ }
+ if (pp->file && fname && strtailcmp(pp->file, fname))
return DWARF_CB_OK;
pr_debug("Matched function: %s [%lx]\n", dwarf_diename(sp_die),
(unsigned long)dwarf_dieoffset(sp_die));
- pf->fname = dwarf_decl_file(sp_die);
+ pf->fname = fname;
if (pp->line) { /* Function relative line */
dwarf_decl_line(sp_die, &pf->lno);
pf->lno += pp->line;
@@ -1134,6 +1140,7 @@ struct pubname_callback_param {
static int pubname_search_cb(Dwarf *dbg, Dwarf_Global *gl, void *data)
{
struct pubname_callback_param *param = data;
+ const char *fname;
if (dwarf_offdie(dbg, gl->die_offset, param->sp_die)) {
if (dwarf_tag(param->sp_die) != DW_TAG_subprogram)
@@ -1143,9 +1150,11 @@ static int pubname_search_cb(Dwarf *dbg, Dwarf_Global *gl, void *data)
if (!dwarf_offdie(dbg, gl->cu_offset, param->cu_die))
return DWARF_CB_OK;
- if (param->file &&
- strtailcmp(param->file, dwarf_decl_file(param->sp_die)))
- return DWARF_CB_OK;
+ if (param->file) {
+ fname = dwarf_decl_file(param->sp_die);
+ if (!fname || strtailcmp(param->file, fname))
+ return DWARF_CB_OK;
+ }
param->found = 1;
return DWARF_CB_ABORT;
@@ -1779,7 +1788,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
}
/* Verify the lineno and baseline are in a same file */
tmp = dwarf_decl_file(&spdie);
- if (!tmp || strcmp(tmp, fname) != 0)
+ if (!tmp || (fname && strcmp(tmp, fname) != 0))
lineno = 0;
}
@@ -1889,10 +1898,14 @@ static int line_range_search_cb(Dwarf_Die *sp_die, void *data)
struct dwarf_callback_param *param = data;
struct line_finder *lf = param->data;
struct line_range *lr = lf->lr;
+ const char *fname;
/* Check declared file */
- if (lr->file && strtailcmp(lr->file, dwarf_decl_file(sp_die)))
- return DWARF_CB_OK;
+ if (lr->file) {
+ fname = dwarf_decl_file(sp_die);
+ if (!fname || strtailcmp(lr->file, fname))
+ return DWARF_CB_OK;
+ }
if (die_match_name(sp_die, lr->function) && die_is_func_def(sp_die)) {
lf->fname = dwarf_decl_file(sp_die);
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] tools/perf: Fix to use dwarf_attr_integrate for generic attr accessor
2022-11-01 13:14 [PATCH 0/3] tools/perf: Fix perf probe crash by broken DWARF file Masami Hiramatsu (Google)
2022-11-01 13:14 ` [PATCH 1/3] tools/perf: Fix to avoid crashing with a " Masami Hiramatsu (Google)
@ 2022-11-01 13:14 ` Masami Hiramatsu (Google)
2022-11-01 13:14 ` [PATCH 3/3] tools/perf: Fix to get declared file name from broken DWARF5 Masami Hiramatsu (Google)
2022-11-01 13:29 ` [PATCH 0/3] tools/perf: Fix perf probe crash by broken DWARF file Masami Hiramatsu
3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2022-11-01 13:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel,
Masami Hiramatsu, Steven Rostedt
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Use dwarf_attr_integrate() instead of dwarf_attr() for generic attribute
acccessor functions, so that it can find the specified attribute from
abstact origin DIE etc.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
tools/perf/util/dwarf-aux.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 406b7bdc851a..216fc3d959e8 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -308,7 +308,7 @@ static int die_get_attr_udata(Dwarf_Die *tp_die, unsigned int attr_name,
{
Dwarf_Attribute attr;
- if (dwarf_attr(tp_die, attr_name, &attr) == NULL ||
+ if (dwarf_attr_integrate(tp_die, attr_name, &attr) == NULL ||
dwarf_formudata(&attr, result) != 0)
return -ENOENT;
@@ -321,7 +321,7 @@ static int die_get_attr_sdata(Dwarf_Die *tp_die, unsigned int attr_name,
{
Dwarf_Attribute attr;
- if (dwarf_attr(tp_die, attr_name, &attr) == NULL ||
+ if (dwarf_attr_integrate(tp_die, attr_name, &attr) == NULL ||
dwarf_formsdata(&attr, result) != 0)
return -ENOENT;
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] tools/perf: Fix to get declared file name from broken DWARF5
2022-11-01 13:14 [PATCH 0/3] tools/perf: Fix perf probe crash by broken DWARF file Masami Hiramatsu (Google)
2022-11-01 13:14 ` [PATCH 1/3] tools/perf: Fix to avoid crashing with a " Masami Hiramatsu (Google)
2022-11-01 13:14 ` [PATCH 2/3] tools/perf: Fix to use dwarf_attr_integrate for generic attr accessor Masami Hiramatsu (Google)
@ 2022-11-01 13:14 ` Masami Hiramatsu (Google)
2022-11-01 13:29 ` [PATCH 0/3] tools/perf: Fix perf probe crash by broken DWARF file Masami Hiramatsu
3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu (Google) @ 2022-11-01 13:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, linux-perf-users, linux-kernel,
Masami Hiramatsu, Steven Rostedt
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Fix to get the declared file name even if it uses index 0 in DWARF5
using custom die_get_decl_file() function.
Actually, this is out of standard(*), but clang generates such
DWARF5 file for the linux kernel. Thus it must be handled.
Without this, the perf probe returns an error;
$ ./perf probe -k $BIN_PATH/vmlinux -s $SRC_PATH -L vfs_read:10
Debuginfo analysis failed.
Error: Failed to show lines.
With this, it can handle the case correctly;
$ ./perf probe -k $BIN_PATH/vmlinux -s $SRC_PATH -L vfs_read:10
<vfs_read@$SRC_PATH/fs/read_write.c:10>
11 ret = rw_verify_area(READ, file, pos, count);
12 if (ret)
return ret;
(* DWARF5 specification 2.14 says "The value 0 indicates that no
source file has been specified.")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
tools/perf/util/dwarf-aux.c | 47 +++++++++++++++++++++++++++-------------
tools/perf/util/dwarf-aux.h | 3 +++
tools/perf/util/probe-finder.c | 14 ++++++------
3 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 216fc3d959e8..30b36b525681 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -123,7 +123,7 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr,
if (die_find_realfunc(cu_die, addr, &die_mem)
&& die_entrypc(&die_mem, &faddr) == 0 &&
faddr == addr) {
- *fname = dwarf_decl_file(&die_mem);
+ *fname = die_get_decl_file(&die_mem);
dwarf_decl_line(&die_mem, lineno);
goto out;
}
@@ -486,6 +486,19 @@ static int die_get_decl_fileno(Dwarf_Die *pdie)
return -ENOENT;
}
+/* Return the file name by index */
+static const char *die_get_file_name(Dwarf_Die *dw_die, int idx)
+{
+ Dwarf_Die cu_die;
+ Dwarf_Files *files;
+
+ if (idx < 0 || !dwarf_diecu(dw_die, &cu_die, NULL, NULL) ||
+ dwarf_getsrcfiles(&cu_die, &files, NULL) != 0)
+ return NULL;
+
+ return dwarf_filesrc(files, idx, NULL, NULL);
+}
+
/**
* die_get_call_file - Get callsite file name of inlined function instance
* @in_die: a DIE of an inlined function instance
@@ -495,18 +508,22 @@ static int die_get_decl_fileno(Dwarf_Die *pdie)
*/
const char *die_get_call_file(Dwarf_Die *in_die)
{
- Dwarf_Die cu_die;
- Dwarf_Files *files;
- int idx;
-
- idx = die_get_call_fileno(in_die);
- if (idx < 0 || !dwarf_diecu(in_die, &cu_die, NULL, NULL) ||
- dwarf_getsrcfiles(&cu_die, &files, NULL) != 0)
- return NULL;
-
- return dwarf_filesrc(files, idx, NULL, NULL);
+ return die_get_file_name(in_die, die_get_call_fileno(in_die));
}
+/**
+ * die_get_decl_file - Find the declared file name of this DIE
+ * @dw_die: a DIE for something declared.
+ *
+ * Get declared file name of @dw_die.
+ * NOTE: Since some version of clang DWARF5 implementation incorrectly uses
+ * file index 0 for DW_AT_decl_file, die_get_decl_file() will return NULL for
+ * such cases. Use this function instead.
+ */
+const char *die_get_decl_file(Dwarf_Die *dw_die)
+{
+ return die_get_file_name(dw_die, die_get_decl_fileno(dw_die));
+}
/**
* die_find_child - Generic DIE search function in DIE tree
@@ -790,7 +807,7 @@ static int __die_walk_funclines_cb(Dwarf_Die *in_die, void *data)
}
if (addr) {
- fname = dwarf_decl_file(in_die);
+ fname = die_get_decl_file(in_die);
if (fname && dwarf_decl_line(in_die, &lineno) == 0) {
lw->retval = lw->callback(fname, lineno, addr, lw->data);
if (lw->retval != 0)
@@ -818,7 +835,7 @@ static int __die_walk_funclines(Dwarf_Die *sp_die, bool recursive,
int lineno;
/* Handle function declaration line */
- fname = dwarf_decl_file(sp_die);
+ fname = die_get_decl_file(sp_die);
if (fname && dwarf_decl_line(sp_die, &lineno) == 0 &&
die_entrypc(sp_die, &addr) == 0) {
lw.retval = callback(fname, lineno, addr, data);
@@ -873,7 +890,7 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
if (dwarf_tag(rt_die) != DW_TAG_compile_unit) {
cu_die = dwarf_diecu(rt_die, &die_mem, NULL, NULL);
dwarf_decl_line(rt_die, &decl);
- decf = dwarf_decl_file(rt_die);
+ decf = die_get_decl_file(rt_die);
if (!decf) {
pr_debug2("Failed to get the declared file name of %s\n",
dwarf_diename(rt_die));
@@ -928,7 +945,7 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
dwarf_decl_line(&die_mem, &inl);
if (inl != decl ||
- decf != dwarf_decl_file(&die_mem))
+ decf != die_get_decl_file(&die_mem))
continue;
}
}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 7ee0fa19b5c4..7ec8bc1083bb 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -50,6 +50,9 @@ int die_get_call_lineno(Dwarf_Die *in_die);
/* Get callsite file name of inlined function instance */
const char *die_get_call_file(Dwarf_Die *in_die);
+/* Get declared file name of a DIE */
+const char *die_get_decl_file(Dwarf_Die *dw_die);
+
/* Get type die */
Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem);
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 1aa8fcc41c76..54b49ce85c9f 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -763,7 +763,7 @@ static int find_best_scope_cb(Dwarf_Die *fn_die, void *data)
/* Skip if declared file name does not match */
if (fsp->file) {
- file = dwarf_decl_file(fn_die);
+ file = die_get_decl_file(fn_die);
if (!file || strcmp(fsp->file, file) != 0)
return 0;
}
@@ -1071,7 +1071,7 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
return DWARF_CB_OK;
/* Check declared file */
- fname = dwarf_decl_file(sp_die);
+ fname = die_get_decl_file(sp_die);
if (!fname) {
pr_warning("A function DIE doesn't have decl_line. Maybe broken DWARF?\n");
return DWARF_CB_OK;
@@ -1151,7 +1151,7 @@ static int pubname_search_cb(Dwarf *dbg, Dwarf_Global *gl, void *data)
return DWARF_CB_OK;
if (param->file) {
- fname = dwarf_decl_file(param->sp_die);
+ fname = die_get_decl_file(param->sp_die);
if (!fname || strtailcmp(param->file, fname))
return DWARF_CB_OK;
}
@@ -1750,7 +1750,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
goto post;
}
- fname = dwarf_decl_file(&spdie);
+ fname = die_get_decl_file(&spdie);
if (addr == baseaddr) {
/* Function entry - Relative line number is 0 */
lineno = baseline;
@@ -1787,7 +1787,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
}
}
/* Verify the lineno and baseline are in a same file */
- tmp = dwarf_decl_file(&spdie);
+ tmp = die_get_decl_file(&spdie);
if (!tmp || (fname && strcmp(tmp, fname) != 0))
lineno = 0;
}
@@ -1902,13 +1902,13 @@ static int line_range_search_cb(Dwarf_Die *sp_die, void *data)
/* Check declared file */
if (lr->file) {
- fname = dwarf_decl_file(sp_die);
+ fname = die_get_decl_file(sp_die);
if (!fname || strtailcmp(lr->file, fname))
return DWARF_CB_OK;
}
if (die_match_name(sp_die, lr->function) && die_is_func_def(sp_die)) {
- lf->fname = dwarf_decl_file(sp_die);
+ lf->fname = die_get_decl_file(sp_die);
dwarf_decl_line(sp_die, &lr->offset);
pr_debug("fname: %s, lineno:%d\n", lf->fname, lr->offset);
lf->lno_s = lr->offset + lr->start;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] tools/perf: Fix perf probe crash by broken DWARF file
2022-11-01 13:14 [PATCH 0/3] tools/perf: Fix perf probe crash by broken DWARF file Masami Hiramatsu (Google)
` (2 preceding siblings ...)
2022-11-01 13:14 ` [PATCH 3/3] tools/perf: Fix to get declared file name from broken DWARF5 Masami Hiramatsu (Google)
@ 2022-11-01 13:29 ` Masami Hiramatsu
3 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2022-11-01 13:29 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
linux-perf-users, linux-kernel, Steven Rostedt
On Tue, 1 Nov 2022 22:14:01 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> Hi,
>
> Here is a series of patches for perf probe which improves the robustness
> against broken DWARF file.
>
> Since the Clang generates the out of standard DWARF5 file, the perf probe
> crashes or failed to analyze it. There are actually fragile code against
> it, so I fixed it ([1/3]) to avoid crash by SEGV. And make it accepts
> Clang's DWARF5 file ([2/3],[3/3]).
>
> Without this series, the perf probe crashes with the DWARF5 file
> which generated by clang as below;
>
> $ ./perf probe -k $BIN_PATH/vmlinux -s $SRC_PATH -L vfs_read:10
> Segmentation fault
>
> This series fixes it to handle such file correctly;
>
> $ ./perf probe -k $BIN_PATH/vmlinux -s $SRC_PATH -L vfs_read:10
> <vfs_read@$SRC_PATH/fs/read_write.c:10>
>
> 11 ret = rw_verify_area(READ, file, pos, count);
> 12 if (ret)
> return ret;
>
> This issue is reported on LLVM ML;
> https://www.mail-archive.com/dwarf-discuss@lists.dwarfstd.org/msg00884.html
Hmm, according to the discussion in this thread, this spec is under discussion
(still?). As far as I can see the "DWARF Debugging Information Format Version 5"
2.14, it says "The value 0 indicates that no source line has been specified."
But the description will be updated(?)
http://wiki.dwarfstd.org/index.php?title=DWARF5_Line_Table_File_Numbers
Let me update the series so that carefully remove the "broken". Maybe "clang"
DWARF5?
Thank you,
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (Google) (3):
> tools/perf: Fix to avoid crashing with a broken DWARF file
> tools/perf: Fix to use dwarf_attr_integrate for generic attr accessor
> tools/perf: Fix to get declared file name from broken DWARF5
>
>
> tools/perf/util/dwarf-aux.c | 58 ++++++++++++++++++++++++++++------------
> tools/perf/util/dwarf-aux.h | 3 ++
> tools/perf/util/probe-finder.c | 37 +++++++++++++++++---------
> 3 files changed, 68 insertions(+), 30 deletions(-)
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread