Thread (82 messages) 82 messages, 2 authors, 13d ago

Re: [PATCH v3 05/58] objtool/klp: Build klp test fixtures through the harness

From: sashiko-bot@kernel.org
Date: 2026-09-14 23:34:27

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] objtool: test harness leaks vmlinux.o state across build_pair() calls
--

commit 87b920426dfa2a217d6f9104d0f701cab5d67f5e
Author: Song Liu [off-list ref]

objtool/klp: Build klp test fixtures through the harness

This patch standardizes how klp test fixtures are built by shifting the
compile steps into harness helpers like build_one() and build_pair(). It
also establishes make_vmlinux_pair() to correctly build and diff vmlinux
images for tests.
quoted hunk ↗ jump to hunk
diff --git a/tools/objtool/tests/lib.sh b/tools/objtool/tests/lib.sh
index efe0a71326799..35be42758ece8 100644
--- a/tools/objtool/tests/lib.sh
+++ b/tools/objtool/tests/lib.sh
[ ... ]
quoted hunk ↗ jump to hunk
@@ -179,6 +192,13 @@ setup()
 	workdir="$(mktemp -d)" || fail "mktemp failed"
 	trap cleanup EXIT
 
+	# A test may run several segments, and make_vmlinux_pair() repoints
+	# orig_obj at the image it builds.  Start each segment from the
+	# defaults, or the next build_pair() diffs the previous segment's
+	# object against the new one.
+	orig_obj=orig.o
+	patched_obj=patched.o
+
 	export_syms "$@"
 }
[Severity: Medium]
Since setup() only executes once during script initialization, doesn't this
fail to reset the state between test segments?

If a test calls make_vmlinux_pair() (which repoints orig_obj to vmlinux.o)
and subsequently calls build_pair() for a new segment, orig_obj will still
point to the stale vmlinux.o from the previous segment instead of orig.o.
This appears to cause run_diff() to evaluate the wrong file.

[ ... ]
 # build_pair <fixture.c> [cflags...]
 build_pair()
 {
-	local fixture="$FIXTURES_DIR/$1"; shift
+	local fixture="$1"; shift
 
-	[ -f "$fixture" ] || fail "missing fixture $fixture"
+	build_one "$fixture" orig.o "$@"
+	build_one "$fixture" patched.o "$@" -DPATCHED
+}
[Severity: Medium]
Should the state reset logic for orig_obj and patched_obj be placed here in
build_pair() instead of setup(), to ensure run_diff() evaluates the correct
objects when multiple segments are executed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914230355.3422996-1-song@kernel.org?part=5
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help