[PATCH v1 0/12] perf coverage: A harness for perf, and the fixes it found

[PATCH v1 0/12] perf coverage: A harness for perf, and the fixes it found

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Hi,

	Please consider merging,

Nothing measured how much of perf the test suite reaches.  Patch 1 adds
tools/perf/scripts/coverage.sh: it builds perf with clang source based
coverage, runs the complete perf test suite and reports line and function
coverage, optionally against a baseline, with --table and --html like
pahole's coverage_table.py.  Also as 'make -C tools/perf coverage',
'coverage-prep' and 'coverage-table'.

The rest is what the first runs found, in the code, the build and the
tests:

  - clang DWARF 5 describes globals with DW_OP_addrx, which was not
    resolved (patch 2).
  - The flexible array member had no test; one showed the struct row at
    0.00% while the member said 100.00% (patches 3-4).
  - A struct pointer that -O2 keeps in a register was not resolved from
    its DWARF location (patch 5).
  - --insn-stat was printed before the loop that accounts the samples,
    so --code-with-type showed zeros (patch 6).
  - The libbfd probe printed a missing bfd.h error five times per
    build-test run (patch 7).
  - FEATURE-DUMP didn't record llvm-perf and rust, disabling both in the
    build-test builds (patch 8).
  - GCC 16 caught an uninitialized T32 instruction buffer read in cs-etm
    (patch 9).
  - The Intel PT virtual LBR check lost every aux sample with a single
    data page (patch 10).
  - The dlfilter test kept samples whose IP the kernel now zeroes on a
    userspace-to-kernel skid (patch 11).
  - The LBR inline unwinding check failed instead of skipping when the
    AMD PMU rejects branch sampling (patch 12).

In a root run on this box the suite covered 49.16% of perf's 201,180
lines; three tests failed in code this series does not change (the
POLL_HUP tracepoint regression, the AMD BRS branch sampling rejection
and the lock contention count).

The harness and the tests were developed with an AI coding assistant
(OpenCode), which drafted the script and the tests and helped debug the
failures; I reviewed every change and reproduced each failure before
fixing it.

Best regards,

- Arnaldo

Arnaldo Carvalho de Melo (12):
  perf scripts: Add a coverage harness for perf
  perf annotate-data: Resolve DW_OP_addrx global variables
  perf tests: Exercise the flexible array member path
  perf annotate-data: Count flexible array accesses for the whole struct
  perf tests: Exercise data type resolution from a register
  perf annotate: Print the stats after the annotations
  perf tests: Silence the libbfd probe in build-test
  perf build: Record the display-only features in FEATURE-DUMP
  perf cs-etm: Fall back to the recorded instruction size on a failed read
  perf tests: Fix the Intel PT virtual LBR check
  perf test: Drop samples without an IP in the dlfilter test
  perf test: Skip the LBR inline unwinding check when unsupported

 tools/perf/.gitignore                         |   1 +
 tools/perf/Makefile                           |  11 +-
 tools/perf/Makefile.config                    |   4 +
 tools/perf/Makefile.perf                      |   4 +
 tools/perf/builtin-annotate.c                 |  10 +-
 tools/perf/scripts/coverage.sh                | 703 ++++++++++++++++++
 tools/perf/tests/make                         |   2 +-
 tools/perf/tests/shell/addr2line_inlines.sh   |   9 +-
 tools/perf/tests/shell/annotate_stats.sh      |  58 ++
 .../perf/tests/shell/data_type_flex_array.sh  | 142 ++++
 .../tests/shell/data_type_profiling_clang.sh  | 108 +++
 tools/perf/tests/shell/data_type_register.sh  | 120 +++
 tools/perf/tests/shell/script_dlfilter.sh     |   2 +-
 tools/perf/tests/shell/test_intel_pt.sh       |   9 +-
 tools/perf/util/annotate-data.c               |  24 +-
 tools/perf/util/cs-etm.c                      |   7 +-
 tools/perf/util/dwarf-aux.c                   |  42 +-
 17 files changed, 1234 insertions(+), 22 deletions(-)
 create mode 100755 tools/perf/scripts/coverage.sh
 create mode 100755 tools/perf/tests/shell/annotate_stats.sh
 create mode 100755 tools/perf/tests/shell/data_type_flex_array.sh
 create mode 100755 tools/perf/tests/shell/data_type_profiling_clang.sh
 create mode 100755 tools/perf/tests/shell/data_type_register.sh


base-commit: cc905a18a647a8e3ad42cd1596baa0e08fba26eb
-- 
2.56.0

[PATCH v1 01/12] perf scripts: Add a coverage harness for perf

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Build perf with clang source based coverage, run the complete perf test
suite, merge the raw profiles and report line and function coverage for
perf, optionally against a baseline.  This is pahole's
coverage/coverage.sh + coverage-diff.sh merged into one script, wired as
'make -C tools/perf coverage' and listed in 'make help'.

Without -f the report lists the files with the most uncovered lines,
which is where new tests should try to get more coverage; -t narrows
the tests, -f focuses on specific files, --html writes a browsable
report and --table/-T, also available as 'make -C tools/perf
coverage-table', prints the last run as an ASCII table with the
per-file line and function coverage, like pahole's coverage_table.py.

The build can be split from the test run: 'make -C tools/perf
coverage-prep' builds the instrumented perf and records the build
directory, and a later run as root, which needs no make, checks that
build, runs the tests and renders the report, or prints the table with
--table.  This is for running the tests that need root on the bare
metal host while the instrumented perf is built in the development
container.

It is a maintainer tool and it already paid for itself: the first run
found the clang DWARF 5 data type test failing (the DW_OP_addrx gap,
fixed by the next patch), and the uncovered line list is what drove the
tests in the rest of the series.

  $ scripts/coverage.sh
  ...
  Total: 67436 of 201128 lines covered (33.53%)

    largest uncovered files (>= 100 lines)
    file                                            lines     cover    missed  fn-cover
    builtin-trace.c                                  3657     4.24%      3502     0.51%
    util/intel-pt.c                                  3237     0.00%      3237     0.00%
    util/intel-pt-decoder/intel-pt-decoder.c         3208     0.00%      3208     0.00%
    builtin-sched.c                                  3737    19.72%      3000    11.35%
    ui/browsers/hists.c                              2730     0.00%      2730     0.00%
    util/probe-event.c                               2765     5.39%      2616    10.68%
    builtin-script.c                                 3212    29.98%      2249    33.06%
    ...
  Report:  /tmp/build/perf-coverage/cov/report.txt

The run above was unprivileged, so tests that need root are skipped or
fail; the report still points at the gaps.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/.gitignore          |   1 +
 tools/perf/Makefile            |  11 +-
 tools/perf/Makefile.perf       |   4 +
 tools/perf/scripts/coverage.sh | 703 +++++++++++++++++++++++++++++++++
 4 files changed, 718 insertions(+), 1 deletion(-)
 create mode 100755 tools/perf/scripts/coverage.sh

diff --git a/tools/perf/.gitignore b/tools/perf/.gitignore
index 3b968c5158b822ce..9052f25597ac9489 100644
--- a/tools/perf/.gitignore
+++ b/tools/perf/.gitignore
@@ -27,6 +27,7 @@ stats-*.csv
 cscope*
 config.mak
 config.mak.autogen
+.coverage-build-dir
 *-bison.*
 *-flex.*
 *.pyc
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 00baf90e5bda7123..d00b63ca011c2f6a 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -140,6 +140,15 @@ check-headers:
 sync-headers:
 	@./scripts/sync-headers.sh $(SYNC_HEADERS_ARGS)
 
+coverage-prep:
+	@./scripts/coverage.sh --prep $(if $(O),--build-dir=$(O)) $(COVERAGE_ARGS)
+
+coverage:
+	@./scripts/coverage.sh $(COVERAGE_ARGS)
+
+coverage-table:
+	@./scripts/coverage.sh --table $(COVERAGE_ARGS)
+
 #
 # All other targets get passed through:
 #
@@ -147,4 +156,4 @@ sync-headers:
 	$(print_msg)
 	$(make)
 
-.PHONY: all clean tags TAGS FORCE Makefile build-test build-test-tarball check-headers sync-headers
+.PHONY: all clean tags TAGS FORCE Makefile build-test build-test-tarball check-headers sync-headers coverage coverage-prep coverage-table
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index b74b534e1153ffcb..782929b0ba10d8f3 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -873,6 +873,10 @@ help:
 	@echo '  check-headers		- check which header copies are out of sync with the kernel'
 	@echo '  sync-headers		- sync those header copies with the kernel, creating commits'
 	@echo '			  (SYNC_HEADERS_ARGS="..." passes options to the script)'
+	@echo '  coverage-prep		- build perf with clang coverage, ready for a root test run'
+	@echo '  coverage		- build perf with clang coverage, run the tests and report'
+	@echo '  coverage-table	- print an ASCII table of the last coverage run'
+	@echo '			  (COVERAGE_ARGS="..." passes options to the script)'
 
 
 DOC_TARGETS := doc man html info pdf
diff --git a/tools/perf/scripts/coverage.sh b/tools/perf/scripts/coverage.sh
new file mode 100755
index 0000000000000000..19b0da22000c3f0d
--- /dev/null
+++ b/tools/perf/scripts/coverage.sh
@@ -0,0 +1,703 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2026, Red Hat Inc, Arnaldo Carvalho de Melo <acme@redhat.com>
+#
+# LLVM source-based coverage for perf: build with clang instrumentation,
+# run the complete perf test suite and report line/function coverage for
+# perf, optionally comparing it against a previously saved report.  -t
+# narrows the tests and -f adds files to the per-file table; without -f
+# the report lists the files with the most uncovered lines, which is
+# where new tests are needed.  --table prints the last run as an ASCII
+# coverage table, like pahole's coverage_table.py.  The build can be
+# split from the test run: --prep builds the instrumented perf and
+# records the build directory, and a later run as root uses that build,
+# skips the build and goes straight to the tests.  The sampling based
+# tests move the percentages by a percent or two between identical runs.
+
+set -euo pipefail
+
+PERF_DIR=$(cd "$(dirname "$0")/.." && pwd)
+
+DEFAULT_BUILD_DIR=/tmp/build/perf-coverage
+BUILD_DIR=${BUILD_DIR:-}
+COV_DIR=${COV_DIR:-}
+BUILD_DIR_SET=0
+COV_DIR_SET=0
+[ -n "$BUILD_DIR" ] && BUILD_DIR_SET=1
+[ -n "$COV_DIR" ] && COV_DIR_SET=1
+JOBS=${JOBS:-$(nproc 2>/dev/null || echo 1)}
+LLVM_PROFDATA=${LLVM_PROFDATA:-llvm-profdata}
+LLVM_COV=${LLVM_COV:-llvm-cov}
+
+DEBUG_BUILD=1
+SKIP_BUILD=0
+REPORT_ONLY=0
+PREP=0
+TABLE=0
+HTML=0
+USE_BASELINE=1
+BASELINE=
+UNCOVERED=${UNCOVERED:-20}
+TOP=${TOP:-20}
+MIN_LINES=${MIN_LINES:-100}
+DWARF_VERSION=${DWARF_VERSION:-4}
+TESTS=()
+
+FOCUS=()
+
+ROOT=0
+[ "$(id -u)" = 0 ] && ROOT=1
+
+say()  { printf '\033[1;34m==>\033[0m %s\n' "$*"; }
+warn() { printf '\033[0;33mwarning:\033[0m %s\n' "$*" >&2; }
+die()  { printf '\033[0;31merror:\033[0m %s\n' "$*" >&2; exit 1; }
+
+usage()
+{
+	cat <<EOF
+Usage: scripts/coverage.sh [options] [test name fragment...]
+
+Builds perf with clang source-based coverage, runs the complete perf
+test suite and reports coverage for perf.  Use -t to run a subset and
+-f to focus the per-file table.
+
+Options:
+  -b, --build-dir DIR    perf build directory (default: $DEFAULT_BUILD_DIR)
+  -c, --cov-dir DIR      profiles and reports (default: BUILD_DIR/cov)
+  -j, --jobs N           parallel build jobs (default: $JOBS)
+  -t, --tests "A;B"      test name fragments, ';' separated (default: all)
+  -f, --focus FILE       add a file to the per-file table (repeatable)
+      --top N            when no -f is given, show the N files with the
+                         most uncovered lines (default: $TOP, 0 to disable)
+      --baseline FILE    baseline report (default: COV_DIR/baseline.txt)
+      --no-baseline      do not compare against or update a baseline
+      --uncovered N      uncovered line numbers per focus file (default: $UNCOVERED)
+      --skip-build       reuse the existing build, just run the tests
+      --report-only      reuse build and profiles, just render the report
+      --prep             build the instrumented perf and stop; run the
+                         tests later as root with this script
+  -T, --table            print an ASCII table of the last run and exit,
+                         like pahole's coverage_table.py
+      --optimized        build with perf's default -O3 instead of -O0 -g
+      --dwarf-version N  DWARF version for the instrumented build (default: $DWARF_VERSION)
+      --html             also write an HTML report
+  -h, --help             show this help
+
+Environment:
+  BUILD_DIR, COV_DIR, JOBS, LLVM_COV, LLVM_PROFDATA, UNCOVERED, TOP,
+  MIN_LINES, DWARF_VERSION
+
+As root, without --prep, the script skips the build, checks the perf
+prepared by 'make coverage-prep', runs the tests and renders the
+report.  The prepared build is recorded in tools/perf/.coverage-build-dir.
+
+Through make:
+  make -C tools/perf coverage COVERAGE_ARGS="-t 'perf annotate'"
+  make -C tools/perf coverage-prep O=/tmp/build/perf-tools-next
+  make -C tools/perf coverage-table
+
+Examples:
+  scripts/coverage.sh                       # full suite, fresh build
+  scripts/coverage.sh --prep                # build only, test later as root
+  scripts/coverage.sh --report-only         # re-render the last run
+  scripts/coverage.sh --table               # ASCII table of the last run
+  scripts/coverage.sh -t "perf annotate"    # a single test fragment
+  scripts/coverage.sh -f util/evsel.c       # add a file to the table
+EOF
+}
+
+while [ $# -gt 0 ]; do
+	arg=$1
+	optval=
+	case "$arg" in
+	--*=*) optval=${arg#*=}; arg=${arg%%=*} ;;
+	esac
+
+	case "$arg" in
+	-b|--build-dir)   BUILD_DIR=${optval:-${2?missing value}}; BUILD_DIR_SET=1
+	                  [ -z "$optval" ] && shift ;;
+	-c|--cov-dir)     COV_DIR=${optval:-${2?missing value}}; COV_DIR_SET=1
+	                  [ -z "$optval" ] && shift ;;
+	-j|--jobs)        JOBS=${optval:-${2?missing value}}; [ -z "$optval" ] && shift ;;
+	-f|--focus)       FOCUS+=("${optval:-${2?missing value}}"); [ -z "$optval" ] && shift ;;
+	--baseline)       BASELINE=${optval:-${2?missing value}}; [ -z "$optval" ] && shift ;;
+	--uncovered)      UNCOVERED=${optval:-${2?missing value}}; [ -z "$optval" ] && shift ;;
+	--top)            TOP=${optval:-${2?missing value}}; [ -z "$optval" ] && shift ;;
+	--dwarf-version)  DWARF_VERSION=${optval:-${2?missing value}}; [ -z "$optval" ] && shift ;;
+	-t|--tests)       IFS=';' read -r -a _t <<< "${optval:-${2?missing value}}"
+	                  TESTS+=("${_t[@]}"); [ -z "$optval" ] && shift ;;
+	--no-baseline)    USE_BASELINE=0 ;;
+	--skip-build)     SKIP_BUILD=1 ;;
+	--report-only)    REPORT_ONLY=1; SKIP_BUILD=1 ;;
+	--prep)           PREP=1 ;;
+	-T|--table)       TABLE=1; REPORT_ONLY=1; SKIP_BUILD=1 ;;
+	--optimized)      DEBUG_BUILD=0 ;;
+	--html)           HTML=1 ;;
+	-h|--help)        usage; exit 0 ;;
+	-*)               die "unknown option: $arg (see --help)" ;;
+	*)                TESTS+=("$arg") ;;
+	esac
+	shift
+done
+
+#
+# The build directory is recorded by --prep (and by a full run) so a
+# later root run, which does not build, finds it without arguments.
+#
+if [ "$BUILD_DIR_SET" = 0 ] && [ -z "$BUILD_DIR" ] && [ "$PREP" = 0 ] &&
+   { [ "$ROOT" = 1 ] || [ "$SKIP_BUILD" = 1 ] ||
+     [ "$REPORT_ONLY" = 1 ] || [ "$TABLE" = 1 ]; } &&
+   [ -f "$PERF_DIR/.coverage-build-dir" ]; then
+	read -r BUILD_DIR < "$PERF_DIR/.coverage-build-dir"
+fi
+[ -n "$BUILD_DIR" ] || BUILD_DIR=$DEFAULT_BUILD_DIR
+
+if [ "$COV_DIR_SET" = 0 ] && [ -z "$COV_DIR" ] &&
+   [ -f "$BUILD_DIR/cov/state" ]; then
+	COV_DIR=$(awk -F= '$1 == "COV_DIR" { print $2; exit }' \
+		"$BUILD_DIR/cov/state")
+fi
+[ -n "$COV_DIR" ] || COV_DIR=$BUILD_DIR/cov
+
+# The tests run from the kernel tree root, so keep these absolute.
+abspath()
+{
+	case "$1" in
+	/*) printf '%s\n' "$1" ;;
+	*)  printf '%s/%s\n' "$PWD" "$1" ;;
+	esac
+}
+BUILD_DIR=$(abspath "$BUILD_DIR")
+COV_DIR=$(abspath "$COV_DIR")
+
+[ -n "$BASELINE" ] || BASELINE=$COV_DIR/baseline.txt
+[[ $TOP =~ ^[0-9]+$ ]] || die "--top wants a number, not '$TOP'"
+[[ $MIN_LINES =~ ^[0-9]+$ ]] || die "MIN_LINES wants a number, not '$MIN_LINES'"
+
+# Root does not build: use the perf prepared by coverage-prep.
+if [ "$ROOT" = 1 ] && [ "$PREP" = 0 ] && [ "$TABLE" = 0 ]; then
+	SKIP_BUILD=1
+	say "Running as root, using the coverage build in $BUILD_DIR"
+fi
+
+BUILD_LOG=$COV_DIR/build.log
+TEST_LOG=$COV_DIR/tests.log
+REPORT=$COV_DIR/report.txt
+PROFDATA=$COV_DIR/perf.profdata
+# Discovery point for a later root or --table run, so it stays under
+# BUILD_DIR even when --cov-dir points elsewhere.
+STATE_FILE=$BUILD_DIR/cov/state
+POINTER=$PERF_DIR/.coverage-build-dir
+
+#
+# Sanity checks
+#
+[ -d "$PERF_DIR" ] || die "$PERF_DIR not found"
+
+if [ "$TABLE" = 0 ]; then
+	if [ "$PREP" = 0 ]; then
+		command -v "$LLVM_COV" > /dev/null ||
+			die "$LLVM_COV not found (set LLVM_COV)"
+	fi
+	if [ "$SKIP_BUILD" = 0 ]; then
+		command -v clang > /dev/null ||
+			die "clang not found, it is the only compiler with -fcoverage-mapping"
+	fi
+	if [ "$PREP" = 0 ] && [ "$REPORT_ONLY" = 0 ]; then
+		command -v "$LLVM_PROFDATA" > /dev/null ||
+			die "$LLVM_PROFDATA not found (set LLVM_PROFDATA)"
+	fi
+	mkdir -p "$COV_DIR"
+fi
+
+#
+# Build: clang, DEBUG=1 gives -O0 -g like pahole's coverage build, which
+# keeps the line mapping honest at the cost of slower test runs.  DWARF 4
+# is the default; --dwarf-version 5 describes the workloads linked into
+# perf with DW_OP_addrx, the encoding clang uses for globals in DWARF 5.
+#
+check_coverage_build()
+{
+	[ -x "$BUILD_DIR/perf" ] ||
+		die "no $BUILD_DIR/perf, run 'make -C $PERF_DIR O=$BUILD_DIR coverage-prep' first"
+	if command -v nm > /dev/null; then
+		# No -q: grep reads all of nm's output, so pipefail does
+		# not see a SIGPIPE from an early exit.
+		nm "$BUILD_DIR/perf" 2>/dev/null |
+			grep '__llvm_profile' > /dev/null ||
+			die "$BUILD_DIR/perf was not built for coverage, run coverage-prep"
+	else
+		warn "nm not found, cannot verify that $BUILD_DIR/perf is instrumented"
+	fi
+}
+
+write_state()
+{
+	mkdir -p "$(dirname "$STATE_FILE")" ||
+		die "could not create $(dirname "$STATE_FILE")"
+	{
+		printf 'BUILD_DIR=%s\n' "$BUILD_DIR"
+		printf 'COV_DIR=%s\n' "$COV_DIR"
+		printf 'DEBUG_BUILD=%s\n' "$DEBUG_BUILD"
+		printf 'DWARF_VERSION=%s\n' "$DWARF_VERSION"
+	} > "$STATE_FILE" || die "could not write $STATE_FILE"
+
+	if ! printf '%s\n' "$BUILD_DIR" > "$POINTER" 2>/dev/null; then
+		warn "could not write $POINTER, root runs will need -b $BUILD_DIR"
+	fi
+}
+
+build_perf()
+{
+	if [ "$SKIP_BUILD" = 1 ]; then
+		check_coverage_build
+		return
+	fi
+
+	say "Building perf in $BUILD_DIR"
+	mkdir -p "$BUILD_DIR"
+	local debug=()
+	[ "$DEBUG_BUILD" = 1 ] && debug=(DEBUG=1)
+
+	if ! make -C "$PERF_DIR" O="$BUILD_DIR" CC=clang \
+		"${debug[@]+"${debug[@]}"}" \
+		EXTRA_CFLAGS="-fprofile-instr-generate -fcoverage-mapping -gdwarf-$DWARF_VERSION" \
+		-j"$JOBS" > "$BUILD_LOG" 2>&1; then
+		tail -20 "$BUILD_LOG" >&2
+		die "build failed, full log in $BUILD_LOG"
+	fi
+	tail -1 "$BUILD_LOG"
+	write_state
+}
+
+#
+# Run the selected tests with the instrumented perf in PATH: the shell
+# tests, the workloads and 'perf test' itself all come from there.
+#
+run_tests()
+{
+	if [ ${#TESTS[@]} -eq 0 ]; then
+		say "Running the complete perf test suite"
+	else
+		say "Running perf tests: ${TESTS[*]}"
+	fi
+	rm -rf "$COV_DIR/raw"
+	mkdir -p "$COV_DIR/raw"
+
+	set +e
+	# The shell tests only get discovered from the kernel tree root.
+	(cd "$PERF_DIR/../.." &&
+		PATH="$BUILD_DIR:$PATH" \
+		LLVM_PROFILE_FILE="$COV_DIR/raw/perf-%p-%m.profraw" \
+			"$BUILD_DIR/perf" test -v \
+				"${TESTS[@]+"${TESTS[@]}"}") 2>&1 | tee "$TEST_LOG"
+	test_status=${PIPESTATUS[0]}
+	set -e
+
+	# 'perf test' exits 0 even when tests fail, only the summary tells.
+	if [ "$test_status" = 0 ] && grep -qE '^Failed tests +: +[1-9]' "$TEST_LOG"; then
+		test_status=1
+	fi
+
+	grep -E '^(Passed|Skipped|Failed) .*:' "$TEST_LOG" || true
+	[ "$test_status" = 0 ] ||
+		warn "some tests failed or were skipped, coverage is still collected"
+	if [ "$test_status" != 0 ] && grep -q 'Access to performance monitoring' "$TEST_LOG"; then
+		warn "perf_event_paranoid=$(cat /proc/sys/kernel/perf_event_paranoid 2>/dev/null)" \
+		     "denied events to tests recording with callchains;" \
+		     "run as root or lower it for the full suite"
+	fi
+}
+
+#
+# Merge the per-process profiles and render the text report.
+#
+merge_profiles()
+{
+	local count size
+	count=$(find "$COV_DIR/raw" -name '*.profraw' | wc -l)
+	size=$(du -sh "$COV_DIR/raw" 2>/dev/null | cut -f1)
+	[ "$count" -gt 0 ] ||
+		die "no .profraw files, was perf rebuilt with -fprofile-instr-generate?"
+
+	say "Merging $count profile files ($size)"
+	"$LLVM_PROFDATA" merge -sparse "$COV_DIR"/raw/*.profraw -o "$PROFDATA" ||
+		die "llvm-profdata merge failed"
+}
+
+llvm_cov_objects()
+{
+	echo "$BUILD_DIR/perf"
+	local so
+	for so in perf.so libperf.so libperf-jvmti.so; do
+		[ -f "$BUILD_DIR/$so" ] && echo "--object=$BUILD_DIR/$so"
+	done
+}
+
+render_report()
+{
+	local objects
+	mapfile -t objects < <(llvm_cov_objects)
+
+	say "Generating report"
+	"$LLVM_COV" report "${objects[@]}" --instr-profile="$PROFDATA" > "$REPORT"
+}
+
+#
+# Line coverage of a full report for one file: "lines missed cover fn%".
+# Report paths are relative to the build directory, so match the requested
+# path as a suffix: a build outside the source tree gives
+# ".../tools/perf/..." and one inside gives "tools/perf/...".
+#
+file_cov()
+{
+	awk -v f="$2" '
+		function perf_relpath(p) {
+			if (p ~ /(^|\/)tools\/perf\//)
+				sub(/^.*tools\/perf\//, "", p)
+			return p
+		}
+		{
+			g = f
+			sub(/^.*tools\/perf\//, "", g)
+			rel = perf_relpath($1)
+			if (rel == f || rel == g || $1 == f) {
+				print $8, $9, $10, $7
+				exit
+			}
+		}' "$1"
+}
+
+total_cov()
+{
+	awk '$1 == "TOTAL" { print $8, $9, $10 }' "$1"
+}
+
+total_delta()
+{
+	local prev=$1 cur=$2 old cur_cov
+	[ -f "$prev" ] || return 0
+	old=$(total_cov "$prev" | awk '{print $3}')
+	cur_cov=$(total_cov "$cur" | awk '{print $3}')
+	[ -n "$old" ] && [ -n "$cur_cov" ] || return 0
+	awk -v cur="$cur_cov" -v old="$old" 'BEGIN {
+		sub(/%$/, "", cur); sub(/%$/, "", old)
+		printf "%+.2f%%", cur - old }'
+}
+
+report_delta()
+{
+	local prev=$1 cur=$2 label=$3
+	local cur_cov old_cov delta
+	cur_cov=$(file_cov "$cur" "$label" | awk '{print $3}')
+	[ -n "$cur_cov" ] || return 0
+
+	if [ -f "$prev" ]; then
+		old_cov=$(file_cov "$prev" "$label" | awk '{print $3}')
+		if [ -n "$old_cov" ]; then
+			delta=$(awk -v cur="$cur_cov" -v old="$old_cov" 'BEGIN {
+				sub(/%$/, "", cur); sub(/%$/, "", old)
+				printf "%+.2f%%", cur - old }')
+		else
+			delta="(new)"
+		fi
+	else
+		old_cov="-"
+		delta="-"
+	fi
+	printf "  %-8s %8s  %8s  %9s\n" "${label##*/}" "${old_cov:--}" "$cur_cov" "$delta"
+}
+
+#
+# With no -f, show where the uncovered lines are: the files with the most
+# missed lines are where new tests should try to get more coverage.
+#
+print_worst_files()
+{
+	[ "$TOP" -gt 0 ] || return 0
+	local rows missed lines cover fn path
+	rows=$(awk -v min="$MIN_LINES" '
+		$1 ~ /(^|\/)tools\/perf\// && $8 >= min && $8 > 0 {
+			path = $1
+			sub(/^.*tools\/perf\//, "", path)
+			printf "%s %s %s %s %s\n", $9, $8, $10, $7, path
+		}' "$REPORT" | sort -k1,1nr -k2,2nr | sed -n "1,${TOP}p")
+	[ -n "$rows" ] || return 0
+
+	printf "  largest uncovered files (>= %s lines)\n" "$MIN_LINES"
+	printf "  %-44s %8s  %8s  %8s  %8s\n" \
+		"file" "lines" "cover" "missed" "fn-cover"
+	while read -r missed lines cover fn path; do
+		printf "  %-44s %8s  %8s  %8s  %8s\n" \
+			"$path" "$lines" "$cover" "$missed" "$fn"
+	done <<< "$rows"
+}
+
+print_summary()
+{
+	local lines missed cover delta file cov
+	read -r lines missed cover < <(total_cov "$REPORT")
+	printf "\n\033[1mTotal:\033[0m %s of %s lines covered (%s)" \
+		"$((lines - missed))" "$lines" "$cover"
+	if [ "$USE_BASELINE" = 1 ] && [ -f "$BASELINE" ]; then
+		delta=$(total_delta "$BASELINE" "$REPORT")
+		[ -n "$delta" ] && printf "  (previous %s, %s)" \
+			"$(total_cov "$BASELINE" | awk '{print $3}')" "$delta"
+	fi
+	printf "\n\n"
+
+	if [ ${#FOCUS[@]} -eq 0 ]; then
+		print_worst_files
+		return
+	fi
+
+	if [ "$USE_BASELINE" = 1 ] && [ -f "$BASELINE" ]; then
+		printf "  %-8s %8s  %8s  %9s\n" "File" "Previous" "Current" "Delta"
+		printf "  %-8s %8s  %8s  %9s\n" "--------" "--------" "--------" "---------"
+	fi
+
+	printf "  \033[1m%-44s\033[0m %8s  %8s  %8s  %8s\n" \
+		"file" "lines" "cover" "missed" "fn-cover"
+	for file in "${FOCUS[@]}"; do
+		cov=$(file_cov "$REPORT" "$file")
+		if [ -z "$cov" ]; then
+			printf "  %-36s %8s\n" "$file" "not covered"
+			continue
+		fi
+		printf "  %-36s %8s  %6s  %6s  %8s\n" "$file" \
+			"$(echo "$cov" | awk '{print $1}')" \
+			"$(echo "$cov" | awk '{print $3}')" \
+			"$(echo "$cov" | awk '{print $2}')" \
+			"$(echo "$cov" | awk '{print $4}')"
+	done
+
+	if [ "$USE_BASELINE" = 1 ] && [ -f "$BASELINE" ]; then
+		printf "\n  per file delta since %s\n" "$BASELINE"
+		for file in "${FOCUS[@]}"; do
+			report_delta "$BASELINE" "$REPORT" "$file"
+		done
+	fi
+}
+
+#
+# --table: render the report of the last run as an ASCII table, like
+# pahole's coverage_table.py.  Without -f the rows are the TOP files with
+# the most uncovered lines, sorted by line coverage; --top 0 lists every
+# file with at least MIN_LINES lines and uncovered lines.  The total is
+# the whole report, which also counts the libraries perf links with.
+#
+bar()
+{
+	local pct=$1 width=$2 filled i out=
+	filled=$(awk -v p="$pct" -v w="$width" 'BEGIN { printf "%d", p * w / 100 }')
+	for ((i = 0; i < filled; i++)); do out+=█; done
+	for ((i = filled; i < width; i++)); do out+=░; done
+	printf '%s' "$out"
+}
+
+center()
+{
+	local left=$(( ($2 - ${#1}) / 2 ))
+	printf '%*s%s%*s' "$left" '' "$1" "$(( $2 - ${#1} - left ))" ''
+}
+
+dashes()
+{
+	local s
+	printf -v s '%*s' "$1" ''
+	printf '%s' "${s// /─}"
+}
+
+table_row()
+{
+	awk -v f="$2" '
+		function perf_relpath(p) {
+			if (p ~ /(^|\/)tools\/perf\//)
+				sub(/^.*tools\/perf\//, "", p)
+			return p
+		}
+		{
+			g = f
+			sub(/^.*tools\/perf\//, "", g)
+			path = perf_relpath($1)
+			if (path == f || path == g || $1 == f) {
+				printf "%s\t%s\t%d\t%d\t%.2f\t%s\t%d\t%s\n",
+				       path, $8, $8 - $9, $9, $10, $5, $5 - $6, $7
+				exit
+			}
+		}' "$1"
+}
+
+table_rows()
+{
+	if [ ${#FOCUS[@]} -gt 0 ]; then
+		local file
+		for file in "${FOCUS[@]}"; do
+			table_row "$REPORT" "$file"
+		done | sort -t$'\t' -k5,5gr
+		return
+	fi
+
+	awk -v min="$MIN_LINES" '
+		$1 ~ /(^|\/)tools\/perf\// && $8 >= min && $9 > 0 {
+			path = $1
+			sub(/^.*tools\/perf\//, "", path)
+			printf "%s\t%s\t%d\t%d\t%.2f\t%s\t%d\t%s\n",
+			       path, $8, $8 - $9, $9, $10, $5, $5 - $6, $7
+		}' "$REPORT" |
+		sort -t$'\t' -k4,4nr |
+		if [ "$TOP" -gt 0 ]; then sed -n "1,${TOP}p"; else cat; fi |
+		sort -t$'\t' -k5,5gr
+}
+
+print_table()
+{
+	local rows total path lines hit missed pct funcs funcs_hit func_cov
+	local row_fmt w d_file d29 d13 d10 d8
+
+	rows=$(table_rows)
+	[ -n "$rows" ] ||
+		die "no coverage data in $REPORT for the requested files"
+	total=$(awk '$1 == "TOTAL" {
+		printf "%s\t%d\t%d\t%.2f\t%s\t%d\t%s\n",
+		       $8, $8 - $9, $9, $10, $5, $5 - $6, $7
+	}' "$REPORT")
+
+	w=4
+	while IFS=$'\t' read -r path _; do
+		if [ ${#path} -gt "$w" ]; then
+			w=${#path}
+		fi
+	done <<< "$rows"
+
+	d8=$(dashes 8)
+	d10=$(dashes 10)
+	d13=$(dashes 13)
+	d29=$(dashes 29)
+	d_file=$(dashes $((w + 2)))
+
+	printf '┌%s┬%s┬%s┬%s┬%s┬%s┬%s┐\n' \
+		"$d_file" "$d8" "$d8" "$d8" "$d29" "$d13" "$d10"
+	printf '│ %-*s │ %6s │ %6s │ %6s │ %s │ %11s │ %8s │\n' \
+		"$w" "File" "Lines" "Hit" "Miss" \
+		"$(center 'Line Coverage' 27)" "Funcs" "FuncCov"
+	printf '├%s┼%s┼%s┼%s┼%s┼%s┼%s┤\n' \
+		"$d_file" "$d8" "$d8" "$d8" "$d29" "$d13" "$d10"
+
+	row_fmt=$(printf '│ %%-%ds │ %%6s │ %%6s │ %%6s │ %%s %%5.1f%%%%' "$w")
+	row_fmt+=' │ %5s/%-5s │ %8s │\n'
+
+	while IFS=$'\t' read -r path lines hit missed pct funcs funcs_hit func_cov; do
+		printf "$row_fmt" "$path" "$lines" "$hit" "$missed" \
+			"$(bar "$pct" 20)" "$pct" "$funcs_hit" "$funcs" "$func_cov"
+	done <<< "$rows"
+
+	printf '├%s┼%s┼%s┼%s┼%s┼%s┼%s┤\n' \
+		"$d_file" "$d8" "$d8" "$d8" "$d29" "$d13" "$d10"
+	while IFS=$'\t' read -r lines hit missed pct funcs funcs_hit func_cov; do
+		printf "$row_fmt" "TOTAL" "$lines" "$hit" "$missed" \
+			"$(bar "$pct" 20)" "$pct" "$funcs_hit" "$funcs" "$func_cov"
+	done <<< "$total"
+	printf '└%s┴%s┴%s┴%s┴%s┴%s┴%s┘\n' \
+		"$d_file" "$d8" "$d8" "$d8" "$d29" "$d13" "$d10"
+}
+
+print_uncovered()
+{
+	[ "$UNCOVERED" -gt 0 ] || return 0
+	[ ${#FOCUS[@]} -gt 0 ] || return 0
+	local objects file out lines header=0
+	mapfile -t objects < <(llvm_cov_objects)
+
+	for file in "${FOCUS[@]}"; do
+		# No report row means the file was not built: print_summary()
+		# already said so, llvm-cov show would dump the whole profile.
+		[ -n "$(file_cov "$REPORT" "$file")" ] || continue
+		out=$("$LLVM_COV" show "${objects[@]}" --instr-profile="$PROFDATA" \
+			"$PERF_DIR/$file" 2>/dev/null |
+			awk -F'|' '$2 ~ /^[[:space:]]*0[[:space:]]*$/ {
+				gsub(/[[:space:]]/, "", $1); printf "%s ", $1 }')
+		lines=$(echo "$out" | wc -w)
+		[ "$lines" -gt 0 ] || continue
+		if [ "$header" = 0 ]; then
+			printf "\n  uncovered lines (first %s per file)\n" "$UNCOVERED"
+			header=1
+		fi
+		printf "  %-24s %4s lines: %s\n" "$file" "$lines" \
+			"$(echo "$out" | cut -d' ' -f1-"$UNCOVERED")"
+	done
+}
+
+save_html()
+{
+	[ "$HTML" = 1 ] || return 0
+	local objects
+	mapfile -t objects < <(llvm_cov_objects)
+	say "Writing HTML report to $COV_DIR/html"
+	"$LLVM_COV" show "${objects[@]}" --instr-profile="$PROFDATA" \
+		--format=html --output-dir="$COV_DIR/html" \
+		--show-line-counts-or-regions --show-branches=count
+}
+
+check_stale()
+{
+	[ "$REPORT_ONLY" = 1 ] || return 0
+	if [ ! -f "$PROFDATA" ]; then
+		[ "$TABLE" = 1 ] && return 0
+		die "no $PROFDATA, run without --report-only first"
+	fi
+	local newer
+	newer=$(find "$PERF_DIR" -name '*.c' -newer "$PROFDATA" -print -quit 2>/dev/null)
+	if [ -n "$newer" ]; then
+		warn "profiles are older than source changes (e.g. $newer), results may be stale"
+	fi
+}
+
+update_baseline()
+{
+	[ "$USE_BASELINE" = 1 ] || return 0
+	[ "$REPORT_ONLY" = 1 ] && return 0
+	[ -f "$BASELINE" ] && mv "$BASELINE" "$BASELINE.prev"
+	cp "$REPORT" "$BASELINE"
+	printf "\nBaseline saved: %s\n" "$BASELINE"
+}
+
+test_status=0
+
+if [ "$TABLE" = 1 ]; then
+	[ -f "$REPORT" ] || die "no $REPORT, run coverage first"
+	check_stale
+	print_table
+	exit 0
+fi
+
+if [ "$PREP" = 1 ]; then
+	build_perf
+	write_state
+	say "Coverage build ready: $BUILD_DIR/perf"
+	printf "As root: %s/scripts/coverage.sh\n" "$PERF_DIR"
+	exit 0
+fi
+
+if [ "$REPORT_ONLY" = 0 ]; then
+	build_perf
+	run_tests
+	merge_profiles
+else
+	check_coverage_build
+	check_stale
+fi
+
+[ -f "$PROFDATA" ] || die "no $PROFDATA, run without --report-only first"
+
+render_report
+print_summary
+print_uncovered
+save_html
+update_baseline
+
+printf "\nReport:  %s\n" "$REPORT"
+[ "$HTML" = 1 ] && printf "HTML:    %s\n" "$COV_DIR/html/index.html"
+
+exit "$test_status"
-- 
2.56.0

[PATCH v1 02/12] perf annotate-data: Resolve DW_OP_addrx global variables

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

clang emits DW_AT_location with DW_OP_addrx for global variables in
DWARF 5, gcc emits DW_OP_addr.  The address index was not resolved, so
those variables were not found by address; the by-name fallback also
missed the ones in .bss past the file backed end of the last segment.

Resolve the index with dwarf_getlocation_attr() + dwarf_formaddr() and
use the resulting address in the by-address lookup and in the global
variable cache.  Add a test that compiles the workload with clang
-gdwarf-5 and checks that the global is annotated.

Fixes: d60469d7c0e5c4e8 ("perf dwarf-aux: Add die_find_variable_by_addr()")
Fixes: d7b60803a790b716 ("perf dwarf-aux: Add die_collect_global_vars()")
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 .../tests/shell/data_type_profiling_clang.sh  | 108 ++++++++++++++++++
 tools/perf/util/dwarf-aux.c                   |  42 ++++++-
 2 files changed, 145 insertions(+), 5 deletions(-)
 create mode 100755 tools/perf/tests/shell/data_type_profiling_clang.sh

diff --git a/tools/perf/tests/shell/data_type_profiling_clang.sh b/tools/perf/tests/shell/data_type_profiling_clang.sh
new file mode 100755
index 0000000000000000..8e425952d4742a2c
--- /dev/null
+++ b/tools/perf/tests/shell/data_type_profiling_clang.sh
@@ -0,0 +1,108 @@
+#!/bin/bash
+# perf data type profiling tests with clang DWARF 5
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+# clang describes the location of globals with DW_OP_addrx where gcc
+# uses DW_OP_addr, so the workload has to be built here to get that
+# encoding.
+
+command -v clang > /dev/null || exit 2
+
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+perfout=$(mktemp /tmp/__perf_test.perf.out.XXXXX)
+prog=$(mktemp /tmp/__perf_test.clang.XXXXX)
+src="${prog}.c"
+
+cleanup()
+{
+	rm -f "$perfdata" "$perfout" "$prog" "$src"
+	trap - EXIT TERM INT
+}
+
+trap_cleanup()
+{
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+# No point in recording if the PMU cannot do memory events.
+if perf mem record -o /dev/null -- true 2>&1 | \
+		grep -q "failed: no PMU supports the memory events"; then
+	cleanup
+	exit 2
+fi
+if ! perf mem record -o /dev/null -- true 2>/dev/null; then
+	echo "Skip: cannot record memory events on this PMU"
+	cleanup
+	exit 2
+fi
+
+cat > "$src" <<'EOF'
+#include <signal.h>
+#include <unistd.h>
+
+struct conn {
+	int saddr;
+	int daddr;
+	long bytes;
+};
+
+static volatile struct conn conn;
+static volatile sig_atomic_t done;
+
+static void stop(int sig)
+{
+	done = 1;
+}
+
+int main(void)
+{
+	unsigned long sum = 0;
+	int i = 0;
+
+	signal(SIGALRM, stop);
+	alarm(1);
+
+	while (!done) {
+		conn.saddr = i;
+		conn.daddr = i + 1;
+		sum += conn.bytes;
+		i++;
+	}
+	return sum != 0;
+}
+EOF
+
+# -fPIE keeps the accesses PC-relative on x86_64, the operand form the
+# data type resolver handles.
+if ! clang -O0 -gdwarf-5 -fPIE -pie -o "$prog" "$src"; then
+	echo "Skip: cannot build the workload with clang"
+	cleanup
+	exit 2
+fi
+
+if ! perf mem record -o "$perfdata" -- "$prog" > /dev/null 2>&1; then
+	echo "FAIL: perf mem record"
+	cleanup
+	exit 1
+fi
+
+if ! perf annotate --code-with-type -i "$perfdata" --stdio \
+		--percent-limit 1 > "$perfout" 2>/dev/null; then
+	echo "FAIL: perf annotate"
+	cleanup
+	exit 1
+fi
+
+if ! grep -q "# data-type: struct conn" "$perfout"; then
+	echo "FAIL: struct conn not resolved, DW_OP_addrx not handled?"
+	cleanup
+	exit 1
+fi
+
+echo "DW_OP_addrx global resolved"
+cleanup
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 48a11c95a67686e4..5db7458f01116a4e 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -14,6 +14,11 @@
 #include "strbuf.h"
 #include "string2.h"
 
+/* DWARF 5 address index, libdw's dwarf_getlocations() returns it unresolved */
+#ifndef DW_OP_addrx
+#define DW_OP_addrx 0xa1
+#endif
+
 /**
  * cu_find_realpath - Find the realpath of the target file
  * @cu_die: A DIE(dwarf information entry) of CU(compilation Unit)
@@ -1629,6 +1634,31 @@ Dwarf_Die *die_find_variable_by_reg(Dwarf_Die *sc_die, Dwarf_Addr pc, int reg,
 	return result;
 }
 
+/*
+ * Get the absolute address of a location expression that consists of a
+ * single address, either DW_OP_addr or DW_OP_addrx.  libdw leaves the
+ * DWARF 5 address index unresolved, it has to be looked up in .debug_addr.
+ */
+static bool die_get_location_addr(Dwarf_Attribute *attr, Dwarf_Op *ops,
+				  Dwarf_Addr *addr)
+{
+	if (ops->atom == DW_OP_addr) {
+		*addr = ops->number;
+		return true;
+	}
+
+	if (ops->atom == DW_OP_addrx) {
+		Dwarf_Attribute addr_attr;
+
+		if (dwarf_getlocation_attr(attr, ops, &addr_attr) != 0)
+			return false;
+
+		return dwarf_formaddr(&addr_attr, addr) == 0;
+	}
+
+	return false;
+}
+
 /* Only checks direct child DIEs in the given scope */
 static int __die_find_var_addr_cb(Dwarf_Die *die_mem, void *arg)
 {
@@ -1647,11 +1677,13 @@ static int __die_find_var_addr_cb(Dwarf_Die *die_mem, void *arg)
 		return DIE_FIND_CB_SIBLING;
 
 	while ((off = dwarf_getlocations(&attr, off, &base, &start, &end, &ops, &nops)) > 0) {
-		if (ops->atom != DW_OP_addr)
+		Dwarf_Addr addr;
+
+		if (!die_get_location_addr(&attr, ops, &addr))
 			continue;
 
 		if (check_allowed_ops(ops, nops) &&
-		    match_var_offset(die_mem, data, data->addr, ops->number,
+		    match_var_offset(die_mem, data, data->addr, addr,
 				     /*is_pointer=*/false))
 			return DIE_FIND_CB_END;
 	}
@@ -1777,7 +1809,7 @@ static int __die_collect_global_vars_cb(Dwarf_Die *die_mem, void *arg)
 	Dwarf_Die type_die;
 	int tag = dwarf_tag(die_mem);
 	Dwarf_Attribute attr;
-	Dwarf_Addr base, start, end;
+	Dwarf_Addr base, start, end, addr;
 	Dwarf_Op *ops;
 	size_t nops;
 	struct die_var_type *vt;
@@ -1792,7 +1824,7 @@ static int __die_collect_global_vars_cb(Dwarf_Die *die_mem, void *arg)
 	if (dwarf_getlocations(&attr, 0, &base, &start, &end, &ops, &nops) <= 0)
 		return DIE_FIND_CB_SIBLING;
 
-	if (ops->atom != DW_OP_addr)
+	if (!die_get_location_addr(&attr, ops, &addr))
 		return DIE_FIND_CB_SIBLING;
 
 	if (!check_allowed_ops(ops, nops))
@@ -1808,7 +1840,7 @@ static int __die_collect_global_vars_cb(Dwarf_Die *die_mem, void *arg)
 	vt->die_off = dwarf_dieoffset(&type_die);
 	vt->die_tag = dwarf_tag(&type_die);
 	vt->from_alt = !die_same_file(die_mem, &type_die);
-	vt->addr = ops->number;
+	vt->addr = addr;
 	vt->end = 0;
 	vt->has_range = false;
 	vt->reg = -1;
-- 
2.56.0

[PATCH v1 03/12] perf tests: Exercise the flexible array member path

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

No test covered find_flex_array() and get_flex_array_member().  Add one
that keeps a struct with a trailing data[] behind a global pointer and
checks that accesses past the struct size are resolved and named after
the flexible array member.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 .../perf/tests/shell/data_type_flex_array.sh  | 104 ++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100755 tools/perf/tests/shell/data_type_flex_array.sh

diff --git a/tools/perf/tests/shell/data_type_flex_array.sh b/tools/perf/tests/shell/data_type_flex_array.sh
new file mode 100755
index 0000000000000000..983cde2b3c7d2b29
--- /dev/null
+++ b/tools/perf/tests/shell/data_type_flex_array.sh
@@ -0,0 +1,104 @@
+#!/bin/bash
+# perf data type profiling tests for flexible array members
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+perfout=$(mktemp /tmp/__perf_test.perf.out.XXXXX)
+prog=$(mktemp /tmp/__perf_test.flex.XXXXX)
+src="${prog}.c"
+
+cleanup()
+{
+	rm -f "$perfdata" "$perfout" "$prog" "$src"
+	trap - EXIT TERM INT
+}
+
+trap_cleanup()
+{
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+# No point in recording if the PMU cannot do memory events.
+if perf mem record -o /dev/null -- true 2>&1 | \
+		grep -q "failed: no PMU supports the memory events"; then
+	cleanup
+	exit 2
+fi
+if ! perf mem record -o /dev/null -- true 2>/dev/null; then
+	echo "Skip: cannot record memory events on this PMU"
+	cleanup
+	exit 2
+fi
+
+cat > "$src" <<'EOF'
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+struct flex {
+	int len;
+	char data[];
+};
+
+static struct flex *gp;
+static volatile sig_atomic_t done;
+
+static void stop(int sig)
+{
+	done = 1;
+}
+
+int main(void)
+{
+	unsigned long sum = 0;
+
+	gp = calloc(1, sizeof(*gp) + 4096);
+	if (gp == NULL)
+		return 1;
+	gp->len = 4096;
+
+	signal(SIGALRM, stop);
+	alarm(1);
+
+	while (!done) {
+		sum += gp->data[3];
+		gp->data[5] = 1;
+	}
+	return sum != 0;
+}
+EOF
+
+# -fPIE keeps the global pointer load PC-relative on x86_64, so the
+# resolver can tell the register holds a struct flex pointer.
+if ! cc -O0 -g -fPIE -pie -o "$prog" "$src" 2>/dev/null; then
+	echo "Skip: cannot build the workload"
+	cleanup
+	exit 2
+fi
+
+if ! perf mem record -o "$perfdata" -- "$prog" > /dev/null 2>&1; then
+	echo "FAIL: perf mem record"
+	cleanup
+	exit 1
+fi
+
+if ! perf annotate --code-with-type -i "$perfdata" --stdio \
+		--percent-limit 1 > "$perfout" 2>/dev/null; then
+	echo "FAIL: perf annotate"
+	cleanup
+	exit 1
+fi
+
+if ! grep -Fq "# data-type: struct flex +0x4 (data)" "$perfout"; then
+	echo "FAIL: flexible array member not resolved"
+	cleanup
+	exit 1
+fi
+
+echo "flexible array member resolved"
+cleanup
-- 
2.56.0

[PATCH v1 04/12] perf annotate-data: Count flexible array accesses for the whole struct

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The per-member percentage sums the histogram entries in the member's
own [offset, offset + size) range.  A flexible array member is resolved
past the end of its struct, so the struct row only covered its declared
size and showed 0.00% while the member row showed 100.00% and the
header said N samples.

Take a member's extent as the end of its last child as well, so the
struct counts what was resolved past it.  Union members all start at
offset 0, so a plain union keeps its size; a member that is itself a
struct with a flexible array extends the union too.

Extend tests/shell/data_type_flex_array.sh to check that the accesses
count for the struct in the percent, samples and period columns.

Fixes: aaca646915a1bb53 ("perf annotate-data: Adjust type offset for flex-array")
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 .../perf/tests/shell/data_type_flex_array.sh  | 40 ++++++++++++++++++-
 tools/perf/util/annotate-data.c               | 24 ++++++++++-
 2 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/data_type_flex_array.sh b/tools/perf/tests/shell/data_type_flex_array.sh
index 983cde2b3c7d2b29..7a7a782320afee23 100755
--- a/tools/perf/tests/shell/data_type_flex_array.sh
+++ b/tools/perf/tests/shell/data_type_flex_array.sh
@@ -100,5 +100,43 @@ if ! grep -Fq "# data-type: struct flex +0x4 (data)" "$perfout"; then
 	exit 1
 fi
 
-echo "flexible array member resolved"
+# The accesses to the flexible array are resolved past the end of the
+# struct: they must count for the struct, not only for the member.
+if ! perf annotate --data-type -i "$perfdata" --stdio > "$perfout" 2>/dev/null; then
+	echo "FAIL: perf annotate --data-type"
+	cleanup
+	exit 1
+fi
+
+if ! awk '
+	$1 + 0 == 100.00 && /struct flex/ { struct_ok = 1 }
+	$1 + 0 == 100.00 && /char\[\]/ && /data;/ { data_ok = 1 }
+	$1 + 0 == 0.00 && /int/ && /len;/ { len_ok = 1 }
+	END { exit !(struct_ok && data_ok && len_ok) }
+' <(sed -n "/^Annotate type: 'struct flex' in/,/^Annotate type: /p" "$perfout"); then
+	echo "FAIL: --data-type does not count the flexible array accesses"
+	cleanup
+	exit 1
+fi
+
+# Same with the samples and period columns, the other two display modes.
+for mode in --show-nr-samples --show-total-period; do
+	if ! perf annotate --data-type -i "$perfdata" --stdio "$mode" > "$perfout" 2>/dev/null; then
+		echo "FAIL: perf annotate --data-type $mode"
+		cleanup
+		exit 1
+	fi
+
+	if ! awk '
+		$1 + 0 > 0 && /struct flex/ { total = $1 + 0 }
+		/char\[\]/ && /data;/ { data = $1 + 0 }
+		END { exit !(total > 0 && total == data) }
+	' <(sed -n "/^Annotate type: 'struct flex' in/,/^Annotate type: /p" "$perfout"); then
+		echo "FAIL: --data-type $mode does not count the flexible array accesses"
+		cleanup
+		exit 1
+	fi
+done
+
+echo "flexible array member resolved and counted"
 cleanup
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index dd9c1d7b2da8be8a..7fae186ffd76b533 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -2233,6 +2233,25 @@ static void print_annotated_data_value(struct type_hist *h, u64 period, int nr_s
 		color_fprintf(stdout, color, " %7.2f", percent);
 }
 
+/*
+ * A flexible array member is resolved past the end of its parent, so the
+ * parent's samples are not bounded by its size.
+ */
+static int member_extent(struct annotated_member *member)
+{
+	struct annotated_member *child;
+	int end = member->offset + member->size;
+
+	list_for_each_entry(child, &member->children, node) {
+		int child_end = member_extent(child);
+
+		if (child_end > end)
+			end = child_end;
+	}
+
+	return end;
+}
+
 static void print_annotated_data_type(struct annotated_data_type *mem_type,
 				      struct annotated_member *member,
 				      struct evsel *evsel, int indent)
@@ -2240,6 +2259,7 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
 	struct annotated_member *child;
 	struct type_hist *h;
 	int i, nr_events = 0, samples = 0;
+	int end = member_extent(member);
 	u64 period = 0;
 	int width = symbol_conf.show_total_period ? 11 : 7;
 	struct evsel *pos;
@@ -2253,10 +2273,10 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
 
 		samples = 0;
 		period = 0;
-		for (i = 0; i < member->size; i++) {
+		for (i = member->offset; i < end; i++) {
 			struct type_hist_entry *entry;
 
-			if (!hashmap__find(&h->samples, member->offset + i, &entry))
+			if (!hashmap__find(&h->samples, i, &entry))
 				continue;
 
 			samples += entry->nr_samples;
-- 
2.56.0

[PATCH v1 05/12] perf tests: Exercise data type resolution from a register

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

At -O2 a struct pointer lives in a register and DWARF describes it with
a register location, while the unoptimized workloads the data type tests
profile keep their variables on the stack.

Build a workload with a noinline function that gets the pointer from a
volatile global, which keeps it in a register, and check that its three
member accesses are resolved from that location.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/shell/data_type_register.sh | 120 +++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100755 tools/perf/tests/shell/data_type_register.sh

diff --git a/tools/perf/tests/shell/data_type_register.sh b/tools/perf/tests/shell/data_type_register.sh
new file mode 100755
index 0000000000000000..e0369f076ae6f4b2
--- /dev/null
+++ b/tools/perf/tests/shell/data_type_register.sh
@@ -0,0 +1,120 @@
+#!/bin/bash
+# perf data type profiling tests for register variables
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+perfout=$(mktemp /tmp/__perf_test.perf.out.XXXXX)
+prog=$(mktemp /tmp/__perf_test.regs.XXXXX)
+src="${prog}.c"
+
+cleanup()
+{
+	rm -f "$perfdata" "$perfout" "$prog" "$src"
+	trap - EXIT TERM INT
+}
+
+trap_cleanup()
+{
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+if ! perf check feature -q dwarf; then
+	echo "Skip: no DWARF (libdw) support"
+	cleanup
+	exit 2
+fi
+
+# No point in recording if the PMU cannot do memory events.
+if perf mem record -o /dev/null -- true 2>&1 | \
+		grep -q "failed: no PMU supports the memory events"; then
+	cleanup
+	exit 2
+fi
+if ! perf mem record -o /dev/null -- true 2>/dev/null; then
+	echo "Skip: cannot record memory events on this PMU"
+	cleanup
+	exit 2
+fi
+
+cat > "$src" <<'EOF'
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+struct node {
+	long a;
+	long b;
+	long c;
+};
+
+static struct node n;
+static struct node *volatile vp;
+static volatile sig_atomic_t done;
+
+static void stop(int sig)
+{
+	done = 1;
+}
+
+__attribute__((noinline)) static long touch(struct node *p)
+{
+	long sum = 0;
+
+	sum += p->a;
+	p->b = sum;
+	sum += p->c;
+	return sum;
+}
+
+int main(void)
+{
+	unsigned long sum = 0;
+
+	vp = &n;
+	signal(SIGALRM, stop);
+	alarm(1);
+
+	while (!done)
+		sum += touch(vp);
+
+	return sum != 0;
+}
+EOF
+
+# -O2: the pointer stays in a register, so the accesses are resolved
+# from its type state instead of from a stack slot.
+if ! cc -O2 -g -fPIE -pie -o "$prog" "$src" 2>/dev/null; then
+	echo "Skip: cannot build the workload"
+	cleanup
+	exit 2
+fi
+
+if ! perf mem record -o "$perfdata" -- "$prog" > /dev/null 2>&1; then
+	echo "FAIL: perf mem record"
+	cleanup
+	exit 1
+fi
+
+if ! perf annotate --code-with-type -i "$perfdata" --stdio \
+		--percent-limit 1 > "$perfout" 2>/dev/null; then
+	echo "FAIL: perf annotate"
+	cleanup
+	exit 1
+fi
+
+for member in "+0 (a)" "+0x8 (b)" "+0x10 (c)"; do
+	if ! grep -Fq "# data-type: struct node ${member}" "$perfout"; then
+		echo "FAIL: member ${member} not resolved from the register"
+		cleanup
+		exit 1
+	fi
+done
+
+echo "register held access resolved"
+
+cleanup
-- 
2.56.0

[PATCH v1 06/12] perf annotate: Print the stats after the annotations

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The instruction stats are printed before the loop that accounts them,
so 'perf annotate --code-with-type --insn-stat' always shows zeros.
The data type counters are updated in --code-with-type mode too, but
only --data-type mode accounts their total, so they are incomplete
there.

Print both after the annotation loops, the data type part only with
--data-type.

Fixes: 58824fa0087e1cb7 ("perf annotate: Add --insn-stat option for debugging")
Fixes: 61a9741e9f78c64c ("perf annotate: Add --type-stat option for debugging")
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c            | 10 ++--
 tools/perf/tests/shell/annotate_stats.sh | 58 ++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100755 tools/perf/tests/shell/annotate_stats.sh

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 331adbdbd635917d..973f4950abd6f917 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -414,11 +414,6 @@ static void hists__find_annotations(struct hists *hists,
 	struct rb_node *nd = rb_first_cached(&hists->entries), *next;
 	int key = K_RIGHT;
 
-	if (ann->type_stat)
-		print_annotate_data_stat(&ann_data_stat);
-	if (ann->insn_stat)
-		print_annotate_item_stat(&ann_insn_stat, "Instruction");
-
 	while (nd) {
 		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
 		struct annotation *notes;
@@ -660,6 +655,11 @@ static int __cmd_annotate(struct perf_annotate *ann)
 		hists__find_annotations(hists, pos, ann);
 	}
 
+	if (ann->data_type && ann->type_stat)
+		print_annotate_data_stat(&ann_data_stat);
+	if (ann->insn_stat)
+		print_annotate_item_stat(&ann_insn_stat, "Instruction");
+
 	if (use_browser == 2) {
 		void (*show_annotations)(void);
 
diff --git a/tools/perf/tests/shell/annotate_stats.sh b/tools/perf/tests/shell/annotate_stats.sh
new file mode 100755
index 0000000000000000..82d2becdff141138
--- /dev/null
+++ b/tools/perf/tests/shell/annotate_stats.sh
@@ -0,0 +1,58 @@
+#!/bin/bash
+# perf annotate instruction stats test
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+perfout=$(mktemp /tmp/__perf_test.perf.out.XXXXX)
+
+cleanup()
+{
+	rm -f "$perfdata" "$perfout"
+	trap - EXIT TERM INT
+}
+
+trap_cleanup()
+{
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+# No point in recording if the PMU cannot do memory events.
+if perf mem record -o /dev/null -- true 2>&1 | \
+		grep -q "failed: no PMU supports the memory events"; then
+	cleanup
+	exit 2
+fi
+if ! perf mem record -o /dev/null -- true 2>/dev/null; then
+	echo "Skip: cannot record memory events on this PMU"
+	cleanup
+	exit 2
+fi
+
+if ! perf mem record -o "$perfdata" -- perf test -w datasym > /dev/null 2>&1; then
+	echo "FAIL: perf mem record"
+	cleanup
+	exit 1
+fi
+
+if ! perf annotate --code-with-type --insn-stat -i "$perfdata" --stdio \
+		--percent-limit 1 > "$perfout" 2>/dev/null; then
+	echo "FAIL: perf annotate"
+	cleanup
+	exit 1
+fi
+
+# The instruction stats are accounted while the instructions are
+# displayed; printing them before the display loop shows zeros.
+if ! grep -Eq '^total [1-9][0-9]*,' "$perfout"; then
+	echo "FAIL: instruction stats not accounted"
+	cleanup
+	exit 1
+fi
+
+echo "instruction stats accounted"
+cleanup
-- 
2.56.0

[PATCH v1 07/12] perf tests: Silence the libbfd probe in build-test

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

build-test evaluates the libbfd probe at parse time and the compiler's
stderr isn't captured by $(shell ...), so without binutils-devel it
prints "bfd.h: No such file or directory" once per tests/make
invocation, five times per run.

Redirect the compiler's stderr; the empty result still skips the
nondistro tests as intended.

Fixes: 523471c5163659c6 ("perf build: Skip nondistro build test if libbfd is old")
Cc: James Clark <james.clark@linaro.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/make | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index 7524de5228029e7e..e6d7ac49b5b1693d 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -128,7 +128,7 @@ make_minimal        += NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
 make_minimal        += NO_CAPSTONE=1
 
 # binutils 2_42 and newer have bfd_thread_init()
-new_libbfd := $(shell echo '#include <bfd.h>' | $(CC) -E -x c - | grep bfd_thread_init)
+new_libbfd := $(shell echo '#include <bfd.h>' | $(CC) -E -x c - 2>/dev/null | grep bfd_thread_init)
 
 # Whether libLLVM can be built, which needs the llvm-devel/llvm-dev headers and
 # libraries of version 13 or newer, not just llvm-config. This mirrors the
-- 
2.56.0

[PATCH v1 08/12] perf build: Record the display-only features in FEATURE-DUMP

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

llvm-perf and rust are in FEATURE_DISPLAY but not in FEATURE_TESTS: they
can't be part of the test-all fast path, which sets every FEATURE_TESTS
entry to 1 when test-all.c builds.  Since FEATURE-DUMP is written from
FEATURE_TESTS only, a FEATURES_DUMP= build, as used by build-test, never
sees them and silently builds without libLLVM and the rust workloads.

Append the display features that aren't in FEATURE_TESTS to the dump
after the regular feature checks have run.

Fixes: 206dcfca1f5cfac3 ("perf build: Autodetect minimum required llvm-dev version")
Fixes: 3f5dfa472ea6771c ("tools build: Fix rust feature detection")
Cc: James Clark <james.clark@linaro.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.config | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index b4764f69f47f4d8c..1b5caa246345c9bb 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1310,3 +1310,7 @@ endif
 # tests.
 $(shell rm -f $(FEATURE_DUMP_FILENAME))
 $(foreach feat,$(FEATURE_TESTS),$(shell echo "$(call feature_assign,$(feat))" >> $(FEATURE_DUMP_FILENAME)))
+# Features only in FEATURE_DISPLAY aren't part of the test-all fast path, so
+# they have to be checked explicitly and recorded here too, otherwise a
+# FEATURES_DUMP= build silently disables them (llvm-perf, rust).
+$(foreach feat,$(filter-out $(FEATURE_TESTS),$(FEATURE_DISPLAY)),$(shell echo "$(call feature_assign,$(feat))" >> $(FEATURE_DUMP_FILENAME)))
-- 
2.56.0

[PATCH v1 09/12] perf cs-etm: Fall back to the recorded instruction size on a failed read

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

cs_etm__t32_instr_size() reads the instruction bytes from the traced
program's memory to tell a 16-bit from a 32-bit T32 instruction, but it
ignores cs_etm__frontend_mem_access() failing, which happens when the
DSO or its debug data is missing, and then reads the uninitialized
buffer.  GCC 16 warns about it and build-test fails with -Werror.

Use the instruction size recorded by the decoder, as the A32/A64 path
already does, when the read doesn't return the full two bytes.

Fixes: a7ee4d625ede4f62 ("perf cs-etm: Support for ARM A32/T32 instruction sets in CoreSight trace")
Cc: James Clark <james.clark@linaro.org>
Cc: Leo Yan <leo.yan@arm.com>
Cc: Mike Leach <mike.leach@arm.com>
Cc: Robert Walker <robert.walker@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 3e01b401ca6f3a08..5d53ed74ae8835a9 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -1370,9 +1370,12 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq,
 					 struct cs_etm_packet *packet, u64 addr)
 {
 	u8 instrBytes[2];
+	u32 len;
 
-	cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
-				    ARRAY_SIZE(instrBytes), instrBytes);
+	len = cs_etm__frontend_mem_access(etmq, tidq, packet, addr,
+					  ARRAY_SIZE(instrBytes), instrBytes);
+	if (len != ARRAY_SIZE(instrBytes))
+		return packet->last_instr_size;
 	/*
 	 * T32 instruction size is indicated by bits[15:11] of the first
 	 * 16-bit word of the instruction: 0b11101, 0b11110 and 0b11111
-- 
2.56.0

[PATCH v1 10/12] perf tests: Fix the Intel PT virtual LBR check

[Arnaldo Carvalho de Melo]

The record this check uses writes aux samples, and with the single data
page the other tests ask for they are lost:

  Warning: Processed 29 samples and lost 100.00%

so no branches are decoded and the check fails with "Failed with virtual
lbr".  Record with 4 data pages and 128 auxtrace pages: the same trace
decodes 64 branches and still fits the mlock budget.

Require the python perf module to provide session, not just to be
importable: a perf whose python bindings are not found can pick up an
unrelated module and fail with

  Error processing events: module 'perf' has no attribute 'session'

instead of skipping the subtest.

Detected building perf on fedora:44 on the runner with Intel PT hardware,
where the subtest failed with "Failed with virtual lbr" in the container,
and with the python error when run as root on the same machine.

Signed-off-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index c0ecece0a8255124..511440b5956a6897 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -384,13 +384,14 @@ test_virtual_lbr()
 	# shellcheck source=lib/setup_python.sh
 	. "${shelldir}"/lib/setup_python.sh
 
-	if ! "$PYTHON" -c 'import perf' > /dev/null 2>&1; then
-		echo "SKIP: Python perf module not found"
+	if ! "$PYTHON" -c 'import perf; perf.session' > /dev/null 2>&1; then
+		echo "SKIP: Python perf module with session support not found"
 		return 2
 	fi
 
-	# Check if virtual lbr is working
-	perf_record_no_bpf -o "${tmpfile}" --aux-sample \
+	# Check if virtual lbr is working.  This records aux samples, which a
+	# single data page overruns, so give it more room than the other tests.
+	perf_record_no_bpf -m 4,128 -o "${tmpfile}" --aux-sample \
 		-e '{intel_pt//,cycles}:u' perf test -w brstack || return 1
 	perf inject --itrace=L -i "${tmpfile}" -o "${perfdatafile}" || return 1
 	output=$("$PYTHON" "${shelldir}/lib/perf_brstack_max.py" -i "${perfdatafile}") || return 1
-- 
2.56.0

[PATCH v1 11/12] perf test: Drop samples without an IP in the dlfilter test

[Arnaldo Carvalho de Melo]

Since v7.3 the kernel zeroes PERF_SAMPLE_IP when a userspace-only event
skids into kernel mode, so that kernel register state is not exposed to
an unprivileged event (166f10836a653dfa "perf/core: Fix kernel register
info leak via hardware skid").

The test dlfilter keeps those samples, and the check that nothing but
test_loop survives then trips on their lines: the comm column reads
"perf".  Drop them, which is what the test wants, everything except
test_loop must be filtered out.

Fixes: 199d5e872a9ce737 ("perf tests script dlfilter: Add a dlfilter test")
Cc: Ian Rogers <irogers@google.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/shell/script_dlfilter.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/script_dlfilter.sh b/tools/perf/tests/shell/script_dlfilter.sh
index 88222280f6094fd0..797cd9ae84141f64 100755
--- a/tools/perf/tests/shell/script_dlfilter.sh
+++ b/tools/perf/tests/shell/script_dlfilter.sh
@@ -46,7 +46,7 @@ int filter_event(void *data, const struct perf_dlfilter_sample *sample, void *ct
 	const struct perf_dlfilter_al *al;
 
 	if (!sample->ip)
-		return 0;
+		return 1;
 
 	al = perf_dlfilter_fns.resolve_ip(ctx);
 	if (!al || !al->sym || strcmp(al->sym, "test_loop"))
-- 
2.56.0

[PATCH v1 12/12] perf test: Skip the LBR inline unwinding check when unsupported

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The check only looks for the PMU's caps/branches file before recording
with --call-graph lbr.  AMD BRS advertises branches but only samples its
own retired branches event, so the record with cycles is rejected and
the test fails instead of skipping.

Probe the record and skip when it fails, as the branch stack test does.

Fixes: 54a23bff770961e0 ("perf test: Test addr2line unwinding works with inline functions")
Cc: Ian Rogers <irogers@google.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/shell/addr2line_inlines.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/addr2line_inlines.sh b/tools/perf/tests/shell/addr2line_inlines.sh
index e8754ef2d7f21ac2..6fe47909dae4f812 100755
--- a/tools/perf/tests/shell/addr2line_inlines.sh
+++ b/tools/perf/tests/shell/addr2line_inlines.sh
@@ -71,7 +71,14 @@ test_lbr() {
     fi
 
     # Record data. Currently only dwarf callchains support inlined functions.
-    perf record --call-graph lbr -e cycles:u -o "${perf_data}" -- perf test -w inlineloop 1
+    # Some PMUs advertise branches but reject branch sampling for cycles, e.g.
+    # AMD BRS, which only samples its own retired branches event.
+    if ! perf record --call-graph lbr -e cycles:u -o "${perf_data}" -- \
+            perf test -w inlineloop 1 2>/dev/null
+    then
+        echo "Skip: LBR not supported by the PMU"
+        return
+    fi
 
     # Check output with inline (default) and srcline
     perf script -i "${perf_data}" --fields +srcline > "${perf_script_txt}"
-- 
2.56.0

Re: [PATCH v1 07/12] perf tests: Silence the libbfd probe in build-test

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> build-test evaluates the libbfd probe at parse time and the compiler's
> stderr isn't captured by $(shell ...), so without binutils-devel it
> prints "bfd.h: No such file or directory" once per tests/make
> invocation, five times per run.
> 
> Redirect the compiler's stderr; the empty result still skips the
> nondistro tests as intended.
> 
> Fixes: 523471c5163659c6 ("perf build: Skip nondistro build test if libbfd is old")
> Cc: James Clark <james.clark@linaro.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=7

Re: [PATCH v1 09/12] perf cs-etm: Fall back to the recorded instruction size on a failed read

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> cs_etm__t32_instr_size() reads the instruction bytes from the traced
> program's memory to tell a 16-bit from a 32-bit T32 instruction, but it
> ignores cs_etm__frontend_mem_access() failing, which happens when the
> DSO or its debug data is missing, and then reads the uninitialized
> buffer.  GCC 16 warns about it and build-test fails with -Werror.
> 
> Use the instruction size recorded by the decoder, as the A32/A64 path
> already does, when the read doesn't return the full two bytes.
> 
> Fixes: a7ee4d625ede4f62 ("perf cs-etm: Support for ARM A32/T32 instruction sets in CoreSight trace")
> Cc: James Clark <james.clark@linaro.org>
> Cc: Leo Yan <leo.yan@arm.com>
> Cc: Mike Leach <mike.leach@arm.com>
> Cc: Robert Walker <robert.walker@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=9

Re: [PATCH v1 03/12] perf tests: Exercise the flexible array member path

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> No test covered find_flex_array() and get_flex_array_member().  Add one
> that keeps a struct with a trailing data[] behind a global pointer and
> checks that accesses past the struct size are resolved and named after
> the flexible array member.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=3

Re: [PATCH v1 05/12] perf tests: Exercise data type resolution from a register

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> At -O2 a struct pointer lives in a register and DWARF describes it with
> a register location, while the unoptimized workloads the data type tests
> profile keep their variables on the stack.
> 
> Build a workload with a noinline function that gets the pointer from a
> volatile global, which keeps it in a register, and check that its three
> member accesses are resolved from that location.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=5

Re: [PATCH v1 11/12] perf test: Drop samples without an IP in the dlfilter test

[sashiko-bot]

> Since v7.3 the kernel zeroes PERF_SAMPLE_IP when a userspace-only event
> skids into kernel mode, so that kernel register state is not exposed to
> an unprivileged event (166f10836a653dfa "perf/core: Fix kernel register
> info leak via hardware skid").
> 
> The test dlfilter keeps those samples, and the check that nothing but
> test_loop survives then trips on their lines: the comm column reads
> "perf".  Drop them, which is what the test wants, everything except
> test_loop must be filtered out.
> 
> Fixes: 199d5e872a9ce737 ("perf tests script dlfilter: Add a dlfilter test")
> Cc: Ian Rogers <irogers@google.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=11

Re: [PATCH v1 02/12] perf annotate-data: Resolve DW_OP_addrx global variables

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> clang emits DW_AT_location with DW_OP_addrx for global variables in
> DWARF 5, gcc emits DW_OP_addr.  The address index was not resolved, so
> those variables were not found by address; the by-name fallback also
> missed the ones in .bss past the file backed end of the last segment.
> 
> Resolve the index with dwarf_getlocation_attr() + dwarf_formaddr() and
> use the resulting address in the by-address lookup and in the global
> variable cache.  Add a test that compiles the workload with clang
> -gdwarf-5 and checks that the global is annotated.
> 
> Fixes: d60469d7c0e5c4e8 ("perf dwarf-aux: Add die_find_variable_by_addr()")
> Fixes: d7b60803a790b716 ("perf dwarf-aux: Add die_collect_global_vars()")
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=2

Re: [PATCH v1 08/12] perf build: Record the display-only features in FEATURE-DUMP

[sashiko-bot]

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> llvm-perf and rust are in FEATURE_DISPLAY but not in FEATURE_TESTS: they
> can't be part of the test-all fast path, which sets every FEATURE_TESTS
> entry to 1 when test-all.c builds.  Since FEATURE-DUMP is written from
> FEATURE_TESTS only, a FEATURES_DUMP= build, as used by build-test, never
> sees them and silently builds without libLLVM and the rust workloads.
> 
> Append the display features that aren't in FEATURE_TESTS to the dump
> after the regular feature checks have run.
> 
> Fixes: 206dcfca1f5cfac3 ("perf build: Autodetect minimum required llvm-dev version")
> Fixes: 3f5dfa472ea6771c ("tools build: Fix rust feature detection")
> Cc: James Clark <james.clark@linaro.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=8

Re: [PATCH v1 09/12] perf cs-etm: Fall back to the recorded instruction size on a failed read

[Leo Yan]

Hi Arnaldo,

On Fri, Nov 13, 2026 at 12:05:50PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> cs_etm__t32_instr_size() reads the instruction bytes from the traced
> program's memory to tell a 16-bit from a 32-bit T32 instruction, but it
> ignores cs_etm__frontend_mem_access() failing, which happens when the
> DSO or its debug data is missing, and then reads the uninitialized
> buffer.  GCC 16 warns about it and build-test fails with -Werror.
> 
> Use the instruction size recorded by the decoder, as the A32/A64 path
> already does, when the read doesn't return the full two bytes.

This patch is duplicate with:

https://lore.kernel.org/linux-perf-users/20261113-perf_fix_gcc15_failure-v1-0-eae592c5ff64@arm.com/T/#m5f610e381705bc587a9ce3d31be4e1e0ed595506

I prefer that one, it handle cs_etm__frontend_mem_access() failure
throughly.

Re: [PATCH v1 12/12] perf test: Skip the LBR inline unwinding check when unsupported

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf test: incorrect Fixes trailer in commit message for
  test_lbr()
--

commit a78b0cf1c3a28b268aa40ae18726740e97e69c32
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

> perf test: Skip the LBR inline unwinding check when unsupported
>
> The check only looks for the PMU's caps/branches file before recording
> with --call-graph lbr.  AMD BRS advertises branches but only samples its
> own retired branches event, so the record with cycles is rejected and
> the test fails instead of skipping.
>
> Probe the record and skip when it fails, as the branch stack test does.
>
> Fixes: 54a23bff770961e0 ("perf test: Test addr2line unwinding works with inline functions")
> Cc: Ian Rogers <irogers@google.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

[Severity: Low]
54a23bff7709 ("perf test: Test addr2line unwinding works with inline
functions") only added the dwarf test; test_lbr() and its unguarded
"--call-graph lbr -e cycles:u" record were introduced later.  Should the
Fixes tag be:

  Fixes: 446c595dc0dd ("perf test addr2line_inlines: Ensure inline information shows on LBR leaves")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=12

Re: [PATCH v1 09/12] perf cs-etm: Fall back to the recorded instruction size on a failed read

[Arnaldo Carvalho de Melo]

On Fri, Nov 13, 2026 at 03:15:49PM +0000, Leo Yan wrote:
> Hi Arnaldo,
> 
> On Fri, Nov 13, 2026 at 12:05:50PM -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > cs_etm__t32_instr_size() reads the instruction bytes from the traced
> > program's memory to tell a 16-bit from a 32-bit T32 instruction, but it
> > ignores cs_etm__frontend_mem_access() failing, which happens when the
> > DSO or its debug data is missing, and then reads the uninitialized
> > buffer.  GCC 16 warns about it and build-test fails with -Werror.
> > 
> > Use the instruction size recorded by the decoder, as the A32/A64 path
> > already does, when the read doesn't return the full two bytes.
> 
> This patch is duplicate with:
> 
> https://lore.kernel.org/linux-perf-users/20261113-perf_fix_gcc15_failure-v1-0-eae592c5ff64@arm.com/T/#m5f610e381705bc587a9ce3d31be4e1e0ed595506
> 
> I prefer that one, it handle cs_etm__frontend_mem_access() failure
> throughly.

Ok, I'll replace it with yours,

- Arnaldo

Re: [PATCH v1 04/12] perf annotate-data: Count flexible array accesses for the whole struct

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf annotate-data: missing member_extent() use in TUI
  get_member_overhead()
--

commit db1590c6b9cdacf042578dbb6359cc6b78b628eb
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf annotate-data: Count flexible array accesses for the whole struct

This commit introduces member_extent() to extend a struct or union member's
sample range to the end of its last child so flexible array accesses count
toward the enclosing type in print_annotated_data_type().

> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index dd9c1d7b2da8b..7fae186ffd76b 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -2233,6 +2233,25 @@ static void print_annotated_data_value(struct type_hist *h, u64 period, int nr_s
>  		color_fprintf(stdout, color, " %7.2f", percent);
>  }
>  
> +/*
> + * A flexible array member is resolved past the end of its parent, so the
> + * parent's samples are not bounded by its size.
> + */
> +static int member_extent(struct annotated_member *member)
> +{
> +	struct annotated_member *child;
> +	int end = member->offset + member->size;
> +
> +	list_for_each_entry(child, &member->children, node) {
> +		int child_end = member_extent(child);
> +
> +		if (child_end > end)
> +			end = child_end;
> +	}
> +
> +	return end;
> +}
> +
>  static void print_annotated_data_type(struct annotated_data_type *mem_type,
>  				      struct annotated_member *member,
>  				      struct evsel *evsel, int indent)
> @@ -2240,6 +2259,7 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
>  	struct annotated_member *child;
>  	struct type_hist *h;
>  	int i, nr_events = 0, samples = 0;
> +	int end = member_extent(member);
>  	u64 period = 0;
>  	int width = symbol_conf.show_total_period ? 11 : 7;
>  	struct evsel *pos;
> @@ -2253,10 +2273,10 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
>  
>  		samples = 0;
>  		period = 0;
> -		for (i = 0; i < member->size; i++) {
> +		for (i = member->offset; i < end; i++) {
>  			struct type_hist_entry *entry;
>  
> -			if (!hashmap__find(&h->samples, member->offset + i, &entry))
> +			if (!hashmap__find(&h->samples, i, &entry))
>  				continue;
>  
>  			samples += entry->nr_samples;

[Severity: Low]
Should get_member_overhead() in the TUI data-type browser also use
member_extent()?

tools/perf/ui/browsers/annotate-data.c:get_member_overhead() {
	struct annotated_member *member = entry->data;
	int i, k;

	for (i = 0; i < member->size; i++) {
		struct type_hist *h;
		struct evsel *evsel;
		int offset = member->offset + i;
    ...
}

Because get_member_overhead() still iterates only up to member->size,
inspecting a struct with flexible array accesses in the TUI browser continues
to omit those samples from the enclosing struct's overhead, sample count, and
period.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=4

Re: [PATCH v1 01/12] perf scripts: Add a coverage harness for perf

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential
issues to consider:
- [Low] perf scripts: llvm_cov_objects() looks for perf.so in wrong
  directory
- [Low] perf scripts: misplaced delta header and bad alignment in
  print_summary()
- [Low] perf scripts: print_uncovered() fails to strip tools/perf/ path
  prefix
- [Low] perf build: inconsistent O= handling in Makefile coverage
  targets
- [Low] perf scripts: locale-sensitive float handling in print_table()
- [Low] perf scripts: print_worst_files() checks total lines instead of
  missed
--

commit e4ee2d1add56afe4f156fafd475dce263c39b01d
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf scripts: Add a coverage harness for perf

This commit adds tools/perf/scripts/coverage.sh and corresponding make
targets to build perf with clang source-based coverage, run the perf test
suite, and generate line and function coverage reports and ASCII tables.

> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 00baf90e5bda7..d00b63ca011c2 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -140,6 +140,15 @@ check-headers:
>  sync-headers:
>  	@./scripts/sync-headers.sh $(SYNC_HEADERS_ARGS)
>  
> +coverage-prep:
> +	@./scripts/coverage.sh --prep $(if $(O),--build-dir=$(O)) $(COVERAGE_ARGS)
> +
> +coverage:
> +	@./scripts/coverage.sh $(COVERAGE_ARGS)
> +
> +coverage-table:
> +	@./scripts/coverage.sh --table $(COVERAGE_ARGS)

[Severity: Low]
Should coverage and coverage-table also pass --build-dir, and should all
three targets use $(FULL_O) instead of $(O)?

Without --build-dir, running make -C tools/perf O=/path/to/build coverage
(or coverage-table) ignores O= and falls back to the default build
directory.  In addition, passing raw $(O) to coverage.sh resolves relative
paths against tools/perf rather than the caller's working directory,
whereas $(FULL_O) canonicalizes $(O) against $(PWD).

[ ... ]

> diff --git a/tools/perf/scripts/coverage.sh b/tools/perf/scripts/coverage.sh
> new file mode 100755
> index 0000000000000..19b0da22000c3
> --- /dev/null
> +++ b/tools/perf/scripts/coverage.sh

[ ... ]

> +llvm_cov_objects()
> +{
> +	echo "$BUILD_DIR/perf"
> +	local so
> +	for so in perf.so libperf.so libperf-jvmti.so; do
> +		[ -f "$BUILD_DIR/$so" ] && echo "--object=$BUILD_DIR/$so"
> +	done
> +}

[Severity: Low]
Does this miss the Python C extension object?  Makefile.perf builds the
extension with PYTHON_EXTENSION_SUFFIX (for example,
perf.cpython-312-x86_64-linux-gnu.so) and copies it into
$(OUTPUT)python/, so $BUILD_DIR/perf.so does not exist and coverage for
util/python.c is omitted from the reports.  Matching
$BUILD_DIR/python/perf*.so would include it.

[ ... ]

> +print_worst_files()
> +{
> +	[ "$TOP" -gt 0 ] || return 0
> +	local rows missed lines cover fn path
> +	rows=$(awk -v min="$MIN_LINES" '
> +		$1 ~ /(^|\/)tools\/perf\// && $8 >= min && $8 > 0 {
> +			path = $1
> +			sub(/^.*tools\/perf\//, "", path)
> +			printf "%s %s %s %s %s\n", $9, $8, $10, $7, path
> +		}' "$REPORT" | sort -k1,1nr -k2,2nr | sed -n "1,${TOP}p")

[Severity: Low]
Should the second awk check be $9 > 0 (missed lines) instead of $8 > 0
(total lines), matching table_rows()?  Checking $8 > 0 is redundant with
$8 >= min and allows files with 0 missed lines to be listed under largest
uncovered files.

[ ... ]

> +	if [ "$USE_BASELINE" = 1 ] && [ -f "$BASELINE" ]; then
> +		printf "  %-8s %8s  %8s  %9s\n" "File" "Previous" "Current" "Delta"
> +		printf "  %-8s %8s  %8s  %9s\n" "--------" "--------" "--------" "---------"
> +	fi
> +
> +	printf "  \033[1m%-44s\033[0m %8s  %8s  %8s  %8s\n" \
> +		"file" "lines" "cover" "missed" "fn-cover"
> +	for file in "${FOCUS[@]}"; do
> +		cov=$(file_cov "$REPORT" "$file")
> +		if [ -z "$cov" ]; then
> +			printf "  %-36s %8s\n" "$file" "not covered"
> +			continue
> +		fi
> +		printf "  %-36s %8s  %6s  %6s  %8s\n" "$file" \
> +			"$(echo "$cov" | awk '{print $1}')" \
> +			"$(echo "$cov" | awk '{print $3}')" \
> +			"$(echo "$cov" | awk '{print $2}')" \
> +			"$(echo "$cov" | awk '{print $4}')"
> +	done
> +
> +	if [ "$USE_BASELINE" = 1 ] && [ -f "$BASELINE" ]; then
> +		printf "\n  per file delta since %s\n" "$BASELINE"
> +		for file in "${FOCUS[@]}"; do
> +			report_delta "$BASELINE" "$REPORT" "$file"
> +		done
> +	fi
> +}

[Severity: Low]
Are the baseline delta header and row widths in print_summary()
misaligned when -f is used?

The 4-column "File Previous Current Delta" header is printed right above
the 5-column focus table instead of in the "per file delta since
$BASELINE" block below where report_delta() is called.  In the 5-column
focus table itself, the header uses "%-44s %8s  %8s  %8s  %8s", while the
rows use "%-36s %8s  %6s  %6s  %8s", shifting the numeric columns out of
alignment.

Also, because update_baseline() copies $REPORT to $BASELINE at the end of
a run (moving the prior baseline to $BASELINE.prev), a subsequent
--report-only run compares $REPORT against $BASELINE (itself) and always
reports a +0.00% delta.

[ ... ]

> +table_rows()
> +{
> +	if [ ${#FOCUS[@]} -gt 0 ]; then
> +		local file
> +		for file in "${FOCUS[@]}"; do
> +			table_row "$REPORT" "$file"
> +		done | sort -t$'\t' -k5,5gr
> +		return
> +	fi
> +
> +	awk -v min="$MIN_LINES" '
> +		$1 ~ /(^|\/)tools\/perf\// && $8 >= min && $9 > 0 {
> +			path = $1
> +			sub(/^.*tools\/perf\//, "", path)
> +			printf "%s\t%s\t%d\t%d\t%.2f\t%s\t%d\t%s\n",
> +			       path, $8, $8 - $9, $9, $10, $5, $5 - $6, $7
> +		}' "$REPORT" |
> +		sort -t$'\t' -k4,4nr |
> +		if [ "$TOP" -gt 0 ]; then sed -n "1,${TOP}p"; else cat; fi |
> +		sort -t$'\t' -k5,5gr
> +}
> +
> +print_table()
> +{
> +	local rows total path lines hit missed pct funcs funcs_hit func_cov
> +	local row_fmt w d_file d29 d13 d10 d8
> +
> +	rows=$(table_rows)

[ ... ]

> +	row_fmt=$(printf '│ %%-%ds │ %%6s │ %%6s │ %%6s │ %%s %%5.1f%%%%' "$w")
> +	row_fmt+=' │ %5s/%-5s │ %8s │\n'
> +
> +	while IFS=$'\t' read -r path lines hit missed pct funcs funcs_hit func_cov; do
> +		printf "$row_fmt" "$path" "$lines" "$hit" "$missed" \
> +			"$(bar "$pct" 20)" "$pct" "$funcs_hit" "$funcs" "$func_cov"
> +	done <<< "$rows"

[Severity: Low]
Does --table fail under locales where LC_NUMERIC uses a comma decimal
separator (for example, de_DE.UTF-8 or pt_BR.UTF-8)?

awk emits dot-separated floats in $pct (such as "33.53"), which sort -g
and bash's built-in printf "%5.1f" then parse according to LC_NUMERIC.
When LC_NUMERIC expects a comma, printf rejects $pct as an invalid number
and exits with status 1, aborting the script under set -e.  Exporting
LC_ALL=C or LC_NUMERIC=C at the top of the script would avoid this.

[ ... ]

> +print_uncovered()
> +{
> +	[ "$UNCOVERED" -gt 0 ] || return 0
> +	[ ${#FOCUS[@]} -gt 0 ] || return 0
> +	local objects file out lines header=0
> +	mapfile -t objects < <(llvm_cov_objects)
> +
> +	for file in "${FOCUS[@]}"; do
> +		# No report row means the file was not built: print_summary()
> +		# already said so, llvm-cov show would dump the whole profile.
> +		[ -n "$(file_cov "$REPORT" "$file")" ] || continue
> +		out=$("$LLVM_COV" show "${objects[@]}" --instr-profile="$PROFDATA" \
> +			"$PERF_DIR/$file" 2>/dev/null |

[Severity: Low]
Should print_uncovered() strip a leading .*tools/perf/ prefix from $file
before prepending $PERF_DIR/?

file_cov() and table_row() both strip ^.*tools/perf/ so that paths like
-f tools/perf/util/evsel.c work, but passing "$PERF_DIR/$file" directly to
llvm-cov show produces "$PERF_DIR/tools/perf/util/evsel.c" and fails to
find the source file.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=1

Re: [PATCH v1 06/12] perf annotate: Print the stats after the annotations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf test: missing DWARF feature check in annotate_stats.sh
  without libdw
--

commit a3aa8082d0e5c5ce9cb6c085dad08c8447fc23aa
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf annotate: Print the stats after the annotations

This commit moves the printing of data type and instruction stats in
perf annotate to after the annotation loops and adds a shell test for
instruction stats.

> diff --git a/tools/perf/tests/shell/annotate_stats.sh b/tools/perf/tests/shell/annotate_stats.sh
> new file mode 100755
> index 0000000000000..82d2becdff141
> --- /dev/null
> +++ b/tools/perf/tests/shell/annotate_stats.sh

[ ... ]

> +if ! perf annotate --code-with-type --insn-stat -i "$perfdata" --stdio \
> +		--percent-limit 1 > "$perfout" 2>/dev/null; then
> +	echo "FAIL: perf annotate"
> +	cleanup
> +	exit 1
> +fi
> +
> +# The instruction stats are accounted while the instructions are
> +# displayed; printing them before the display loop shows zeros.
> +if ! grep -Eq '^total [1-9][0-9]*,' "$perfout"; then
> +	echo "FAIL: instruction stats not accounted"
> +	cleanup
> +	exit 1
> +fi

[Severity: Low]
Will this test fail with exit status 1 instead of skipping when perf is
built without libdw dwarf support (HAVE_LIBDW_SUPPORT)?

Unlike --data-type, --code-with-type does not exit early when
HAVE_LIBDW_SUPPORT is disabled.  Instead, debuginfo__new() returns NULL,
so annotation_line__print() skips __hist_entry__get_data_type() and
prints "total 0".  Checking "perf check feature -q dwarf" before running
the test, as in data_type_register.sh, would skip cleanly with exit
status 2.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=6

Re: [PATCH v1 10/12] perf tests: Fix the Intel PT virtual LBR check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] perf tests: missing Fixes: trailer for commit 915d6b14dc0c
--

commit 8d6a959ddd3628adc90143588c99d2bf9b1a9f23
Author: Arnaldo Carvalho de Melo <acme@kernel.org>

> perf tests: Fix the Intel PT virtual LBR check
>
> The record this check uses writes aux samples, and with the single data
> page the other tests ask for they are lost:
>
>   Warning: Processed 29 samples and lost 100.00%
>
> so no branches are decoded and the check fails with "Failed with virtual
> lbr".  Record with 4 data pages and 128 auxtrace pages: the same trace
> decodes 64 branches and still fits the mlock budget.
>
> Require the python perf module to provide session, not just to be
> importable: a perf whose python bindings are not found can pick up an
> unrelated module and fail with
>
>   Error processing events: module 'perf' has no attribute 'session'
>
> instead of skipping the subtest.
>
> Detected building perf on fedora:44 on the runner with Intel PT hardware,
> where the subtest failed with "Failed with virtual lbr" in the container,
> and with the python error when run as root on the same machine.
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

[Severity: Low]
Both the python perf module check and the perf_record_no_bpf call in
test_virtual_lbr() were introduced by 915d6b14dc0c ("perf test: Migrate
Intel PT virtual LBR test to Python API").  Should this include:

  Fixes: 915d6b14dc0c ("perf test: Migrate Intel PT virtual LBR test to Python API")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113150555.1163049-1-acme@kernel.org?part=10