[PATCH 1/9] perf header: Tolerate inconsistent HEADER_GROUP_DESC

[Amir Ayupov <aaupov@fb.com>]

process_group_desc() rejects the entire perf.data file ("invalid group
desc" -> "incompatible file format") whenever the group description is
inconsistent with the event list. Group information is optional metadata
and is not needed to decode samples, so a single bad group descriptor
should not make an otherwise valid file unreadable.

This is observable with AUX area recordings (Arm CoreSight ETM, Intel PT)
that use aux-action pause/resume: the aux-action regrouping inflates the
AUX group leader's nr_members, producing a group descriptor that the
strict reader rejects, even though the file is otherwise fine (older perf
and other tooling read it by rebuilding groups from event records).

Warn and fall back to a consistent ungrouped event list instead of
failing the read.

Signed-off-by: Amir Ayupov <aaupov@fb.com>
---
 tools/perf/util/header.c | 42 +++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index e90e541f546b4..ddc59624d639b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3373,6 +3373,19 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 	i = nr = 0;
 	evlist__for_each_entry(session->evlist, evsel) {
 		if (i < nr_groups && evsel->core.idx == (int) desc[i].leader_idx) {
+			if (!desc[i].nr_members)
+				goto out_inconsistent;
+
+			if (nr > 0) {
+				/*
+				 * A new leader was found before the previous
+				 * group's members were all consumed, so the
+				 * group description is inconsistent with the
+				 * event list.
+				 */
+				goto out_inconsistent;
+			}
+
 			evsel__set_leader(evsel, evsel);
 			/* {anon_group} is a dummy name */
 			if (strcmp(desc[i].name, "{anon_group}")) {
@@ -3381,11 +3394,6 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 			}
 			evsel->core.nr_members = desc[i].nr_members;
 
-			if (i >= nr_groups || nr > 0) {
-				pr_debug("invalid group desc\n");
-				goto out_free;
-			}
-
 			leader = evsel;
 			nr = evsel->core.nr_members - 1;
 			i++;
@@ -3397,10 +3405,8 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 		}
 	}
 
-	if (i != nr_groups || nr != 0) {
-		pr_debug("invalid group desc\n");
-		goto out_free;
-	}
+	if (i != nr_groups || nr != 0)
+		goto out_inconsistent;
 
 	ret = 0;
 out_free:
@@ -3409,6 +3415,24 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
 	free(desc);
 
 	return ret;
+
+out_inconsistent:
+	/*
+	 * Group information is optional metadata and is not required to decode
+	 * samples. Rather than rejecting the whole file, warn and fall back to
+	 * a consistent ungrouped event list. This can happen with otherwise
+	 * valid perf.data files, e.g. AUX area (Intel PT, Arm CoreSight ETM)
+	 * recordings using aux-action pause/resume.
+	 */
+	pr_warning("Inconsistent HEADER_GROUP_DESC, ignoring group information\n");
+	env->nr_groups = 0;
+	evlist__for_each_entry(session->evlist, evsel) {
+		evsel__set_leader(evsel, evsel);
+		evsel->core.nr_members = 1;
+		zfree(&evsel->group_name);
+	}
+	ret = 0;
+	goto out_free;
 }
 
 static int process_auxtrace(struct feat_fd *ff, void *data __maybe_unused)
-- 
2.52.0