[PATCH] perf hist: Don't update the map of a hist entry to a NULL map

[PATCH] perf hist: Don't update the map of a hist entry to a NULL map

[Casey Chen]

perf c2c report, including --stats, segfaults on c2c data recorded on
a loaded system:

  map__find_symbol
  hists__findnew_entry
  __hists__add_entry
  process_sample_event      (builtin-c2c.c)
  perf_session__deliver_event
  ordered_events__deliver_event
  __ordered_events__flush
  perf_session__process_user_event
  perf_session__process_events
  perf_c2c__report

When a new sample matches an existing hist entry whose map differs,
hists__findnew_entry() replaces the entry's map with the new sample's
and re-resolves the entry's symbol in it.  The new sample's map is NULL
when its address is not in any known mapping, and
map__find_symbol(NULL, ...) then dereferences it in map__load().

perf c2c sets the sym flag for its symbol and iaddr columns but groups
samples by cacheline, so samples from different code locations, with
and without a map, end up in the same entry.  perf report without a sym
sort key never reaches this code, and perf report and perf script
process the same perf.data without problems.

Skip the update when the new sample has no map.  There is nothing to
update to, and the entry keeps its existing map and symbol instead of
being reset by a sample that did not resolve.

Tested on an x86-64 system (2 sockets, 192 CPUs) with perf c2c data
recorded under heavy load: perf c2c report --stats and
perf c2c report --stdio --full-symbols --no-source -NN segfault without
this change and complete with it.  The data comes from a proprietary
workload and cannot be shared; no synthetic reproducer was produced.

The bug was found and analyzed, and the fix written, with the help of
an AI coding assistant.  The patch was also reviewed with Sashiko, which
reported no issues.  I reviewed and tested the change.

Fixes: ac01c8c42465 ("perf hist: Update hist symbol when updating maps")
Signed-off-by: Casey Chen <cachen@purestorage.com>
---
 tools/perf/util/hist.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 364754e9d12a..d5763199205b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -723,9 +723,12 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 			 * become out-of-date due to an exec() or
 			 * similar, update it.  Otherwise we will
 			 * mis-adjust symbol addresses when computing
-			 * the history counter to increment.
+			 * the history counter to increment.  Skip it when the
+			 * new sample has no map (an unresolved address):
+			 * there is nothing to update to.
 			 */
-			if (hists__has(hists, sym) && he->ms.map != entry->ms.map) {
+			if (hists__has(hists, sym) && entry->ms.map &&
+			    he->ms.map != entry->ms.map) {
 				if (he->ms.sym) {
 					u64 addr = he->ms.sym->start;
 					he->ms.sym = map__find_symbol(entry->ms.map, addr);

base-commit: edd09316bd3784bcae2b3281a892700e272d5d40
-- 
2.34.1

Re: [PATCH] perf hist: Don't update the map of a hist entry to a NULL map

[sashiko-bot]

> perf c2c report, including --stats, segfaults on c2c data recorded on
> a loaded system:
> 
>   map__find_symbol
>   hists__findnew_entry
>   __hists__add_entry
>   process_sample_event      (builtin-c2c.c)
>   perf_session__deliver_event
>   ordered_events__deliver_event
>   __ordered_events__flush
>   perf_session__process_user_event
>   perf_session__process_events
>   perf_c2c__report
> 
> When a new sample matches an existing hist entry whose map differs,
> [ ... ]
> an AI coding assistant.  The patch was also reviewed with Sashiko, which
> reported no issues.  I reviewed and tested the change.
> 
> Fixes: ac01c8c42465 ("perf hist: Update hist symbol when updating maps")
> Signed-off-by: Casey Chen <cachen@purestorage.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111234156.2276500-1-cachen@purestorage.com?part=1