All of lore.kernel.org
 help / color / mirror / Atom feed
* perf annotate --data-type segfault
@ 2024-05-10 14:40 Arnaldo Carvalho de Melo
  2024-05-10 20:56 ` Namhyung Kim
  0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-05-10 14:40 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: Linux Kernel Mailing List

root@number:~# perf --debug type-profile annotate --data-type -i perf.data.perf-trace-bpf &> perf--debug.type-profile-annotate--data-type.perf-trace-bpf.output
Segmentation fault (core dumped)
root@number:~#

The output ended in:

Annotate type: 'struct sk_security_struct' in [kernel.kallsyms] (1 samples):
============================================================================
 Percent     offset       size  field
  100.00          0         32  struct sk_security_struct        {
    0.00          0          4      enum        nlbl_state;
    0.00          8          8      struct netlbl_lsm_secattr*  nlbl_secattr;
  100.00         16          4      u32 sid;
    0.00         20          4      u32 peer_sid;
    0.00         24          2      u16 sclass;
    0.00         28          4      enum        sctp_assoc_state;
                                };

Annotate type: 'struct hlist_head[]' in [kernel.kallsyms] (1 samples):
============================================================================
 Percent     offset       size  field
  100.00          0         72  struct hlist_head[]     ;

Annotate type: 'struct x86_pmu' in [kernel.kallsyms] (5 samples):
============================================================================
 Percent     offset       size  field
  100.00          0        640  struct x86_pmu   {
    0.00          0          8      char*       name;
    0.00          8          4      int version;
  100.00         16          8      (function_type)*    handle_irq;
    0.00         24          8      (function_type)*    disable_all;
    0.00         32          8      (function_type)*    enable_all;
    0.00         40          8      (function_type)*    enable;
    0.00         48          8      (function_type)*    disable;
    0.00         56          8      (function_type)*    assign;
    0.00         64          8      (function_type)*    add;
    0.00         72          8      (function_type)*    del;
    0.00         80          8      (function_type)*    read;
    0.00         88          8      (function_type)*    set_period;
    0.00         96          8      (function_type)*    update;
    0.00        104          8      (function_type)*    hw_config;
    0.00        112          8      (function_type)*    schedule_events;
    0.00        120          4      unsigned int        eventsel;
    0.00        124          4      unsigned int        perfctr;
    0.00        128          8      (function_type)*    addr_offset;
    0.00        136          8      (function_type)*    rdpmc_index;
    0.00        144          8      (function_type)*    event_map;
    0.00        152          4      int max_events;
    0.00        156          4      int num_counters;
    0.00        160          4      int num_counters_fixed;
    0.00        164          4      int cntval_bits;
    0.00        168          8      u64 cntval_mask;
    0.00        176          8      union        {
    0.00        176          8          long unsigned int       events_maskl;
    0.00        176          8          long unsigned int[]     events_mask;
                                    };
    0.00        184          4      int events_mask_len;
    0.00        188          4      int apic;
    0.00        192          8      u64 max_period;
    0.00        200          8      (function_type)*    get_event_constraints;
    0.00        208          8      (function_type)*    put_event_constraints;
    0.00        216          8      (function_type)*    start_scheduling;
    0.00        224          8      (function_type)*    commit_scheduling;
    0.00        232          8      (function_type)*    stop_scheduling;
    0.00        240          8      struct event_constraint*    event_constraints;
    0.00        248          8      struct x86_pmu_quirk*       quirks;
    0.00        256          8      (function_type)*    limit_period;
    0.00          0          4      unsigned int        late_ack;
    0.00          0          4      unsigned int        mid_ack;
    0.00          0          4      unsigned int        enabled_ack;
    0.00        268          4      int attr_rdpmc_broken;
    0.00        272          4      int attr_rdpmc;
    0.00        280          8      struct attribute**  format_attrs;
    0.00        288          8      (function_type)*    events_sysfs_show;
    0.00        296          8      struct attribute_group**    attr_update;
    0.00        304          8      long unsigned int   attr_freeze_on_smi;
    0.00        312          8      (function_type)*    cpu_prepare;
    0.00        320          8      (function_type)*    cpu_starting;
    0.00        328          8      (function_type)*    cpu_dying;
    0.00        336          8      (function_type)*    cpu_dead;
    0.00        344          8      (function_type)*    check_microcode;
    0.00        352          8      (function_type)*    sched_task;
    0.00        360          8      u64 intel_ctrl;
    0.00        368          8      union perf_capabilities     intel_cap {
    0.00        368          8          struct   {
    0.00        368          8              u64 lbr_format;
    0.00        368          8              u64 pebs_trap;
    0.00        368          8              u64 pebs_arch_reg;
    0.00        368          8              u64 pebs_format;
    0.00        368          8              u64 smm_freeze;
    0.00        368          8              u64 full_width_write;
    0.00        368          8              u64 pebs_baseline;
    0.00        368          8              u64 perf_metrics;
    0.00        368          8              u64 pebs_output_pt_available;
    0.00        368          8              u64 pebs_timing_info;
    0.00        368          8              u64 anythread_deprecated;
                                        };
    0.00        368          8          u64     capabilities;
                                    };
    0.00          0          4      unsigned int        bts;
    0.00          0          4      unsigned int        bts_active;
    0.00          0          4      unsigned int        pebs;
    0.00          0          4      unsigned int        pebs_active;
    0.00          0          4      unsigned int        pebs_broken;
    0.00          0          4      unsigned int        pebs_prec_dist;
    0.00          0          4      unsigned int        pebs_no_tlb;
    0.00          0          4      unsigned int        pebs_no_isolation;
    0.00          0          4      unsigned int        pebs_block;
    0.00          0          4      unsigned int        pebs_ept;
    0.00        380          4      int pebs_record_size;

0x00000000006c3fba in __zfree (ptr=0x0) at ../../lib/zalloc.c:13
13		free(*ptr);
(gdb) bt
#0  0x00000000006c3fba in __zfree (ptr=0x0) at ../../lib/zalloc.c:13
#1  0x00000000006728b5 in delete_data_type_histograms (adt=0xd151f70) at util/annotate-data.c:1829
#2  0x0000000000672958 in annotated_data_type__tree_delete (root=0xe82e40) at util/annotate-data.c:1843
#3  0x000000000055658e in dso__delete (dso=0xe82dd0) at util/dso.c:1487
#4  0x000000000055673e in dso__put (dso=0xe82dd0) at util/dso.c:1523
#5  0x000000000058289d in __dso__zput (dso=0x11fc500) at util/dso.h:644
#6  0x0000000000583dc5 in map__exit (map=0x11fc4e0) at util/map.c:298
#7  0x0000000000583e03 in map__delete (map=0x11fc4e0) at util/map.c:303
#8  0x0000000000583e6c in map__put (map=0x11fc4e0) at util/map.c:310
#9  0x00000000005854c3 in __map__zput (map=0x11fcdf0) at util/map.h:196
#10 0x0000000000585e13 in maps__exit (maps=0x11fb740) at util/maps.c:236
#11 0x0000000000585f0e in maps__delete (maps=0x11fb740) at util/maps.c:258
#12 0x0000000000585fcf in maps__put (maps=0x11fb740) at util/maps.c:275
#13 0x0000000000597d2c in thread__delete (thread=0x11fb580) at util/thread.c:96
#14 0x0000000000597fd6 in thread__put (thread=0x11fb580) at util/thread.c:140
#15 0x00000000005c4940 in __thread__zput (thread=0x25bb7c8) at util/thread.h:83
#16 0x00000000005c8267 in hist_entry__delete (he=0x25bb720) at util/hist.c:1318
#17 0x00000000005c5bc2 in hists__delete_entry (hists=0xe7f9f0, he=0x25bb720) at util/hist.c:388
#18 0x00000000005c5d10 in hists__delete_entries (hists=0xe7f9f0) at util/hist.c:416
#19 0x00000000005cc62d in hists__delete_all_entries (hists=0xe7f9f0) at util/hist.c:2872
#20 0x00000000005cc6a7 in hists_evsel__exit (evsel=0xe7f780) at util/hist.c:2884
#21 0x000000000053378a in evsel__exit (evsel=0xe7f780) at util/evsel.c:1495
#22 0x00000000005337cf in evsel__delete (evsel=0xe7f780) at util/evsel.c:1503
#23 0x00000000005288af in evlist__purge (evlist=0xe7e410) at util/evlist.c:163
#24 0x00000000005289bc in evlist__delete (evlist=0xe7e410) at util/evlist.c:185
#25 0x0000000000589c2d in perf_session__delete (session=0xe7daf0) at util/session.c:313
#26 0x00000000004136cb in cmd_annotate (argc=0, argv=0x7fffffffe400) at builtin-annotate.c:936
#27 0x0000000000507cf9 in run_builtin (p=0xe55fd8 <commands+408>, argc=4, argv=0x7fffffffe400) at perf.c:350
#28 0x0000000000507f68 in handle_internal_command (argc=4, argv=0x7fffffffe400) at perf.c:403
#29 0x00000000005080b7 in run_argv (argcp=0x7fffffffe1dc, argv=0x7fffffffe1d0) at perf.c:447
#30 0x00000000005083ae in main (argc=4, argv=0x7fffffffe400) at perf.c:561
(gdb)

1826	static void delete_data_type_histograms(struct annotated_data_type *adt)
1827	{
1828		for (int i = 0; i < adt->nr_histograms; i++)
1829			zfree(&(adt->histograms[i]));
1830		zfree(&adt->histograms);
1831	}


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

* Re: perf annotate --data-type segfault
  2024-05-10 14:40 perf annotate --data-type segfault Arnaldo Carvalho de Melo
@ 2024-05-10 20:56 ` Namhyung Kim
  2024-05-10 21:04   ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Namhyung Kim
  0 siblings, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2024-05-10 20:56 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Linux Kernel Mailing List

Hi Arnaldo,

On Fri, May 10, 2024 at 7:40 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> root@number:~# perf --debug type-profile annotate --data-type -i perf.data.perf-trace-bpf &> perf--debug.type-profile-annotate--data-type.perf-trace-bpf.output
> Segmentation fault (core dumped)
> root@number:~#
[SNIP]
> 0x00000000006c3fba in __zfree (ptr=0x0) at ../../lib/zalloc.c:13
> 13              free(*ptr);
> (gdb) bt
> #0  0x00000000006c3fba in __zfree (ptr=0x0) at ../../lib/zalloc.c:13
> #1  0x00000000006728b5 in delete_data_type_histograms (adt=0xd151f70) at util/annotate-data.c:1829
> #2  0x0000000000672958 in annotated_data_type__tree_delete (root=0xe82e40) at util/annotate-data.c:1843
> #3  0x000000000055658e in dso__delete (dso=0xe82dd0) at util/dso.c:1487
> #4  0x000000000055673e in dso__put (dso=0xe82dd0) at util/dso.c:1523
> #5  0x000000000058289d in __dso__zput (dso=0x11fc500) at util/dso.h:644
> #6  0x0000000000583dc5 in map__exit (map=0x11fc4e0) at util/map.c:298
> #7  0x0000000000583e03 in map__delete (map=0x11fc4e0) at util/map.c:303
> #8  0x0000000000583e6c in map__put (map=0x11fc4e0) at util/map.c:310
> #9  0x00000000005854c3 in __map__zput (map=0x11fcdf0) at util/map.h:196
> #10 0x0000000000585e13 in maps__exit (maps=0x11fb740) at util/maps.c:236
> #11 0x0000000000585f0e in maps__delete (maps=0x11fb740) at util/maps.c:258
> #12 0x0000000000585fcf in maps__put (maps=0x11fb740) at util/maps.c:275
> #13 0x0000000000597d2c in thread__delete (thread=0x11fb580) at util/thread.c:96
> #14 0x0000000000597fd6 in thread__put (thread=0x11fb580) at util/thread.c:140
> #15 0x00000000005c4940 in __thread__zput (thread=0x25bb7c8) at util/thread.h:83
> #16 0x00000000005c8267 in hist_entry__delete (he=0x25bb720) at util/hist.c:1318
> #17 0x00000000005c5bc2 in hists__delete_entry (hists=0xe7f9f0, he=0x25bb720) at util/hist.c:388
> #18 0x00000000005c5d10 in hists__delete_entries (hists=0xe7f9f0) at util/hist.c:416
> #19 0x00000000005cc62d in hists__delete_all_entries (hists=0xe7f9f0) at util/hist.c:2872
> #20 0x00000000005cc6a7 in hists_evsel__exit (evsel=0xe7f780) at util/hist.c:2884
> #21 0x000000000053378a in evsel__exit (evsel=0xe7f780) at util/evsel.c:1495
> #22 0x00000000005337cf in evsel__delete (evsel=0xe7f780) at util/evsel.c:1503
> #23 0x00000000005288af in evlist__purge (evlist=0xe7e410) at util/evlist.c:163
> #24 0x00000000005289bc in evlist__delete (evlist=0xe7e410) at util/evlist.c:185
> #25 0x0000000000589c2d in perf_session__delete (session=0xe7daf0) at util/session.c:313
> #26 0x00000000004136cb in cmd_annotate (argc=0, argv=0x7fffffffe400) at builtin-annotate.c:936
> #27 0x0000000000507cf9 in run_builtin (p=0xe55fd8 <commands+408>, argc=4, argv=0x7fffffffe400) at perf.c:350
> #28 0x0000000000507f68 in handle_internal_command (argc=4, argv=0x7fffffffe400) at perf.c:403
> #29 0x00000000005080b7 in run_argv (argcp=0x7fffffffe1dc, argv=0x7fffffffe1d0) at perf.c:447
> #30 0x00000000005083ae in main (argc=4, argv=0x7fffffffe400) at perf.c:561
> (gdb)
>
> 1826    static void delete_data_type_histograms(struct annotated_data_type *adt)
> 1827    {
> 1828            for (int i = 0; i < adt->nr_histograms; i++)
> 1829                    zfree(&(adt->histograms[i]));
> 1830            zfree(&adt->histograms);
> 1831    }

I don't understand why it has a NULL histogram entry.
One possibility would be it get a failure in the allocation
or it's freed but the nr_histograms field is not updated.

I also found a problem related to the type annotation.
I'll send a patchset to address the issues soon.

Thanks,
Namhyung

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

* [PATCH 1/2] perf annotate: Fix segfault on sample histogram
  2024-05-10 20:56 ` Namhyung Kim
@ 2024-05-10 21:04   ` Namhyung Kim
  2024-05-10 21:04     ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim
  2024-05-10 21:27     ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Ian Rogers
  0 siblings, 2 replies; 7+ messages in thread
From: Namhyung Kim @ 2024-05-10 21:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
  Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users

A symbol can have no samples, then accessing annotated_source->samples
hashmap will get a segfault.

Fixes: a3f7768bcf48 ("perf annotate: Fix memory leak in annotated_source")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 541988cf6e19..1451caf25e77 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -113,10 +113,11 @@ static __maybe_unused void annotated_source__delete(struct annotated_source *src
 	if (src == NULL)
 		return;
 
-	hashmap__for_each_entry(src->samples, cur, bkt)
-		zfree(&cur->pvalue);
-
-	hashmap__free(src->samples);
+	if (src->samples) {
+		hashmap__for_each_entry(src->samples, cur, bkt)
+			zfree(&cur->pvalue);
+		hashmap__free(src->samples);
+	}
 	zfree(&src->histograms);
 	free(src);
 }
-- 
2.45.0.118.g7fe29c98d7-goog


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

* [PATCH 2/2] perf annotate-data: Ensure the number of type histograms
  2024-05-10 21:04   ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Namhyung Kim
@ 2024-05-10 21:04     ` Namhyung Kim
  2024-05-10 21:27       ` Ian Rogers
  2024-05-10 21:27     ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Ian Rogers
  1 sibling, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2024-05-10 21:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
  Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users

Arnaldo reported that there is a case where nr_histograms and histograms
don't agree each other.  It ended up in a segfault trying to access NULL
histograms array.  Let's make sure to update the nr_histograms when the
histograms array is changed.

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate-data.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 57e7d4b3550b..965da6c0b542 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1800,7 +1800,6 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en
 	sz += sizeof(struct type_hist_entry) * adt->self.size;
 
 	/* Allocate a table of pointers for each event */
-	adt->nr_histograms = nr_entries;
 	adt->histograms = calloc(nr_entries, sizeof(*adt->histograms));
 	if (adt->histograms == NULL)
 		return -ENOMEM;
@@ -1814,6 +1813,8 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en
 		if (adt->histograms[i] == NULL)
 			goto err;
 	}
+
+	adt->nr_histograms = nr_entries;
 	return 0;
 
 err:
@@ -1827,7 +1828,9 @@ static void delete_data_type_histograms(struct annotated_data_type *adt)
 {
 	for (int i = 0; i < adt->nr_histograms; i++)
 		zfree(&(adt->histograms[i]));
+
 	zfree(&adt->histograms);
+	adt->nr_histograms = 0;
 }
 
 void annotated_data_type__tree_delete(struct rb_root *root)
-- 
2.45.0.118.g7fe29c98d7-goog


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

* Re: [PATCH 1/2] perf annotate: Fix segfault on sample histogram
  2024-05-10 21:04   ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Namhyung Kim
  2024-05-10 21:04     ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim
@ 2024-05-10 21:27     ` Ian Rogers
  1 sibling, 0 replies; 7+ messages in thread
From: Ian Rogers @ 2024-05-10 21:27 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> A symbol can have no samples, then accessing annotated_source->samples
> hashmap will get a segfault.
>
> Fixes: a3f7768bcf48 ("perf annotate: Fix memory leak in annotated_source")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/annotate.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 541988cf6e19..1451caf25e77 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -113,10 +113,11 @@ static __maybe_unused void annotated_source__delete(struct annotated_source *src
>         if (src == NULL)
>                 return;
>
> -       hashmap__for_each_entry(src->samples, cur, bkt)
> -               zfree(&cur->pvalue);
> -
> -       hashmap__free(src->samples);
> +       if (src->samples) {
> +               hashmap__for_each_entry(src->samples, cur, bkt)
> +                       zfree(&cur->pvalue);
> +               hashmap__free(src->samples);
> +       }
>         zfree(&src->histograms);
>         free(src);
>  }
> --
> 2.45.0.118.g7fe29c98d7-goog
>

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

* Re: [PATCH 2/2] perf annotate-data: Ensure the number of type histograms
  2024-05-10 21:04     ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim
@ 2024-05-10 21:27       ` Ian Rogers
  2024-05-11 15:43         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2024-05-10 21:27 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Arnaldo reported that there is a case where nr_histograms and histograms
> don't agree each other.  It ended up in a segfault trying to access NULL
> histograms array.  Let's make sure to update the nr_histograms when the
> histograms array is changed.
>
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/annotate-data.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 57e7d4b3550b..965da6c0b542 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -1800,7 +1800,6 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en
>         sz += sizeof(struct type_hist_entry) * adt->self.size;
>
>         /* Allocate a table of pointers for each event */
> -       adt->nr_histograms = nr_entries;
>         adt->histograms = calloc(nr_entries, sizeof(*adt->histograms));
>         if (adt->histograms == NULL)
>                 return -ENOMEM;
> @@ -1814,6 +1813,8 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en
>                 if (adt->histograms[i] == NULL)
>                         goto err;
>         }
> +
> +       adt->nr_histograms = nr_entries;
>         return 0;
>
>  err:
> @@ -1827,7 +1828,9 @@ static void delete_data_type_histograms(struct annotated_data_type *adt)
>  {
>         for (int i = 0; i < adt->nr_histograms; i++)
>                 zfree(&(adt->histograms[i]));
> +
>         zfree(&adt->histograms);
> +       adt->nr_histograms = 0;
>  }
>
>  void annotated_data_type__tree_delete(struct rb_root *root)
> --
> 2.45.0.118.g7fe29c98d7-goog
>

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

* Re: [PATCH 2/2] perf annotate-data: Ensure the number of type histograms
  2024-05-10 21:27       ` Ian Rogers
@ 2024-05-11 15:43         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-05-11 15:43 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

On Fri, May 10, 2024 at 02:27:36PM -0700, Ian Rogers wrote:
> On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Arnaldo reported that there is a case where nr_histograms and histograms
> > don't agree each other.  It ended up in a segfault trying to access NULL
> > histograms array.  Let's make sure to update the nr_histograms when the
> > histograms array is changed.
> >
> > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks, applied to perf-tools-next,

- Arnaldo
 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/annotate-data.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> > index 57e7d4b3550b..965da6c0b542 100644
> > --- a/tools/perf/util/annotate-data.c
> > +++ b/tools/perf/util/annotate-data.c
> > @@ -1800,7 +1800,6 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en
> >         sz += sizeof(struct type_hist_entry) * adt->self.size;
> >
> >         /* Allocate a table of pointers for each event */
> > -       adt->nr_histograms = nr_entries;
> >         adt->histograms = calloc(nr_entries, sizeof(*adt->histograms));
> >         if (adt->histograms == NULL)
> >                 return -ENOMEM;
> > @@ -1814,6 +1813,8 @@ static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_en
> >                 if (adt->histograms[i] == NULL)
> >                         goto err;
> >         }
> > +
> > +       adt->nr_histograms = nr_entries;
> >         return 0;
> >
> >  err:
> > @@ -1827,7 +1828,9 @@ static void delete_data_type_histograms(struct annotated_data_type *adt)
> >  {
> >         for (int i = 0; i < adt->nr_histograms; i++)
> >                 zfree(&(adt->histograms[i]));
> > +
> >         zfree(&adt->histograms);
> > +       adt->nr_histograms = 0;
> >  }
> >
> >  void annotated_data_type__tree_delete(struct rb_root *root)
> > --
> > 2.45.0.118.g7fe29c98d7-goog
> >

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

end of thread, other threads:[~2024-05-11 15:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-10 14:40 perf annotate --data-type segfault Arnaldo Carvalho de Melo
2024-05-10 20:56 ` Namhyung Kim
2024-05-10 21:04   ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Namhyung Kim
2024-05-10 21:04     ` [PATCH 2/2] perf annotate-data: Ensure the number of type histograms Namhyung Kim
2024-05-10 21:27       ` Ian Rogers
2024-05-11 15:43         ` Arnaldo Carvalho de Melo
2024-05-10 21:27     ` [PATCH 1/2] perf annotate: Fix segfault on sample histogram Ian Rogers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.