linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] perf script: perf script tests fails with segfault
@ 2025-04-29  6:51 Aditya Bodkhe
  2025-04-30 23:13 ` Ian Rogers
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Aditya Bodkhe @ 2025-04-29  6:51 UTC (permalink / raw)
  To: linux-perf-users; +Cc: Aditya Bodkhe, Disha Goel, Adrian Hunter

From: Aditya Bodkhe <aditya.b1@linux.ibm.com>

perf script: pert script tests fails with segmentation fault as below:

 92: perf script tests:
 --- start ---
test child forked, pid 103769
DB test
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
/usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
 --- Cleaning up ---
 ---- end(-1) ----
	92: perf script tests                                               : FAILED!

Backtrace pointed to :
	#0  0x0000000010247dd0 in maps.machine ()
	#1  0x00000000101d178c in db_export.sample ()
	#2  0x00000000103412c8 in python_process_event ()
	#3  0x000000001004eb28 in process_sample_event ()
	#4  0x000000001024fcd0 in machines.deliver_event ()
	#5  0x000000001025005c in perf_session.deliver_event ()
	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
	#7  0x0000000010251618 in perf_session.process_events ()
	#8  0x0000000010053620 in cmd_script ()
	#9  0x00000000100b5a28 in run_builtin ()
	#10 0x00000000100b5f94 in handle_internal_command ()
	#11 0x0000000010011114 in main ()

Further investigation reveals that this occurs in the `perf script tests`,
because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.

With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
perf doesn't set maps for "[H]" sample in the code. Consequently, `al->maps` remains NULL
when `maps__machine(al->maps)` is called from `db_export__sample`.

As al->maps can be NULL in case of Hypervisor samples , use thread->maps
because even for Hypervisor sample, machine should exist.
If we don't have machine for some reason, return -1 to avoid segmentation fault.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changelog:
v2 -> v3:
This patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to add extra spaces
before ---start--- and ---end---
to be able to apply patch without any issues

v1 -> v2:
The below patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to handle the segmentation fault ,
as well as initialisation of al.thread

 tools/perf/util/db-export.c                           | 11 ++++++++---
 .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
index 50f916374d87..8f52e8cefcf3 100644
--- a/tools/perf/util/db-export.c
+++ b/tools/perf/util/db-export.c
@@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
 	if (al->map) {
 		struct dso *dso = map__dso(al->map);

-		err = db_export__dso(dbe, dso, maps__machine(al->maps));
+		err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));
 		if (err)
 			return err;
 		*dso_db_id = dso__db_id(dso);
@@ -256,6 +256,7 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
 		al.map = map__get(node->ms.map);
 		al.maps = maps__get(thread__maps(thread));
 		al.addr = node->ip;
+		al.thread = thread__get(thread);

 		if (al.map && !al.sym)
 			al.sym = dso__find_symbol(map__dso(al.map), al.addr);
@@ -358,14 +359,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
 	};
 	struct thread *main_thread;
 	struct comm *comm = NULL;
-	struct machine *machine;
+	struct machine *machine = NULL;
 	int err;

+	if (thread__maps(thread))
+		machine = maps__machine(thread__maps(thread));
+	if (!machine)
+		return -1;
+
 	err = db_export__evsel(dbe, evsel);
 	if (err)
 		return err;

-	machine = maps__machine(al->maps);
 	err = db_export__machine(dbe, machine);
 	if (err)
 		return err;
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 520729e78965..00f2c6c5114d 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -1306,7 +1306,7 @@ static void python_export_sample_table(struct db_export *dbe,

 	tuple_set_d64(t, 0, es->db_id);
 	tuple_set_d64(t, 1, es->evsel->db_id);
-	tuple_set_d64(t, 2, maps__machine(es->al->maps)->db_id);
+	tuple_set_d64(t, 2, maps__machine(thread__maps(es->al->thread))->db_id);
 	tuple_set_d64(t, 3, thread__db_id(es->al->thread));
 	tuple_set_d64(t, 4, es->comm_db_id);
 	tuple_set_d64(t, 5, es->dso_db_id);
--
2.43.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] perf script: perf script tests fails with segfault
  2025-04-29  6:51 [PATCH v3] perf script: perf script tests fails with segfault Aditya Bodkhe
@ 2025-04-30 23:13 ` Ian Rogers
  2025-05-05 13:50   ` Adrian Hunter
  2025-05-06  5:39 ` Adrian Hunter
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2025-04-30 23:13 UTC (permalink / raw)
  To: Aditya Bodkhe; +Cc: linux-perf-users, Aditya Bodkhe, Disha Goel, Adrian Hunter

On Mon, Apr 28, 2025 at 11:52 PM Aditya Bodkhe <adityab1@linux.ibm.com> wrote:
>
> From: Aditya Bodkhe <aditya.b1@linux.ibm.com>
>
> perf script: pert script tests fails with segmentation fault as below:
>
>  92: perf script tests:
>  --- start ---
> test child forked, pid 103769
> DB test
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
> /usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>  --- Cleaning up ---
>  ---- end(-1) ----
>         92: perf script tests                                               : FAILED!
>
> Backtrace pointed to :
>         #0  0x0000000010247dd0 in maps.machine ()
>         #1  0x00000000101d178c in db_export.sample ()
>         #2  0x00000000103412c8 in python_process_event ()
>         #3  0x000000001004eb28 in process_sample_event ()
>         #4  0x000000001024fcd0 in machines.deliver_event ()
>         #5  0x000000001025005c in perf_session.deliver_event ()
>         #6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
>         #7  0x0000000010251618 in perf_session.process_events ()
>         #8  0x0000000010053620 in cmd_script ()
>         #9  0x00000000100b5a28 in run_builtin ()
>         #10 0x00000000100b5f94 in handle_internal_command ()
>         #11 0x0000000010011114 in main ()
>
> Further investigation reveals that this occurs in the `perf script tests`,
> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
>
> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
> perf doesn't set maps for "[H]" sample in the code. Consequently, `al->maps` remains NULL
> when `maps__machine(al->maps)` is called from `db_export__sample`.
>
> As al->maps can be NULL in case of Hypervisor samples , use thread->maps
> because even for Hypervisor sample, machine should exist.
> If we don't have machine for some reason, return -1 to avoid segmentation fault.
>
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> Changelog:
> v2 -> v3:
> This patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to add extra spaces
> before ---start--- and ---end---
> to be able to apply patch without any issues
>
> v1 -> v2:
> The below patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to handle the segmentation fault ,
> as well as initialisation of al.thread
>
>  tools/perf/util/db-export.c                           | 11 ++++++++---
>  .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 50f916374d87..8f52e8cefcf3 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
>         if (al->map) {
>                 struct dso *dso = map__dso(al->map);
>
> -               err = db_export__dso(dbe, dso, maps__machine(al->maps));
> +               err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));

Would it be better to add a new helper function to minimize the chance
of a segv, like:
```
struct machine *addr_location__machine(struct addr_location *al)
{
  struct machine *machine = NULL;
  struct maps *maps = al->maps;

  if (maps)
    machine = maps__machine(maps)
  if (!machine && al->thread) {
    maps = thread__maps(al->thread);
    if (maps)
      machine = maps__machine(maps)
 }
  return machine;
}
```
>                 if (err)
>                         return err;
>                 *dso_db_id = dso__db_id(dso);
> @@ -256,6 +256,7 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
>                 al.map = map__get(node->ms.map);
>                 al.maps = maps__get(thread__maps(thread));
>                 al.addr = node->ip;
> +               al.thread = thread__get(thread);
>
>                 if (al.map && !al.sym)
>                         al.sym = dso__find_symbol(map__dso(al.map), al.addr);
> @@ -358,14 +359,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>         };
>         struct thread *main_thread;
>         struct comm *comm = NULL;
> -       struct machine *machine;
> +       struct machine *machine = NULL;
>         int err;
>
> +       if (thread__maps(thread))
> +               machine = maps__machine(thread__maps(thread));
> +       if (!machine)
> +               return -1;
> +

Perhaps the machine should be stashed somewhere like struct db_export ?

Thanks,
Ian

>         err = db_export__evsel(dbe, evsel);
>         if (err)
>                 return err;
>
> -       machine = maps__machine(al->maps);
>         err = db_export__machine(dbe, machine);
>         if (err)
>                 return err;
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 520729e78965..00f2c6c5114d 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1306,7 +1306,7 @@ static void python_export_sample_table(struct db_export *dbe,
>
>         tuple_set_d64(t, 0, es->db_id);
>         tuple_set_d64(t, 1, es->evsel->db_id);
> -       tuple_set_d64(t, 2, maps__machine(es->al->maps)->db_id);
> +       tuple_set_d64(t, 2, maps__machine(thread__maps(es->al->thread))->db_id);
>         tuple_set_d64(t, 3, thread__db_id(es->al->thread));
>         tuple_set_d64(t, 4, es->comm_db_id);
>         tuple_set_d64(t, 5, es->dso_db_id);
> --
> 2.43.0
>
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] perf script: perf script tests fails with segfault
  2025-04-30 23:13 ` Ian Rogers
@ 2025-05-05 13:50   ` Adrian Hunter
  0 siblings, 0 replies; 7+ messages in thread
From: Adrian Hunter @ 2025-05-05 13:50 UTC (permalink / raw)
  To: Ian Rogers, Aditya Bodkhe; +Cc: linux-perf-users, Aditya Bodkhe, Disha Goel

On 01/05/2025 02:13, Ian Rogers wrote:
> On Mon, Apr 28, 2025 at 11:52 PM Aditya Bodkhe <adityab1@linux.ibm.com> wrote:
>>
>> From: Aditya Bodkhe <aditya.b1@linux.ibm.com>
>>
>> perf script: pert script tests fails with segmentation fault as below:
>>
>>  92: perf script tests:
>>  --- start ---
>> test child forked, pid 103769
>> DB test
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
>> /usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>>  --- Cleaning up ---
>>  ---- end(-1) ----
>>         92: perf script tests                                               : FAILED!
>>
>> Backtrace pointed to :
>>         #0  0x0000000010247dd0 in maps.machine ()
>>         #1  0x00000000101d178c in db_export.sample ()
>>         #2  0x00000000103412c8 in python_process_event ()
>>         #3  0x000000001004eb28 in process_sample_event ()
>>         #4  0x000000001024fcd0 in machines.deliver_event ()
>>         #5  0x000000001025005c in perf_session.deliver_event ()
>>         #6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
>>         #7  0x0000000010251618 in perf_session.process_events ()
>>         #8  0x0000000010053620 in cmd_script ()
>>         #9  0x00000000100b5a28 in run_builtin ()
>>         #10 0x00000000100b5f94 in handle_internal_command ()
>>         #11 0x0000000010011114 in main ()
>>
>> Further investigation reveals that this occurs in the `perf script tests`,
>> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
>>
>> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
>> perf doesn't set maps for "[H]" sample in the code. Consequently, `al->maps` remains NULL
>> when `maps__machine(al->maps)` is called from `db_export__sample`.
>>
>> As al->maps can be NULL in case of Hypervisor samples , use thread->maps
>> because even for Hypervisor sample, machine should exist.
>> If we don't have machine for some reason, return -1 to avoid segmentation fault.
>>
>> Reported-by: Disha Goel <disgoel@linux.ibm.com>
>> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
>> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>> Changelog:
>> v2 -> v3:
>> This patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to add extra spaces
>> before ---start--- and ---end---
>> to be able to apply patch without any issues
>>
>> v1 -> v2:
>> The below patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to handle the segmentation fault ,
>> as well as initialisation of al.thread
>>
>>  tools/perf/util/db-export.c                           | 11 ++++++++---
>>  .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
>>  2 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
>> index 50f916374d87..8f52e8cefcf3 100644
>> --- a/tools/perf/util/db-export.c
>> +++ b/tools/perf/util/db-export.c
>> @@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
>>         if (al->map) {
>>                 struct dso *dso = map__dso(al->map);
>>
>> -               err = db_export__dso(dbe, dso, maps__machine(al->maps));
>> +               err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));
> 
> Would it be better to add a new helper function to minimize the chance
> of a segv, like:
> ```
> struct machine *addr_location__machine(struct addr_location *al)
> {
>   struct machine *machine = NULL;
>   struct maps *maps = al->maps;
> 
>   if (maps)
>     machine = maps__machine(maps)
>   if (!machine && al->thread) {
>     maps = thread__maps(al->thread);
>     if (maps)
>       machine = maps__machine(maps)
>  }
>   return machine;
> }
> ```

Those things have already been checked in db_export__sample(),
and there will still be a segfault if there is no machine,
so it does not really help.

>>                 if (err)
>>                         return err;
>>                 *dso_db_id = dso__db_id(dso);
>> @@ -256,6 +256,7 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
>>                 al.map = map__get(node->ms.map);
>>                 al.maps = maps__get(thread__maps(thread));
>>                 al.addr = node->ip;
>> +               al.thread = thread__get(thread);
>>
>>                 if (al.map && !al.sym)
>>                         al.sym = dso__find_symbol(map__dso(al.map), al.addr);
>> @@ -358,14 +359,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>>         };
>>         struct thread *main_thread;
>>         struct comm *comm = NULL;
>> -       struct machine *machine;
>> +       struct machine *machine = NULL;
>>         int err;
>>
>> +       if (thread__maps(thread))
>> +               machine = maps__machine(thread__maps(thread));
>> +       if (!machine)
>> +               return -1;
>> +
> 
> Perhaps the machine should be stashed somewhere like struct db_export ?

Different samples can be from different machines i.e. VMs.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] perf script: perf script tests fails with segfault
  2025-04-29  6:51 [PATCH v3] perf script: perf script tests fails with segfault Aditya Bodkhe
  2025-04-30 23:13 ` Ian Rogers
@ 2025-05-06  5:39 ` Adrian Hunter
  2025-05-06  6:48 ` Disha Goel
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Adrian Hunter @ 2025-05-06  5:39 UTC (permalink / raw)
  To: Aditya Bodkhe, linux-perf-users; +Cc: Aditya Bodkhe, Disha Goel, Ian Rogers

On 29/04/2025 09:51, Aditya Bodkhe wrote:
> From: Aditya Bodkhe <aditya.b1@linux.ibm.com>
> 
> perf script: pert script tests fails with segmentation fault as below:
> 
>  92: perf script tests:
>  --- start ---
> test child forked, pid 103769
> DB test
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
> /usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>  --- Cleaning up ---
>  ---- end(-1) ----
> 	92: perf script tests                                               : FAILED!
> 
> Backtrace pointed to :
> 	#0  0x0000000010247dd0 in maps.machine ()
> 	#1  0x00000000101d178c in db_export.sample ()
> 	#2  0x00000000103412c8 in python_process_event ()
> 	#3  0x000000001004eb28 in process_sample_event ()
> 	#4  0x000000001024fcd0 in machines.deliver_event ()
> 	#5  0x000000001025005c in perf_session.deliver_event ()
> 	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
> 	#7  0x0000000010251618 in perf_session.process_events ()
> 	#8  0x0000000010053620 in cmd_script ()
> 	#9  0x00000000100b5a28 in run_builtin ()
> 	#10 0x00000000100b5f94 in handle_internal_command ()
> 	#11 0x0000000010011114 in main ()
> 
> Further investigation reveals that this occurs in the `perf script tests`,
> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
> 
> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
> perf doesn't set maps for "[H]" sample in the code. Consequently, `al->maps` remains NULL
> when `maps__machine(al->maps)` is called from `db_export__sample`.
> 
> As al->maps can be NULL in case of Hypervisor samples , use thread->maps
> because even for Hypervisor sample, machine should exist.
> If we don't have machine for some reason, return -1 to avoid segmentation fault.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>

Ian suggested adding a helper but this fix is fine for now.
Anyone inclined could add the helper as a separate tidy-up patch.

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changelog:
> v2 -> v3:
> This patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to add extra spaces
> before ---start--- and ---end---
> to be able to apply patch without any issues
> 
> v1 -> v2:
> The below patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to handle the segmentation fault ,
> as well as initialisation of al.thread
> 
>  tools/perf/util/db-export.c                           | 11 ++++++++---
>  .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 50f916374d87..8f52e8cefcf3 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
>  	if (al->map) {
>  		struct dso *dso = map__dso(al->map);
> 
> -		err = db_export__dso(dbe, dso, maps__machine(al->maps));
> +		err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));
>  		if (err)
>  			return err;
>  		*dso_db_id = dso__db_id(dso);
> @@ -256,6 +256,7 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
>  		al.map = map__get(node->ms.map);
>  		al.maps = maps__get(thread__maps(thread));
>  		al.addr = node->ip;
> +		al.thread = thread__get(thread);
> 
>  		if (al.map && !al.sym)
>  			al.sym = dso__find_symbol(map__dso(al.map), al.addr);
> @@ -358,14 +359,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>  	};
>  	struct thread *main_thread;
>  	struct comm *comm = NULL;
> -	struct machine *machine;
> +	struct machine *machine = NULL;
>  	int err;
> 
> +	if (thread__maps(thread))
> +		machine = maps__machine(thread__maps(thread));
> +	if (!machine)
> +		return -1;
> +
>  	err = db_export__evsel(dbe, evsel);
>  	if (err)
>  		return err;
> 
> -	machine = maps__machine(al->maps);
>  	err = db_export__machine(dbe, machine);
>  	if (err)
>  		return err;
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 520729e78965..00f2c6c5114d 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1306,7 +1306,7 @@ static void python_export_sample_table(struct db_export *dbe,
> 
>  	tuple_set_d64(t, 0, es->db_id);
>  	tuple_set_d64(t, 1, es->evsel->db_id);
> -	tuple_set_d64(t, 2, maps__machine(es->al->maps)->db_id);
> +	tuple_set_d64(t, 2, maps__machine(thread__maps(es->al->thread))->db_id);
>  	tuple_set_d64(t, 3, thread__db_id(es->al->thread));
>  	tuple_set_d64(t, 4, es->comm_db_id);
>  	tuple_set_d64(t, 5, es->dso_db_id);
> --
> 2.43.0
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] perf script: perf script tests fails with segfault
  2025-04-29  6:51 [PATCH v3] perf script: perf script tests fails with segfault Aditya Bodkhe
  2025-04-30 23:13 ` Ian Rogers
  2025-05-06  5:39 ` Adrian Hunter
@ 2025-05-06  6:48 ` Disha Goel
  2025-05-11  5:33 ` Aditya Bodkhe
  2025-06-10 18:39 ` Namhyung Kim
  4 siblings, 0 replies; 7+ messages in thread
From: Disha Goel @ 2025-05-06  6:48 UTC (permalink / raw)
  To: Aditya Bodkhe, linux-perf-users; +Cc: Aditya Bodkhe, Adrian Hunter

On 29/04/25 12:21 pm, Aditya Bodkhe wrote:
> From: Aditya Bodkhe <aditya.b1@linux.ibm.com>
> 
> perf script: pert script tests fails with segmentation fault as below:
> 
>   92: perf script tests:
>   --- start ---
> test child forked, pid 103769
> DB test
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
> /usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>   --- Cleaning up ---
>   ---- end(-1) ----
> 	92: perf script tests                                               : FAILED!
> 
> Backtrace pointed to :
> 	#0  0x0000000010247dd0 in maps.machine ()
> 	#1  0x00000000101d178c in db_export.sample ()
> 	#2  0x00000000103412c8 in python_process_event ()
> 	#3  0x000000001004eb28 in process_sample_event ()
> 	#4  0x000000001024fcd0 in machines.deliver_event ()
> 	#5  0x000000001025005c in perf_session.deliver_event ()
> 	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
> 	#7  0x0000000010251618 in perf_session.process_events ()
> 	#8  0x0000000010053620 in cmd_script ()
> 	#9  0x00000000100b5a28 in run_builtin ()
> 	#10 0x00000000100b5f94 in handle_internal_command ()
> 	#11 0x0000000010011114 in main ()
> 
> Further investigation reveals that this occurs in the `perf script tests`,
> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
> 
> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
> perf doesn't set maps for "[H]" sample in the code. Consequently, `al->maps` remains NULL
> when `maps__machine(al->maps)` is called from `db_export__sample`.
> 
> As al->maps can be NULL in case of Hypervisor samples , use thread->maps
> because even for Hypervisor sample, machine should exist.
> If we don't have machine for some reason, return -1 to avoid segmentation fault.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>

I have tested the patch on PowerPC, it fixes the reported issue and test 
passes.

# ./perf test -v 'perf script tests'
  95: perf script tests : Ok

Tested-by: Disha Goel <disgoel@linux.ibm.com>

> ---
> Changelog:
> v2 -> v3:
> This patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to add extra spaces
> before ---start--- and ---end---
> to be able to apply patch without any issues
> 
> v1 -> v2:
> The below patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to handle the segmentation fault ,
> as well as initialisation of al.thread
> 
>   tools/perf/util/db-export.c                           | 11 ++++++++---
>   .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 50f916374d87..8f52e8cefcf3 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
>   	if (al->map) {
>   		struct dso *dso = map__dso(al->map);
> 
> -		err = db_export__dso(dbe, dso, maps__machine(al->maps));
> +		err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));
>   		if (err)
>   			return err;
>   		*dso_db_id = dso__db_id(dso);
> @@ -256,6 +256,7 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
>   		al.map = map__get(node->ms.map);
>   		al.maps = maps__get(thread__maps(thread));
>   		al.addr = node->ip;
> +		al.thread = thread__get(thread);
> 
>   		if (al.map && !al.sym)
>   			al.sym = dso__find_symbol(map__dso(al.map), al.addr);
> @@ -358,14 +359,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>   	};
>   	struct thread *main_thread;
>   	struct comm *comm = NULL;
> -	struct machine *machine;
> +	struct machine *machine = NULL;
>   	int err;
> 
> +	if (thread__maps(thread))
> +		machine = maps__machine(thread__maps(thread));
> +	if (!machine)
> +		return -1;
> +
>   	err = db_export__evsel(dbe, evsel);
>   	if (err)
>   		return err;
> 
> -	machine = maps__machine(al->maps);
>   	err = db_export__machine(dbe, machine);
>   	if (err)
>   		return err;
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 520729e78965..00f2c6c5114d 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1306,7 +1306,7 @@ static void python_export_sample_table(struct db_export *dbe,
> 
>   	tuple_set_d64(t, 0, es->db_id);
>   	tuple_set_d64(t, 1, es->evsel->db_id);
> -	tuple_set_d64(t, 2, maps__machine(es->al->maps)->db_id);
> +	tuple_set_d64(t, 2, maps__machine(thread__maps(es->al->thread))->db_id);
>   	tuple_set_d64(t, 3, thread__db_id(es->al->thread));
>   	tuple_set_d64(t, 4, es->comm_db_id);
>   	tuple_set_d64(t, 5, es->dso_db_id);
> --
> 2.43.0
> 
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] perf script: perf script tests fails with segfault
  2025-04-29  6:51 [PATCH v3] perf script: perf script tests fails with segfault Aditya Bodkhe
                   ` (2 preceding siblings ...)
  2025-05-06  6:48 ` Disha Goel
@ 2025-05-11  5:33 ` Aditya Bodkhe
  2025-06-10 18:39 ` Namhyung Kim
  4 siblings, 0 replies; 7+ messages in thread
From: Aditya Bodkhe @ 2025-05-11  5:33 UTC (permalink / raw)
  To: linux-perf-users; +Cc: Aditya Bodkhe, Disha Goel, Adrian Hunter

I will handle helper as a separate patch . Can this patch be pulled in.


Thanks

Aditya Bodkhe

On 29/04/25 12:21 pm, Aditya Bodkhe wrote:
> From: Aditya Bodkhe <aditya.b1@linux.ibm.com>
>
> perf script: pert script tests fails with segmentation fault as below:
>
>   92: perf script tests:
>   --- start ---
> test child forked, pid 103769
> DB test
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
> /usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>   --- Cleaning up ---
>   ---- end(-1) ----
> 	92: perf script tests                                               : FAILED!
>
> Backtrace pointed to :
> 	#0  0x0000000010247dd0 in maps.machine ()
> 	#1  0x00000000101d178c in db_export.sample ()
> 	#2  0x00000000103412c8 in python_process_event ()
> 	#3  0x000000001004eb28 in process_sample_event ()
> 	#4  0x000000001024fcd0 in machines.deliver_event ()
> 	#5  0x000000001025005c in perf_session.deliver_event ()
> 	#6  0x00000000102568b0 in __ordered_events__flush.part.0 ()
> 	#7  0x0000000010251618 in perf_session.process_events ()
> 	#8  0x0000000010053620 in cmd_script ()
> 	#9  0x00000000100b5a28 in run_builtin ()
> 	#10 0x00000000100b5f94 in handle_internal_command ()
> 	#11 0x0000000010011114 in main ()
>
> Further investigation reveals that this occurs in the `perf script tests`,
> because it uses `db_test.py` script. This script sets `perf_db_export_mode = True`.
>
> With `perf_db_export_mode` enabled, if a sample originates from a hypervisor,
> perf doesn't set maps for "[H]" sample in the code. Consequently, `al->maps` remains NULL
> when `maps__machine(al->maps)` is called from `db_export__sample`.
>
> As al->maps can be NULL in case of Hypervisor samples , use thread->maps
> because even for Hypervisor sample, machine should exist.
> If we don't have machine for some reason, return -1 to avoid segmentation fault.
>
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Aditya Bodkhe <aditya.b1@linux.ibm.com>
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> Changelog:
> v2 -> v3:
> This patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to add extra spaces
> before ---start--- and ---end---
> to be able to apply patch without any issues
>
> v1 -> v2:
> The below patch contains changes suggested by Adrian Hunter<adrian.hunter@intel.com> to handle the segmentation fault ,
> as well as initialisation of al.thread
>
>   tools/perf/util/db-export.c                           | 11 ++++++++---
>   .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
>   2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/db-export.c b/tools/perf/util/db-export.c
> index 50f916374d87..8f52e8cefcf3 100644
> --- a/tools/perf/util/db-export.c
> +++ b/tools/perf/util/db-export.c
> @@ -181,7 +181,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
>   	if (al->map) {
>   		struct dso *dso = map__dso(al->map);
>
> -		err = db_export__dso(dbe, dso, maps__machine(al->maps));
> +		err = db_export__dso(dbe, dso, maps__machine(thread__maps(al->thread)));
>   		if (err)
>   			return err;
>   		*dso_db_id = dso__db_id(dso);
> @@ -256,6 +256,7 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
>   		al.map = map__get(node->ms.map);
>   		al.maps = maps__get(thread__maps(thread));
>   		al.addr = node->ip;
> +		al.thread = thread__get(thread);
>
>   		if (al.map && !al.sym)
>   			al.sym = dso__find_symbol(map__dso(al.map), al.addr);
> @@ -358,14 +359,18 @@ int db_export__sample(struct db_export *dbe, union perf_event *event,
>   	};
>   	struct thread *main_thread;
>   	struct comm *comm = NULL;
> -	struct machine *machine;
> +	struct machine *machine = NULL;
>   	int err;
>
> +	if (thread__maps(thread))
> +		machine = maps__machine(thread__maps(thread));
> +	if (!machine)
> +		return -1;
> +
>   	err = db_export__evsel(dbe, evsel);
>   	if (err)
>   		return err;
>
> -	machine = maps__machine(al->maps);
>   	err = db_export__machine(dbe, machine);
>   	if (err)
>   		return err;
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 520729e78965..00f2c6c5114d 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1306,7 +1306,7 @@ static void python_export_sample_table(struct db_export *dbe,
>
>   	tuple_set_d64(t, 0, es->db_id);
>   	tuple_set_d64(t, 1, es->evsel->db_id);
> -	tuple_set_d64(t, 2, maps__machine(es->al->maps)->db_id);
> +	tuple_set_d64(t, 2, maps__machine(thread__maps(es->al->thread))->db_id);
>   	tuple_set_d64(t, 3, thread__db_id(es->al->thread));
>   	tuple_set_d64(t, 4, es->comm_db_id);
>   	tuple_set_d64(t, 5, es->dso_db_id);
> --
> 2.43.0
>
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] perf script: perf script tests fails with segfault
  2025-04-29  6:51 [PATCH v3] perf script: perf script tests fails with segfault Aditya Bodkhe
                   ` (3 preceding siblings ...)
  2025-05-11  5:33 ` Aditya Bodkhe
@ 2025-06-10 18:39 ` Namhyung Kim
  4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2025-06-10 18:39 UTC (permalink / raw)
  To: linux-perf-users, Aditya Bodkhe; +Cc: Aditya Bodkhe, Disha Goel, Adrian Hunter

On Tue, 29 Apr 2025 12:21:32 +0530, Aditya Bodkhe wrote:
> perf script: pert script tests fails with segmentation fault as below:
> 
>  92: perf script tests:
>  --- start ---
> test child forked, pid 103769
> DB test
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.012 MB /tmp/perf-test-script.7rbftEpOzX/perf.data (9 samples) ]
> /usr/libexec/perf-core/tests/shell/script.sh: line 35: 103780 Segmentation fault      (core dumped) perf script -i "${perfdatafile}" -s "${db_test}"
>  --- Cleaning up ---
>  ---- end(-1) ----
> 	92: perf script tests                                               : FAILED!
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-06-10 18:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-29  6:51 [PATCH v3] perf script: perf script tests fails with segfault Aditya Bodkhe
2025-04-30 23:13 ` Ian Rogers
2025-05-05 13:50   ` Adrian Hunter
2025-05-06  5:39 ` Adrian Hunter
2025-05-06  6:48 ` Disha Goel
2025-05-11  5:33 ` Aditya Bodkhe
2025-06-10 18:39 ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).