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

[Adrian Hunter <adrian.hunter@intel.com>]

On 03/08/2026 12:06, Amir Ayupov wrote:
> 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

Regrouping by auxtrace_parse_aux_action() is done before group
description is written, so it should match.  What exactly is
wrong with it?

Can you give example perf commands?

> 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.

Seems like just papering over the issue.  Why can't the group
description be written correctly in the first place?

> 
> 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)