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

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

[Arnaldo Carvalho de Melo]

Hi,

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:

  - 32-bit x86 accesses globals with absolute addresses, which the data
    type resolver did not handle (patch 2).
  - clang DWARF 5 describes globals with DW_OP_addrx, which was not
    resolved (patch 3).
  - The flexible array member had no test; one showed the struct row at
    0.00% while the member said 100.00% (patches 4-5).
  - Resolving a struct pointer that -O2 keeps in a register from its
    DWARF location had no test (patch 6).
  - --insn-stat was printed before the loop that accounts the samples,
    so --code-with-type showed zeros (patch 7).
  - The libbfd probe printed a missing bfd.h error five times per
    build-test run (patch 8).
  - FEATURE-DUMP didn't record llvm-perf and rust, disabling both in the
    build-test builds (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 51.12% of perf's 205,007
lines.  The only failure is the POLL_HUP tracepoint regression, in code
this series does not change.

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.

What changed from v2:

  - Patch 2 is new: absolute addressing is resolved, so the new data type
    tests pass on 32-bit non-PIE builds and skip on 32-bit PIE.
  - Patches 3, 4 and 6: the C programs became 'perf test -w' workloads
    (global_data, flex_array, regptr), the shell tests no longer compile
    anything (Ian Rogers).
  - Patch 6: regptr uses long long members, so the offsets are the same
    on ILP32 and LP64.
  - Patch 5: add the missing annotated_member__extent() stub, so a
    build with slang but without libdw works (sashiko-bot); it is now
    a method of struct annotated_member.

What changed from v1:

  - Patch 9 (cs-etm) is gone: Leo Yan's series replaces it.
  - Patch 1: O= is accepted by all targets, the Python extension is in
    the report, -f is normalized and the delta matches its baseline.
  - Patch 5: the TUI browser counts flexible array accesses too.
  - Patch 7: the stats test skips without libdw.
  - Patches 10 and 12: Fixes tags corrected.

Best regards,

- Arnaldo

Arnaldo Carvalho de Melo (12):
  perf scripts: Add a coverage harness for perf
  perf annotate: Handle absolute addressing in data type profiling
  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 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                | 722 ++++++++++++++++++
 tools/perf/tests/builtin-test.c               |   3 +
 tools/perf/tests/make                         |   2 +-
 tools/perf/tests/shell/addr2line_inlines.sh   |   9 +-
 tools/perf/tests/shell/annotate_stats.sh      |  64 ++
 .../perf/tests/shell/data_type_flex_array.sh  | 108 +++
 tools/perf/tests/shell/data_type_global.sh    |  70 ++
 tools/perf/tests/shell/data_type_register.sh  |  64 ++
 tools/perf/tests/shell/script_dlfilter.sh     |   2 +-
 tools/perf/tests/shell/test_intel_pt.sh       |   9 +-
 tools/perf/tests/tests.h                      |   3 +
 tools/perf/tests/workloads/Build              |   6 +
 tools/perf/tests/workloads/flex_array.c       |  54 ++
 tools/perf/tests/workloads/global_data.c      |  55 ++
 tools/perf/tests/workloads/regptr.c           |  60 ++
 tools/perf/ui/browsers/annotate-data.c        |   7 +-
 tools/perf/util/annotate-arch/annotate-x86.c  |  41 +-
 tools/perf/util/annotate-data.c               |  24 +-
 tools/perf/util/annotate-data.h               |   8 +
 tools/perf/util/annotate.c                    |  20 +-
 tools/perf/util/annotate.h                    |   6 +
 tools/perf/util/dwarf-aux.c                   |  42 +-
 27 files changed, 1380 insertions(+), 29 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_global.sh
 create mode 100755 tools/perf/tests/shell/data_type_register.sh
 create mode 100644 tools/perf/tests/workloads/flex_array.c
 create mode 100644 tools/perf/tests/workloads/global_data.c
 create mode 100644 tools/perf/tests/workloads/regptr.c

base-commit: cc905a18a647a8e3ad42cd1596baa0e08fba26eb
v1-head: 3f5465bea8f5a97a
v2-head: 9706c488afffeb6d
-- 
2.56.0

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

[Arnaldo Carvalho de Melo]

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 in patch 3; the default build is DWARF 4, --dwarf-version 5 hits
it), 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 | 722 +++++++++++++++++++++++++++++++++
 4 files changed, 737 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..8df8e9ed37f0a8f5 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 $(FULL_O),--build-dir=$(FULL_O)) $(COVERAGE_ARGS)
+
+coverage:
+	@./scripts/coverage.sh $(if $(FULL_O),--build-dir=$(FULL_O)) $(COVERAGE_ARGS)
+
+coverage-table:
+	@./scripts/coverage.sh --table $(if $(FULL_O),--build-dir=$(FULL_O)) $(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..2f248d61a733938a
--- /dev/null
+++ b/tools/perf/scripts/coverage.sh
@@ -0,0 +1,722 @@
+#!/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
+
+# awk prints dots and printf/sort parse them: keep the numeric locale fixed
+# so --table doesn't abort where the decimal separator is a comma.
+export LC_ALL=C
+
+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
+	# The Python extension is named after the interpreter ABI and lives
+	# in its own directory.
+	for so in "$BUILD_DIR"/python/perf*.so; do
+		[ -f "$so" ] && echo "--object=$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/...".
+#
+# -f takes the path as it shows in the report, or a tools/perf/ relative one.
+focus_path()
+{
+	case "$1" in
+	*tools/perf/*) printf '%s\n' "${1#*tools/perf/}" ;;
+	*) printf '%s\n' "$1" ;;
+	esac
+}
+
+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
+	local cur_cov old_cov delta
+	label=$(focus_path "$3")
+	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 "  %-36s %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 && $9 > 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" ] &&
+	   ! cmp -s "$BASELINE" "$REPORT"; 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
+
+	printf "  \033[1m%-36s\033[0m %8s  %6s  %6s  %8s\n" \
+		"file" "lines" "cover" "missed" "fn-cover"
+	for file in "${FOCUS[@]}"; do
+		file=$(focus_path "$file")
+		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" ] &&
+	   ! cmp -s "$BASELINE" "$REPORT"; then
+		printf "\n  per file delta since %s\n" "$BASELINE"
+		printf "  %-36s %8s  %8s  %9s\n" "file" "previous" "current" "delta"
+		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
+		file=$(focus_path "$file")
+		# 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 v3 02/12] perf annotate: Handle absolute addressing in data type profiling

[Arnaldo Carvalho de Melo]

32-bit x86 accesses globals with absolute addresses, like
"movl %eax,0x8c73448", but the operand parser only recognized the
offset(reg) forms, so the operand had no memory reference, the resolver
saw a zero data address and resolved nothing.

Parse a bare number as an absolute memory operand, leaving jumps and
calls out, use the address in the resolver, and let the instruction
tracking record the type of an absolute global load in the destination
register.  The displacement is sign extended for 64-bit objects, as the
CPU does, and zero extended for 32-bit ones; a zero address is skipped
by both users, since it is what an unrelocated operand looks like.

With bare numbers being memory operands, a frame base register of -1,
"none", would also compare equal to the -1 "no register" of an absolute
operand and take it for a stack access, so guard those comparisons.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate-arch/annotate-x86.c | 41 ++++++++++++++++++--
 tools/perf/util/annotate.c                   | 20 +++++++++-
 tools/perf/util/annotate.h                   |  6 +++
 3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
index e38a4e7a54d5585c..297f2fe9bec79c83 100644
--- a/tools/perf/util/annotate-arch/annotate-x86.c
+++ b/tools/perf/util/annotate-arch/annotate-x86.c
@@ -230,6 +230,11 @@ static bool vtable_get_rettype(Dwarf_Die *class_die, int offset,
 	return die_get_real_type(&func_die, type_die) != NULL;
 }
 
+static bool is_stack_or_fb_reg(struct type_state *state, int fbreg, int reg)
+{
+	return (fbreg != -1 && reg == fbreg) || reg == state->stack_reg;
+}
+
 static void update_insn_state_x86(struct type_state *state,
 				  struct data_loc_info *dloc, Dwarf_Die *cu_die,
 				  struct disasm_line *dl)
@@ -437,7 +442,7 @@ static void update_insn_state_x86(struct type_state *state,
 		invalidate_reg_state(tsr);
 
 		/* Case 1: Based on stack pointer or frame pointer */
-		if (sreg == fbreg || sreg == state->stack_reg) {
+		if (is_stack_or_fb_reg(state, fbreg, sreg)) {
 			struct type_state_stack *stack;
 			int offset = src->offset - fboff;
 
@@ -626,7 +631,7 @@ static void update_insn_state_x86(struct type_state *state,
 
 retry:
 		/* Check stack variables with offset */
-		if (sreg == fbreg || sreg == state->stack_reg) {
+		if (is_stack_or_fb_reg(state, fbreg, sreg)) {
 			struct type_state_stack *stack;
 			int offset = src->offset - fboff;
 
@@ -741,6 +746,29 @@ static void update_insn_state_x86(struct type_state *state,
 				     insn_offset, addr, dst->reg1);
 			pr_debug_type_name(&type_die, tsr->kind);
 		}
+		/* Or an absolute address, as used by 32-bit code */
+		else if (annotated_op_loc__is_abs(src)) {
+			struct map_symbol *ms = dloc->ms;
+			u64 ip = ms->sym->start + dl->al.offset;
+			u64 addr = annotated_op_loc__abs_addr(src, map__dso(ms->map));
+			int offset;
+
+			if (!get_global_var_type(cu_die, dloc, ip, addr, &offset,
+						 &type_die) ||
+			    !die_get_member_type(&type_die, offset, &type_die)) {
+				invalidate_reg_state(tsr);
+				return;
+			}
+
+			tsr->type = type_die;
+			tsr->kind = TSR_KIND_TYPE;
+			tsr->offset = 0;
+			tsr->ok = true;
+
+			pr_debug_dtp("mov [%x] global addr=%#"PRIx64" -> reg%d",
+				     insn_offset, addr, dst->reg1);
+			pr_debug_type_name(&type_die, tsr->kind);
+		}
 		/* And check percpu access with base register */
 		else if (has_reg_type(state, sreg) &&
 			 state->regs[sreg].kind == TSR_KIND_PERCPU_BASE) {
@@ -827,7 +855,7 @@ static void update_insn_state_x86(struct type_state *state,
 	/* Case 3. register to memory transfers */
 	if (!src->mem_ref && dst->mem_ref) {
 		/* Check stack variables with offset */
-		if (dst->reg1 == fbreg || dst->reg1 == state->stack_reg) {
+		if (is_stack_or_fb_reg(state, fbreg, dst->reg1)) {
 			struct type_state_stack *stack;
 			int offset = dst->offset - fboff;
 
@@ -931,7 +959,7 @@ static int extract_reg_offset(const struct arch *arch, const char *str,
 }
 
 static void extract_op_location_x86(const struct arch *arch,
-				    struct disasm_line *dl __maybe_unused,
+				    struct disasm_line *dl,
 				    const char *op_str, int op_idx __maybe_unused,
 				    struct annotated_op_loc *op_loc)
 {
@@ -960,6 +988,11 @@ static void extract_op_location_x86(const struct arch *arch,
 			op_loc->offset = strtol(s + 1, &p, 0);
 			if (p && p != s + 1)
 				op_loc->imm = true;
+		} else if (isdigit(*s) && !ins__is_jump(&dl->ins) &&
+			   !ins__is_call(&dl->ins)) {
+			/* Absolute address, as used by 32-bit code */
+			op_loc->mem_ref = true;
+			op_loc->offset = strtoull(s, NULL, 0);
 		}
 	}
 }
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 86a4beec6528e9bd..56fffe598634c441 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2605,6 +2605,19 @@ int arch__dwarf_regnum(const struct arch *arch, const char *str)
 	return reg < 0 ? -1 : reg;
 }
 
+bool annotated_op_loc__is_abs(const struct annotated_op_loc *op_loc)
+{
+	return op_loc->mem_ref && op_loc->reg1 == -1 && op_loc->reg2 == -1 &&
+	       op_loc->segment == INSN_SEG_NONE && op_loc->offset != 0;
+}
+
+/* The disp32 of a 64-bit object is sign extended, a 32-bit one is not */
+u64 annotated_op_loc__abs_addr(const struct annotated_op_loc *op_loc,
+			       const struct dso *dso)
+{
+	return dso__is_64_bit(dso) ? (u64)(s64)op_loc->offset : (u32)op_loc->offset;
+}
+
 /**
  * annotate_get_insn_location - Get location of instruction
  * @arch: the architecture info
@@ -2622,7 +2635,7 @@ int arch__dwarf_regnum(const struct arch *arch, const char *str)
  *   mov  (%rax), %rcx   # src_reg1 = rax, src_mem = 1, src_offset = 0
  *                       # dst_reg1 = rcx, dst_mem = 0
  *
- *   mov  0x18, %r8      # src_reg1 = -1, src_mem = 0
+ *   mov  0x18, %r8      # src_reg1 = -1, src_mem = 1, src_offset = 0x18
  *                       # dst_reg1 = r8, dst_mem = 0
  *
  *   mov  %rsi, 8(%rbx,%rcx,4)  # src_reg1 = rsi, src_mem = 0, src_multi_regs = 0
@@ -2894,6 +2907,11 @@ __hist_entry__get_data_type(struct hist_entry *he, const struct arch *arch,
 			dloc.var_addr = annotate_calc_pcrel(ms, dloc.ip,
 							    op_loc->offset, dl);
 		}
+		/* Absolute addressing, as used by 32-bit code */
+		else if (annotated_op_loc__is_abs(op_loc)) {
+			dloc.var_addr = annotated_op_loc__abs_addr(op_loc, map__dso(ms->map));
+			op_loc->reg1 = DWARF_REG_PC;
+		}
 
 		/* This CPU access in kernel - pretend PC-relative addressing */
 		if (dso__kernel(map__dso(ms->map)) && arch__is_x86(arch) &&
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 1b47c1156b9a72e1..43d365807daf2107 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -18,6 +18,7 @@
 #include "branch.h"
 #include "evsel.h"
 
+struct dso;
 struct hist_browser_timer;
 struct hist_entry;
 struct map;
@@ -638,6 +639,11 @@ struct annotated_insn_loc {
 	     i < INSN_OP_MAX;						\
 	     i++, op_loc++)
 
+/* Bare number operand, as used by 32-bit code to access globals */
+bool annotated_op_loc__is_abs(const struct annotated_op_loc *op_loc);
+u64 annotated_op_loc__abs_addr(const struct annotated_op_loc *op_loc,
+			       const struct dso *dso);
+
 /* Get detailed location info in the instruction */
 int annotate_get_insn_location(const struct arch *arch, struct disasm_line *dl,
 			       struct annotated_insn_loc *loc);
-- 
2.56.0

[PATCH v3 03/12] perf annotate-data: Resolve DW_OP_addrx global variables

[Arnaldo Carvalho de Melo]

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.  The new global_data workload keeps a zero initialized
struct in .bss and its test checks that the accesses are annotated: when
perf is built with clang and DWARF 5 it goes through DW_OP_addrx, with
gcc through DW_OP_addr.  The test skips on 32-bit PIE builds, which
load globals through the GOT, something the resolver does not follow.

Fixes: d60469d7c0e5c4e8 ("perf dwarf-aux: Add die_find_variable_by_addr()")
Fixes: d7d60803a790b716 ("perf dwarf-aux: Add die_collect_global_vars()")
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/builtin-test.c            |  1 +
 tools/perf/tests/shell/data_type_global.sh | 70 ++++++++++++++++++++++
 tools/perf/tests/tests.h                   |  1 +
 tools/perf/tests/workloads/Build           |  2 +
 tools/perf/tests/workloads/global_data.c   | 55 +++++++++++++++++
 tools/perf/util/annotate-data.c            |  4 ++
 tools/perf/util/dwarf-aux.c                | 42 +++++++++++--
 7 files changed, 170 insertions(+), 5 deletions(-)
 create mode 100755 tools/perf/tests/shell/data_type_global.sh
 create mode 100644 tools/perf/tests/workloads/global_data.c

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 86698b86657f45b0..299e07bd16a5a53f 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -199,6 +199,7 @@ static struct test_workload *workloads[] = {
 	&workload__deterministic,
 	&workload__callchain,
 	&workload__false_sharing,
+	&workload__global_data,
 
 #ifdef HAVE_RUST_SUPPORT
 	&workload__code_with_type,
diff --git a/tools/perf/tests/shell/data_type_global.sh b/tools/perf/tests/shell/data_type_global.sh
new file mode 100755
index 0000000000000000..7db2e898a41657bc
--- /dev/null
+++ b/tools/perf/tests/shell/data_type_global.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+# perf data type profiling test for a global variable
+# 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
+
+if ! perf check feature -q dwarf; then
+	echo "Skip: no DWARF (libdw) support"
+	cleanup
+	exit 2
+fi
+
+# 32-bit PIE loads globals through the GOT, which the resolver does not follow
+if [ "$(getconf LONG_BIT)" = 32 ] &&
+   [ "$(od -An -tx1 -j16 -N2 "$(command -v perf)" | awk '{print $1}')" = 03 ]; then
+	echo "Skip: 32-bit PIE uses GOT-relative global accesses"
+	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
+
+if ! perf mem record -o "$perfdata" -- perf test -w global_data 1 > /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: global variable in .bss not resolved"
+	cleanup
+	exit 1
+fi
+
+echo "global variable resolved"
+cleanup
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 8c399840a735910d..bbd23dd7e0455a9d 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -272,6 +272,7 @@ DECLARE_WORKLOAD(context_switch_loop);
 DECLARE_WORKLOAD(deterministic);
 DECLARE_WORKLOAD(callchain);
 DECLARE_WORKLOAD(false_sharing);
+DECLARE_WORKLOAD(global_data);
 
 #ifdef HAVE_RUST_SUPPORT
 DECLARE_WORKLOAD(code_with_type);
diff --git a/tools/perf/tests/workloads/Build b/tools/perf/tests/workloads/Build
index 43cc4da6fb0786de..03017aaaaa0bec32 100644
--- a/tools/perf/tests/workloads/Build
+++ b/tools/perf/tests/workloads/Build
@@ -17,6 +17,7 @@ perf-test-y += context_switch_loop.o
 perf-test-y += deterministic.o
 perf-test-y += callchain.o
 perf-test-y += false_sharing.o
+perf-test-y += global_data.o
 
 ifeq ($(CONFIG_RUST_SUPPORT),y)
     perf-test-y += code_with_type.o
@@ -35,3 +36,4 @@ CFLAGS_deterministic.o    = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_named_threads.o    = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_callchain.o        = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_false_sharing.o    = -g -O0 -fno-inline -U_FORTIFY_SOURCE
+CFLAGS_global_data.o      = -g -O0 -fno-inline -U_FORTIFY_SOURCE
diff --git a/tools/perf/tests/workloads/global_data.c b/tools/perf/tests/workloads/global_data.c
new file mode 100644
index 0000000000000000..63da5300c5d92387
--- /dev/null
+++ b/tools/perf/tests/workloads/global_data.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A zero initialized global struct, so it lands in .bss, past the file
+ * backed end of the last segment.  clang describes its address with
+ * DW_OP_addrx in DWARF 5, gcc with DW_OP_addr.
+ *
+ * 'perf mem record' of this workload followed by 'perf annotate
+ * --code-with-type' (see tests/shell/data_type_global.sh) resolves the
+ * accesses to struct conn.
+ */
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <linux/compiler.h>
+#include "../tests.h"
+
+struct conn {
+	int saddr;
+	int daddr;
+	long bytes;
+};
+
+static volatile struct conn conn;
+static volatile unsigned long conn_sink;
+static volatile sig_atomic_t done;
+
+static void sighandler(int sig __maybe_unused)
+{
+	done = 1;
+}
+
+static int global_data(int argc, const char **argv)
+{
+	unsigned long sum = 0;
+	int i = 0;
+	int sec = 1;
+
+	if (argc > 0)
+		sec = atoi(argv[0]);
+
+	signal(SIGINT, sighandler);
+	signal(SIGALRM, sighandler);
+	alarm(sec);
+
+	while (!done) {
+		conn.saddr = i;
+		conn.daddr = i + 1;
+		sum += conn.bytes;
+		i++;
+	}
+	conn_sink = sum;
+	return 0;
+}
+
+DEFINE_WORKLOAD(global_data);
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index dd9c1d7b2da8be8a..d8d00646ef63b30f 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -140,6 +140,10 @@ static void pr_debug_location(Dwarf_Die *die, u64 pc, int reg)
 		case DW_OP_addr:
 			pr_info("address=%#lx\n", (long)ops->number);
 			break;
+		case DW_OP_addrx:
+		case DW_OP_GNU_addr_index:
+			pr_info("address index=%#lx\n", (long)ops->number);
+			break;
 		default:
 			pr_info("unknown: code=%#x, number=%#lx\n",
 				ops->atom, (long)ops->number);
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 v3 04/12] perf tests: Exercise the flexible array member path

[Arnaldo Carvalho de Melo]

No test covered find_flex_array() and get_flex_array_member().  Add a
flex_array workload that keeps a struct with a trailing data[] behind a
global pointer, and a test that checks the accesses past the struct size
are resolved and named after the flexible array member.  The test skips
on 32-bit PIE builds, which load globals through the GOT.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/builtin-test.c               |  1 +
 .../perf/tests/shell/data_type_flex_array.sh  | 70 +++++++++++++++++++
 tools/perf/tests/tests.h                      |  1 +
 tools/perf/tests/workloads/Build              |  2 +
 tools/perf/tests/workloads/flex_array.c       | 54 ++++++++++++++
 5 files changed, 128 insertions(+)
 create mode 100755 tools/perf/tests/shell/data_type_flex_array.sh
 create mode 100644 tools/perf/tests/workloads/flex_array.c

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 299e07bd16a5a53f..18311673b231e657 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -200,6 +200,7 @@ static struct test_workload *workloads[] = {
 	&workload__callchain,
 	&workload__false_sharing,
 	&workload__global_data,
+	&workload__flex_array,
 
 #ifdef HAVE_RUST_SUPPORT
 	&workload__code_with_type,
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..8d6b277991aea96e
--- /dev/null
+++ b/tools/perf/tests/shell/data_type_flex_array.sh
@@ -0,0 +1,70 @@
+#!/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)
+
+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
+
+if ! perf check feature -q dwarf; then
+	echo "Skip: no DWARF (libdw) support"
+	cleanup
+	exit 2
+fi
+
+# 32-bit PIE loads globals through the GOT, which the resolver does not follow
+if [ "$(getconf LONG_BIT)" = 32 ] &&
+   [ "$(od -An -tx1 -j16 -N2 "$(command -v perf)" | awk '{print $1}')" = 03 ]; then
+	echo "Skip: 32-bit PIE uses GOT-relative global accesses"
+	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
+
+if ! perf mem record -o "$perfdata" -- perf test -w flex_array 1 > /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
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index bbd23dd7e0455a9d..f99910b0f8d93a82 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -273,6 +273,7 @@ DECLARE_WORKLOAD(deterministic);
 DECLARE_WORKLOAD(callchain);
 DECLARE_WORKLOAD(false_sharing);
 DECLARE_WORKLOAD(global_data);
+DECLARE_WORKLOAD(flex_array);
 
 #ifdef HAVE_RUST_SUPPORT
 DECLARE_WORKLOAD(code_with_type);
diff --git a/tools/perf/tests/workloads/Build b/tools/perf/tests/workloads/Build
index 03017aaaaa0bec32..74a0cbe49dc93533 100644
--- a/tools/perf/tests/workloads/Build
+++ b/tools/perf/tests/workloads/Build
@@ -18,6 +18,7 @@ perf-test-y += deterministic.o
 perf-test-y += callchain.o
 perf-test-y += false_sharing.o
 perf-test-y += global_data.o
+perf-test-y += flex_array.o
 
 ifeq ($(CONFIG_RUST_SUPPORT),y)
     perf-test-y += code_with_type.o
@@ -37,3 +38,4 @@ CFLAGS_named_threads.o    = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_callchain.o        = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_false_sharing.o    = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_global_data.o      = -g -O0 -fno-inline -U_FORTIFY_SOURCE
+CFLAGS_flex_array.o       = -g -O0 -fno-inline -U_FORTIFY_SOURCE
diff --git a/tools/perf/tests/workloads/flex_array.c b/tools/perf/tests/workloads/flex_array.c
new file mode 100644
index 0000000000000000..20f9b0ec17969909
--- /dev/null
+++ b/tools/perf/tests/workloads/flex_array.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Flexible array member workload for data type profiling: the accesses to
+ * the array are past the end of the struct, where the resolver has to find
+ * the flexible array member.
+ *
+ * See tests/shell/data_type_flex_array.sh.
+ */
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <linux/compiler.h>
+#include "../tests.h"
+
+struct flex {
+	int len;
+	char data[];
+};
+
+static struct flex *gp;
+static volatile unsigned long flex_sink;
+static volatile sig_atomic_t done;
+
+static void sighandler(int sig __maybe_unused)
+{
+	done = 1;
+}
+
+static int flex_array(int argc, const char **argv)
+{
+	unsigned long sum = 0;
+	int sec = 1;
+
+	if (argc > 0)
+		sec = atoi(argv[0]);
+
+	gp = calloc(1, sizeof(*gp) + 4096);
+	if (gp == NULL)
+		return 1;
+	gp->len = 4096;
+
+	signal(SIGINT, sighandler);
+	signal(SIGALRM, sighandler);
+	alarm(sec);
+
+	while (!done) {
+		sum += gp->data[3];
+		gp->data[5] = 1;
+	}
+	flex_sink = sum;
+	return 0;
+}
+
+DEFINE_WORKLOAD(flex_array);
-- 
2.56.0

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

[Arnaldo Carvalho de Melo]

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.  The TUI browser
accounts the members the same way, use the extent there as well.

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/ui/browsers/annotate-data.c        |  7 ++--
 tools/perf/util/annotate-data.c               | 24 ++++++++++-
 tools/perf/util/annotate-data.h               |  8 ++++
 4 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/tools/perf/tests/shell/data_type_flex_array.sh b/tools/perf/tests/shell/data_type_flex_array.sh
index 8d6b277991aea96e..c8e8ccd130abd771 100755
--- a/tools/perf/tests/shell/data_type_flex_array.sh
+++ b/tools/perf/tests/shell/data_type_flex_array.sh
@@ -66,5 +66,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/ui/browsers/annotate-data.c b/tools/perf/ui/browsers/annotate-data.c
index 0f778788df4a934b..e4047e97e8a943a4 100644
--- a/tools/perf/ui/browsers/annotate-data.c
+++ b/tools/perf/ui/browsers/annotate-data.c
@@ -53,12 +53,11 @@ static int get_member_overhead(struct annotated_data_type *adt,
 			       struct evsel *leader)
 {
 	struct annotated_member *member = entry->data;
-	int i, k;
+	int i, k, end = annotated_member__extent(member);
 
-	for (i = 0; i < member->size; i++) {
+	for (i = member->offset; i < end; i++) {
 		struct type_hist *h;
 		struct evsel *evsel;
-		int offset = member->offset + i;
 
 		k = 0;
 		for_each_group_evsel(evsel, leader) {
@@ -69,7 +68,7 @@ static int get_member_overhead(struct annotated_data_type *adt,
 				continue;
 
 			h = &adt->histograms[evsel->core.idx];
-			if (hashmap__find(&h->samples, offset, &hist))
+			if (hashmap__find(&h->samples, i, &hist))
 				update_hist_entry(&entry->hists[k], hist);
 			k++;
 		}
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index d8d00646ef63b30f..a74f894cbda70f5d 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -2237,6 +2237,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.
+ */
+int annotated_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 = annotated_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)
@@ -2244,6 +2263,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 = annotated_member__extent(member);
 	u64 period = 0;
 	int width = symbol_conf.show_total_period ? 11 : 7;
 	struct evsel *pos;
@@ -2257,10 +2277,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;
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 184262c7356213fa..6e0d410929651426 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -260,6 +260,9 @@ void global_var_type__tree_delete(struct rb_root *root);
 /* Print data type annotation (including members) on stdout */
 int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel);
 
+/* End offset of a member, including a flexible array's children */
+int annotated_member__extent(struct annotated_member *member);
+
 /* Get name of member field at the given offset in the data type */
 int annotated_data_type__get_member_name(struct annotated_data_type *adt,
 					 char *buf, size_t sz, int member_offset);
@@ -316,6 +319,11 @@ static inline int hist_entry__annotate_data_tty(struct hist_entry *he __maybe_un
 	return -1;
 }
 
+static inline int annotated_member__extent(struct annotated_member *member)
+{
+	return member->offset + member->size;
+}
+
 static inline int annotated_data_type__get_member_name(struct annotated_data_type *adt __maybe_unused,
 						       char *buf __maybe_unused,
 						       size_t sz __maybe_unused,
-- 
2.56.0

[PATCH v3 06/12] perf tests: Exercise data type resolution from a register

[Arnaldo Carvalho de Melo]

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.

Add a regptr 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/builtin-test.c              |  1 +
 tools/perf/tests/shell/data_type_register.sh | 64 ++++++++++++++++++++
 tools/perf/tests/tests.h                     |  1 +
 tools/perf/tests/workloads/Build             |  2 +
 tools/perf/tests/workloads/regptr.c          | 60 ++++++++++++++++++
 5 files changed, 128 insertions(+)
 create mode 100755 tools/perf/tests/shell/data_type_register.sh
 create mode 100644 tools/perf/tests/workloads/regptr.c

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 18311673b231e657..b6d31f5f009d206b 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -201,6 +201,7 @@ static struct test_workload *workloads[] = {
 	&workload__false_sharing,
 	&workload__global_data,
 	&workload__flex_array,
+	&workload__regptr,
 
 #ifdef HAVE_RUST_SUPPORT
 	&workload__code_with_type,
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..0b7cb4bfe0aacfbe
--- /dev/null
+++ b/tools/perf/tests/shell/data_type_register.sh
@@ -0,0 +1,64 @@
+#!/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)
+
+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
+
+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
+
+if ! perf mem record -o "$perfdata" -- perf test -w regptr 1 > /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
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index f99910b0f8d93a82..6d284bc79cc31779 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -274,6 +274,7 @@ DECLARE_WORKLOAD(callchain);
 DECLARE_WORKLOAD(false_sharing);
 DECLARE_WORKLOAD(global_data);
 DECLARE_WORKLOAD(flex_array);
+DECLARE_WORKLOAD(regptr);
 
 #ifdef HAVE_RUST_SUPPORT
 DECLARE_WORKLOAD(code_with_type);
diff --git a/tools/perf/tests/workloads/Build b/tools/perf/tests/workloads/Build
index 74a0cbe49dc93533..17189458032c0a17 100644
--- a/tools/perf/tests/workloads/Build
+++ b/tools/perf/tests/workloads/Build
@@ -19,6 +19,7 @@ perf-test-y += callchain.o
 perf-test-y += false_sharing.o
 perf-test-y += global_data.o
 perf-test-y += flex_array.o
+perf-test-y += regptr.o
 
 ifeq ($(CONFIG_RUST_SUPPORT),y)
     perf-test-y += code_with_type.o
@@ -39,3 +40,4 @@ CFLAGS_callchain.o        = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_false_sharing.o    = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_global_data.o      = -g -O0 -fno-inline -U_FORTIFY_SOURCE
 CFLAGS_flex_array.o       = -g -O0 -fno-inline -U_FORTIFY_SOURCE
+CFLAGS_regptr.o           = -g -O2
diff --git a/tools/perf/tests/workloads/regptr.c b/tools/perf/tests/workloads/regptr.c
new file mode 100644
index 0000000000000000..d5d357b047e1cfd1
--- /dev/null
+++ b/tools/perf/tests/workloads/regptr.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A struct pointer that -O2 keeps in a register, so the member accesses
+ * have to be resolved from the register's type state and not from a
+ * stack slot.
+ *
+ * See tests/shell/data_type_register.sh.
+ */
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <linux/compiler.h>
+#include "../tests.h"
+
+struct node {
+	long long a;
+	long long b;
+	long long c;
+};
+
+static struct node n;
+static struct node *volatile vp;
+static volatile unsigned long long reg_sink;
+static volatile sig_atomic_t done;
+
+static void sighandler(int sig __maybe_unused)
+{
+	done = 1;
+}
+
+static noinline long long touch(struct node *p)
+{
+	long long sum = 0;
+
+	sum += p->a;
+	p->b = sum;
+	sum += p->c;
+	return sum;
+}
+
+static int regptr(int argc, const char **argv)
+{
+	unsigned long long sum = 0;
+	int sec = 1;
+
+	if (argc > 0)
+		sec = atoi(argv[0]);
+
+	vp = &n;
+	signal(SIGINT, sighandler);
+	signal(SIGALRM, sighandler);
+	alarm(sec);
+
+	while (!done)
+		sum += touch(vp);
+	reg_sink = sum;
+	return 0;
+}
+
+DEFINE_WORKLOAD(regptr);
-- 
2.56.0

[PATCH v3 07/12] perf annotate: Print the stats after the annotations

[Arnaldo Carvalho de Melo]

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 | 64 ++++++++++++++++++++++++
 2 files changed, 69 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..fe4891e8535d3944
--- /dev/null
+++ b/tools/perf/tests/shell/annotate_stats.sh
@@ -0,0 +1,64 @@
+#!/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
+
+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
+
+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 v3 08/12] perf tests: Silence the libbfd probe in build-test

[Arnaldo Carvalho de Melo]

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 v3 09/12] perf build: Record the display-only features in FEATURE-DUMP

[Arnaldo Carvalho de Melo]

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

Fixes: 915d6b14dc0c ("perf test: Migrate Intel PT virtual LBR test to Python API")
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 v3 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@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 v3 12/12] perf test: Skip the LBR inline unwinding check when unsupported

[Arnaldo Carvalho de Melo]

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: 446c595dc0dd ("perf test addr2line_inlines: Ensure inline information shows on LBR leaves")
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 v3 08/12] perf tests: Silence the libbfd probe in build-test

[sashiko-bot]

> 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/20261115014102.2879372-1-acme@kernel.org?part=8

Re: [PATCH v3 07/12] perf annotate: Print the stats after the annotations

[sashiko-bot]

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

Sashiko has reviewed this patch and found no issues.

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

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

[sashiko-bot]

> 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.  The TUI browser
> accounts the members the same way, use the extent there as well.
> 
> 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>

Sashiko has reviewed this patch and found no issues.

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

Re: [PATCH v3 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@redhat.com>

Sashiko has reviewed this patch and found no issues.

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

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

[sashiko-bot]

> 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.
> 
> Add a regptr 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/20261115014102.2879372-1-acme@kernel.org?part=6

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

[sashiko-bot]

> 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: 446c595dc0dd ("perf test addr2line_inlines: Ensure inline information shows on LBR leaves")
> Cc: Ian Rogers <irogers@google.com>
> 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/20261115014102.2879372-1-acme@kernel.org?part=12

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

[sashiko-bot]

> 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/20261115014102.2879372-1-acme@kernel.org?part=9

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

[sashiko-bot]

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

Sashiko has reviewed this patch and found no issues.

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

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

[sashiko-bot]

> 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.
> 
> Fixes: 915d6b14dc0c ("perf test: Migrate Intel PT virtual LBR test to Python API")
> 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/20261115014102.2879372-1-acme@kernel.org?part=10

Re: [PATCH v3 04/12] perf tests: Exercise the flexible array member path

[sashiko-bot]

> No test covered find_flex_array() and get_flex_array_member().  Add a
> flex_array workload that keeps a struct with a trailing data[] behind a
> global pointer, and a test that checks the accesses past the struct size
> are resolved and named after the flexible array member.  The test skips
> on 32-bit PIE builds, which load globals through the GOT.
> 
> 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/20261115014102.2879372-1-acme@kernel.org?part=4

Re: [PATCH v3 02/12] perf annotate: Handle absolute addressing in data type profiling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] perf annotate: unhandled zero address in
  __hist_entry__get_data_type()
--

commit 4e70868608c9afc77ab4fffa4bb7f6aa84106803
Author: Arnaldo Carvalho de Melo <acme@kernel.org>

perf annotate: Handle absolute addressing in data type profiling

This commit adds support for bare numeric operands as absolute memory
addresses in x86 data type profiling and guards frame-base register
comparisons in instruction tracking against -1.

> diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> index e38a4e7a54d55..297f2fe9bec79 100644
> --- a/tools/perf/util/annotate-arch/annotate-x86.c
> +++ b/tools/perf/util/annotate-arch/annotate-x86.c
[ ... ]
> @@ -960,6 +988,11 @@ static void extract_op_location_x86(const struct arch *arch,
>  			op_loc->offset = strtol(s + 1, &p, 0);
>  			if (p && p != s + 1)
>  				op_loc->imm = true;
> +		} else if (isdigit(*s) && !ins__is_jump(&dl->ins) &&
> +			   !ins__is_call(&dl->ins)) {
> +			/* Absolute address, as used by 32-bit code */
> +			op_loc->mem_ref = true;
> +			op_loc->offset = strtoull(s, NULL, 0);
>  		}
>  	}
>  }
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 86a4beec6528e..56fffe598634c 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2605,6 +2605,19 @@ int arch__dwarf_regnum(const struct arch *arch, const char *str)
>  	return reg < 0 ? -1 : reg;
>  }
>  
> +bool annotated_op_loc__is_abs(const struct annotated_op_loc *op_loc)
> +{
> +	return op_loc->mem_ref && op_loc->reg1 == -1 && op_loc->reg2 == -1 &&
> +	       op_loc->segment == INSN_SEG_NONE && op_loc->offset != 0;
> +}
[ ... ]
> @@ -2894,6 +2907,11 @@ __hist_entry__get_data_type(struct hist_entry *he, const struct arch *arch,
>  			dloc.var_addr = annotate_calc_pcrel(ms, dloc.ip,
>  							    op_loc->offset, dl);
>  		}
> +		/* Absolute addressing, as used by 32-bit code */
> +		else if (annotated_op_loc__is_abs(op_loc)) {
> +			dloc.var_addr = annotated_op_loc__abs_addr(op_loc, map__dso(ms->map));
> +			op_loc->reg1 = DWARF_REG_PC;
> +		}
>  
>  		/* This CPU access in kernel - pretend PC-relative addressing */
>  		if (dso__kernel(map__dso(ms->map)) && arch__is_x86(arch) &&

[Severity: Medium]
Can an unrelocated bare 0x0 operand cause an out-of-bounds read of
state->regs[-1] in check_matching_type()?

extract_op_location_x86() sets op_loc->mem_ref = true even when the parsed
address is 0, so the early !op_loc->mem_ref check in
__hist_entry__get_data_type() no longer skips the operand.  Because
annotated_op_loc__is_abs() requires op_loc->offset != 0, op_loc->reg1 is
left as -1 when __hist_entry__get_data_type() calls find_data_type(&dloc).

In find_data_type_die(), is_fbreg = (reg == fbreg) is unguarded and
matches (-1 == -1) when fbreg is -1, and if no scope variable matches,
find_data_type_block() -> find_data_type_insn() -> check_matching_type()
is reached with reg == -1:

tools/perf/util/annotate-data.c:check_matching_type() {
    ...
	int reg = dloc->op->reg1;
    ...
	pr_debug_dtp("chk [%x] reg%d offset=%s%#x ok=%d kind=%d ",
		     insn_offset, reg, offset_sign, abs(offset),
		     state->regs[reg].ok, state->regs[reg].kind);

	if (!state->regs[reg].ok)
		goto check_non_register;
    ...
check_non_register:
	if (reg == dloc->fbreg || reg == state->stack_reg) {
    ...
}

This reads state->regs[-1] before struct type_state on the stack (passing
&state->regs[-1].type to DWARF helpers if the adjacent byte is non-zero)
and otherwise matches reg == dloc->fbreg (-1 == -1) at check_non_register.

Only setting op_loc->mem_ref = true in extract_op_location_x86() when the
parsed offset is non-zero (or skipping mem_ref operands with reg1 == -1 in
__hist_entry__get_data_type()) would avoid this.

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

Re: [PATCH v3 03/12] perf annotate-data: Resolve DW_OP_addrx global variables

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] perf annotate-data: DW_OP_addrx fallback in dwarf-aux.c
  breaks annotate-data.c build
- [Low] perf annotate-data: typo in Fixes trailer SHA d7d60803a790b716
  in commit message
--

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

> perf annotate-data: Resolve DW_OP_addrx global variables
>
> 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.  The new global_data workload keeps a zero initialized
> struct in .bss and its test checks that the accesses are annotated: when
> perf is built with clang and DWARF 5 it goes through DW_OP_addrx, with
> gcc through DW_OP_addr.  The test skips on 32-bit PIE builds, which
> load globals through the GOT, something the resolver does not follow.
>
> Fixes: d60469d7c0e5c4e8 ("perf dwarf-aux: Add die_find_variable_by_addr()")
> Fixes: d7d60803a790b716 ("perf dwarf-aux: Add die_collect_global_vars()")
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

[Severity: Low]
The second Fixes: tag has a typo in the SHA (d7d60803a790b716 cannot be
resolved); the commit that introduced die_collect_global_vars() is
d7b60803a790 ("perf dwarf-aux: Add die_collect_global_vars()").

[ ... ]

> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index dd9c1d7b2da8b..d8d00646ef63b 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -140,6 +140,10 @@ static void pr_debug_location(Dwarf_Die *die, u64 pc, int reg)
>  		case DW_OP_addr:
>  			pr_info("address=%#lx\n", (long)ops->number);
>  			break;
> +		case DW_OP_addrx:
> +		case DW_OP_GNU_addr_index:
> +			pr_info("address index=%#lx\n", (long)ops->number);
> +			break;
>  		default:
>  			pr_info("unknown: code=%#x, number=%#lx\n",
>  				ops->atom, (long)ops->number);
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 48a11c95a6768..5db7458f01116 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
> +

[Severity: Medium]
Does defining the DW_OP_addrx fallback only in dwarf-aux.c break the
build of annotate-data.c against older elfutils headers?

The libdw feature check accepts elfutils starting from 0.160, while
DW_OP_addrx was added to dwarf.h in 0.171.  Moving the #ifndef DW_OP_addrx
definition to tools/perf/util/dwarf-aux.h would make it visible to both
translation units.

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