Re: [PATCH v7 01/59] perf inject: Fix itrace branch stack synthesis

sashiko-bot
2026-04-25T23:29:37Z

Sashiko AI review found 4 potential issue(s):
- [High] Uninitialized `header.misc` field in synthesized branch stack samples corrupts CPU mode metadata.
- [High] Pipeline stream corruption in pipe mode due to missing ATTR header updates.
- [High] Hardcoded `branch_sample_type=0` in auxtrace decoders omits `hw_idx` for PEBS samples, breaking alignment.
- [High] `last_branch` injection bypasses AUX data stripping, causing a mismatch between the payload and the file header.
--

commit f5581930b39cbe639866b1a06c082d518b3d6c69
Author: Ian Rogers <irogers@google.com>

perf inject: Fix itrace branch stack synthesis

When using "perf inject --itrace=L" to synthesize branch stacks from aux data,
failures occurred due to incorrect sample_type and unhandled hardware indices.
This commit fixes sample_type flags during synthesis and explicitly passes
branch_sample_type to properly write the nr and hw_idx fields.

> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index f174bc69cec4..88c0ef4f5ff1 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -375,7 +375,59 @@ static int perf_event__repipe_sample(const struct perf_tool *tool,
>
> build_id__mark_dso_hit(tool, event, sample, evsel, machine);
>
> - if (inject->itrace_synth_opts.set && sample->aux_sample.size) {
> + if (inject->itrace_synth_opts.set &&
> + (inject->itrace_synth_opts.last_branch ||
> + inject->itrace_synth_opts.add_last_branch)) {
> + union perf_event *event_copy = (void *)inject->event_copy;
> + struct branch_stack dummy_bs = { .nr = 0 };
> + int err;
> + size_t sz;
> + u64 orig_type = evsel->core.attr.sample_type;
> + u64 orig_branch_type = evsel->core.attr.branch_sample_type;
> +
> + if (event_copy == NULL) {
> + inject->event_copy = malloc(PERF_SAMPLE_MAX_SIZE);
> + if (!inject->event_copy)
> + return -ENOMEM;
> +
> + event_copy = (void *)inject->event_copy;
> + }

Since event_copy is allocated with malloc(), is it possible that
event_copy->header.misc is left uninitialized?

event_copy->header.type and event_copy->header.size are populated below, but
header.misc seems to be skipped. Could this result in garbage data being
passed for CPU mode and other flags, corrupting downstream symbol resolution?

> +
> + if (!sample->branch_stack)
> + sample->branch_stack = &dummy_bs;
> +
> + if (inject->itrace_synth_opts.add_last_branch) {
> + /* Temporarily add in type bits for synthesis. */
> + evsel->core.attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> + evsel->core.attr.branch_sample_type |= PERF_SAMPLE_BRANCH_HW_INDEX;
> + evsel->core.attr.sample_type &= ~PERF_SAMPLE_AUX;
> + }

If perf inject is run with --itrace=L, last_branch is true but
add_last_branch is false, so PERF_SAMPLE_AUX is not stripped here.

Since __cmd_inject() unconditionally strips PERF_SAMPLE_AUX from the output
session header, could this lead to samples being synthesized and written with
their aux data intact, causing a mismatch between the declared header
attributes and the actual sample payload?

[ ... ]
> @@ -2434,12 +2488,25 @@ static int __cmd_inject(struct perf_inject *inject)
> * synthesized hardware events, so clear the feature flag.
> */
> if (inject->itrace_synth_opts.set) {
> + struct evsel *evsel;
> +
> perf_header__clear_feat(&session->header,
> HEADER_AUXTRACE);
> - if (inject->itrace_synth_opts.last_branch ||
> - inject->itrace_synth_opts.add_last_branch)
> +
> + evlist__for_each_entry(session->evlist, evsel) {
> + evsel->core.attr.sample_type &= ~PERF_SAMPLE_AUX;
> + }
> +
> + if (inject->itrace_synth_opts.add_last_branch) {
> perf_header__set_feat(&session->header,
> HEADER_BRANCH_STACK);
> +
> + evlist__for_each_entry(session->evlist, evsel) {
> + evsel->core.attr.sample_type |= PERF_SAMPLE_BRANCH_STACK;
> + evsel->core.attr.branch_sample_type |=
> + PERF_SAMPLE_BRANCH_HW_INDEX;
> + }
> + }
> }

This block updates the attributes to reflect the new sample payload, but
it is inside an if (!inject->output.is_pipe && !inject->in_place_update)
check.

When pipe mode is used, attributes are emitted inline at the start of the
stream. If the payload is modified in perf_event__repipe_sample() without
the corresponding attribute updates in pipe mode, will downstream pipe
consumers receive initial attributes that do not match the modified payloads?

[ ... ]
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index fc9eec8b54b8..2dce6106c038 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -1731,8 +1731,12 @@ static void intel_pt_prep_b_sample(struct intel_pt *pt,
> static int intel_pt_inject_event(union perf_event *event,
> struct perf_sample *sample, u64 type)
> {
> - event->header.size = perf_event__sample_event_size(sample, type, 0);
> - return perf_event__synthesize_sample(event, type, 0, sample);
> + event->header.type = PERF_RECORD_SAMPLE;
> + event->header.size = perf_event__sample_event_size(sample, type, /*read_format=*/0,
> + /*branch_sample_type=*/0);
> +
> + return perf_event__synthesize_sample(event, type, /*read_format=*/0,
> + /*branch_sample_type=*/0, sample);
> }

Passing 0 for branch_sample_type here hardcodes the synthesis to omit the
hw_idx field.

Since intel_pt_inject_event() is used for synthesizing pebs samples which
inherit the original evsel configuration, what happens if the original pebs
evsel had PERF_SAMPLE_BRANCH_HW_INDEX enabled? Will this cause the payload
to omit hw_idx while the attributes still expect it, misaligning all
subsequent payload reads?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260425224951.174663-1-irogers@google.com?part=1