LKML Archive mirror
 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 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 read-only IMAP folder(s) and NNTP newsgroup(s).