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 }
Hi Arnaldo,
On Fri, May 10, 2024 at 7:40 AM Arnaldo Carvalho de Melo
<[email protected]> 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/threadc: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
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 <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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
On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <[email protected]> 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 <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
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
>
On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <[email protected]> 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 <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
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
>
On Fri, May 10, 2024 at 02:27:36PM -0700, Ian Rogers wrote:
> On Fri, May 10, 2024 at 2:04 PM Namhyung Kim <[email protected]> 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 <[email protected]>
> > Signed-off-by: Namhyung Kim <[email protected]>
>
> Reviewed-by: Ian Rogers <[email protected]>
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
> >