[PATCH v2] Subject: perf jit: Fix incorrect file name in DWARF line table

[PATCH v2] Subject: perf jit: Fix incorrect file name in DWARF line table

[Elisabeth]

From: elisabeth <paniii94@gmail.com>

Fixes an issue where an incorrect filename was added in the DWARF line table of
an ELF object file when calling 'perf inject --jit' due to not checking the
filename of a debug entry against the repeated name marker (/xff/0).
The marker is mentioned in the tools/perf/util/jitdump.h header, which describes
the jitdump binary format, and indicitates that the filename in a debug entry
is the same as the previous enrty.
In the function emit_lineno_info(), in the file tools/perf/util/genelf-debug.c,
the debug entry filename gets compared to the previous entry filename. If they
are not the same, a new filename is added to the DWARF line table. However,
since there is no check against '\xff\0', in some cases '\xff\0' is inserted
as the filename into the DWARF line table.
This can be seen with `objdump --dwarf=line` on the ELF file after `perf inject --jit`.
It also makes no source code information show up in 'perf annotate'.

Signed-off-by: Elisabeth Panholzer <elisabeth@leaningtech.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-perf-users@vger.kernel.org
---
Changes in v2:
	- Made the commit message more descriptive
	- Added a comment that mentions the jitdump format
	- Changed memcmp() to strcmp()

 tools/perf/util/genelf_debug.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/genelf_debug.c b/tools/perf/util/genelf_debug.c
index aa5dcc56b2ac..138fdf87eaa2 100644
--- a/tools/perf/util/genelf_debug.c
+++ b/tools/perf/util/genelf_debug.c
@@ -337,6 +337,9 @@ static void emit_lineno_info(struct buffer_ext *be,
 {
 	size_t i;
 
+	/* as described in the jitdump format */
+	const char repeated_name_marker[] = {'\xff', '\0'}; 
+
 	/*
 	 * Machine state at start of a statement program
 	 * address = 0
@@ -363,7 +366,8 @@ static void emit_lineno_info(struct buffer_ext *be,
 		/*
 		 * check if filename changed, if so add it
 		 */
-		if (!cur_filename || strcmp(cur_filename, ent->name)) {
+		if ((!cur_filename || strcmp(cur_filename, ent->name)) &&
+			strcmp(repeated_name_marker, ent->name)) {
 			emit_lne_define_filename(be, ent->name);
 			cur_filename = ent->name;
 			emit_set_file(be, ++cur_file_idx);
-- 
2.34.1

Re: [PATCH v2] Subject: perf jit: Fix incorrect file name in DWARF line table

[Namhyung Kim]

Hello,

On Fri, Jun 2, 2023 at 5:38 AM Elisabeth <paniii94@gmail.com> wrote:
>
> From: elisabeth <paniii94@gmail.com>
>
> Fixes an issue where an incorrect filename was added in the DWARF line table of
> an ELF object file when calling 'perf inject --jit' due to not checking the
> filename of a debug entry against the repeated name marker (/xff/0).
> The marker is mentioned in the tools/perf/util/jitdump.h header, which describes
> the jitdump binary format, and indicitates that the filename in a debug entry
> is the same as the previous enrty.
> In the function emit_lineno_info(), in the file tools/perf/util/genelf-debug.c,
> the debug entry filename gets compared to the previous entry filename. If they
> are not the same, a new filename is added to the DWARF line table. However,
> since there is no check against '\xff\0', in some cases '\xff\0' is inserted
> as the filename into the DWARF line table.
> This can be seen with `objdump --dwarf=line` on the ELF file after `perf inject --jit`.
> It also makes no source code information show up in 'perf annotate'.
>
> Signed-off-by: Elisabeth Panholzer <elisabeth@leaningtech.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-perf-users@vger.kernel.org

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

Thanks,
Namhyung


> ---
> Changes in v2:
>         - Made the commit message more descriptive
>         - Added a comment that mentions the jitdump format
>         - Changed memcmp() to strcmp()
>
>  tools/perf/util/genelf_debug.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/genelf_debug.c b/tools/perf/util/genelf_debug.c
> index aa5dcc56b2ac..138fdf87eaa2 100644
> --- a/tools/perf/util/genelf_debug.c
> +++ b/tools/perf/util/genelf_debug.c
> @@ -337,6 +337,9 @@ static void emit_lineno_info(struct buffer_ext *be,
>  {
>         size_t i;
>
> +       /* as described in the jitdump format */
> +       const char repeated_name_marker[] = {'\xff', '\0'};
> +
>         /*
>          * Machine state at start of a statement program
>          * address = 0
> @@ -363,7 +366,8 @@ static void emit_lineno_info(struct buffer_ext *be,
>                 /*
>                  * check if filename changed, if so add it
>                  */
> -               if (!cur_filename || strcmp(cur_filename, ent->name)) {
> +               if ((!cur_filename || strcmp(cur_filename, ent->name)) &&
> +                       strcmp(repeated_name_marker, ent->name)) {
>                         emit_lne_define_filename(be, ent->name);
>                         cur_filename = ent->name;
>                         emit_set_file(be, ++cur_file_idx);
> --
> 2.34.1
>

Re: [PATCH v2] Subject: perf jit: Fix incorrect file name in DWARF line table

[Namhyung Kim]

On Fri, Jun 2, 2023 at 5:38 AM Elisabeth <paniii94@gmail.com> wrote:
>
> From: elisabeth <paniii94@gmail.com>
>
> Fixes an issue where an incorrect filename was added in the DWARF line table of
> an ELF object file when calling 'perf inject --jit' due to not checking the
> filename of a debug entry against the repeated name marker (/xff/0).
> The marker is mentioned in the tools/perf/util/jitdump.h header, which describes
> the jitdump binary format, and indicitates that the filename in a debug entry
> is the same as the previous enrty.
> In the function emit_lineno_info(), in the file tools/perf/util/genelf-debug.c,
> the debug entry filename gets compared to the previous entry filename. If they
> are not the same, a new filename is added to the DWARF line table. However,
> since there is no check against '\xff\0', in some cases '\xff\0' is inserted
> as the filename into the DWARF line table.
> This can be seen with `objdump --dwarf=line` on the ELF file after `perf inject --jit`.
> It also makes no source code information show up in 'perf annotate'.
>
> Signed-off-by: Elisabeth Panholzer <elisabeth@leaningtech.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-perf-users@vger.kernel.org

Applied to perf-tools-next, thanks!