mbox series

[v2,0/3] tools/perf: Fix perf probe crash by clang DWARF5 file

Message ID 166731050151.2100653.8202870942871353491.stgit@devnote3
Headers
Series tools/perf: Fix perf probe crash by clang DWARF5 file |

Message

Masami Hiramatsu (Google) Nov. 1, 2022, 1:48 p.m. UTC 
  Hi,

Here is the 2nd version of the patches for perf probe which improves the
robustness against clang DWARF5 file.

Since the Clang generates a bit different DWARF5 file, the perf probe
crashes or failes 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;


On the DWARF5 specification, Sec 2.14, there is
"The value 0 indicates that no source file has been specified."
for DW_AT_decl_file, but clang generated DWARF5 will use the value 0.

This issue is discussed on DWARF std ML;
https://www.mail-archive.com/dwarf-discuss@lists.dwarfstd.org/msg00884.html

And suggested that removing this part from the specification.
http://wiki.dwarfstd.org/index.php?title=DWARF5_Line_Table_File_Numbers

So as far as I understand, this is out of standard at this moment,
but the standard itself has a discussion on this part. And maybe updated
as currently clang does in the next release/revision.

Thank you,

---

Masami Hiramatsu (Google) (3):
      tools/perf: Fix to avoid crashing if DW_AT_decl_file is NULL
      tools/perf: Fix to use dwarf_attr_integrate for generic attr accessor
      tools/perf: Fix to get declared file name from clang 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>

Comments

Namhyung Kim Nov. 2, 2022, 12:03 a.m. UTC | #1 
Hi Masami,

On Tue, Nov 1, 2022 at 6:48 AM Masami Hiramatsu (Google)
<mhiramat@kernel.org> wrote:
>
> Hi,
>
> Here is the 2nd version of the patches for perf probe which improves the
> robustness against clang DWARF5 file.
>
> Since the Clang generates a bit different DWARF5 file, the perf probe
> crashes or failes 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;
>
>
> On the DWARF5 specification, Sec 2.14, there is
> "The value 0 indicates that no source file has been specified."
> for DW_AT_decl_file, but clang generated DWARF5 will use the value 0.
>
> This issue is discussed on DWARF std ML;
> https://www.mail-archive.com/dwarf-discuss@lists.dwarfstd.org/msg00884.html
>
> And suggested that removing this part from the specification.
> http://wiki.dwarfstd.org/index.php?title=DWARF5_Line_Table_File_Numbers
>
> So as far as I understand, this is out of standard at this moment,
> but the standard itself has a discussion on this part. And maybe updated
> as currently clang does in the next release/revision.
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (Google) (3):
>       tools/perf: Fix to avoid crashing if DW_AT_decl_file is NULL
>       tools/perf: Fix to use dwarf_attr_integrate for generic attr accessor
>       tools/perf: Fix to get declared file name from clang DWARF5

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


>
>
>  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>
Arnaldo Carvalho de Melo Nov. 3, 2022, 12:31 p.m. UTC | #2 
Em Tue, Nov 01, 2022 at 05:03:51PM -0700, Namhyung Kim escreveu:
> On Tue, Nov 1, 2022 at 6:48 AM Masami Hiramatsu (Google)
> <mhiramat@kernel.org> wrote:
> > Here is the 2nd version of the patches for perf probe which improves the
> > robustness against clang DWARF5 file.
> >
> > Since the Clang generates a bit different DWARF5 file, the perf probe
> > crashes or failes 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]).

> > Masami Hiramatsu (Google) (3):
> >       tools/perf: Fix to avoid crashing if DW_AT_decl_file is NULL
> >       tools/perf: Fix to use dwarf_attr_integrate for generic attr accessor
> >       tools/perf: Fix to get declared file name from clang DWARF5
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied.

- Arnaldo