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

[sashiko-bot@kernel.org]

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 <song@kernel.org>

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.

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