Re: [PATCH v3 23/58] objtool/klp: Add test for rejecting references to init code/data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] objtool: test suite aborts due to fragile compiler assumptions
--

commit fa8e7a87126e5978150272fd5c3b8728c0d8f68a
Author: Puranjay Mohan <puranjay@kernel.org>

objtool/klp: Add test for rejecting references to init code/data

Init code and data are freed once boot finishes, so a klp relocation
against them can never resolve.

> diff --git a/tools/objtool/tests/generic/fixtures/init_reference.c b/tools/objtool/tests/generic/fixtures/init_reference.c
> new file mode 100644
> index 0000000000000..51b2a57dde14f
> --- /dev/null
> +++ b/tools/objtool/tests/generic/fixtures/init_reference.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Patched function referencing data in an .init section. */
> +
> +static const char __modinfo[]
> +	__attribute__((section(".modinfo"), used, aligned(1))) = "\0name=vmlinux";
> +
> +static int init_only __attribute__((section(".init.data"), used)) = 5;

[Severity: Low]
Can this static variable be constant-folded by the compiler since it lacks the
volatile qualifier and is never modified?

Modern compilers compiling at -O2 may constant-fold non-volatile static
variables or use symbol-relative relocations.

[ ... ]

> diff --git a/tools/objtool/tests/generic/test-init-reference.sh b/tools/objtool/tests/generic/test-init-reference.sh
> new file mode 100755
> index 0000000000000..16df80a112cfd
> --- /dev/null
> +++ b/tools/objtool/tests/generic/test-init-reference.sh

[ ... ]

> +# The rejection can only happen if the patched build really does reference the
> +# init symbol.  Were the compiler to fold the read into a constant there would
> +# be no relocation, klp diff would succeed, and the failure below would look
> +# like a missing check rather than a fixture which stopped posing the question.
> +# The reference is relocated against the section symbol rather than against
> +# init_only, since the variable is file-local -- which is the form klp diff
> +# has to recognise anyway.
> +in_relocs "$patched_obj" | grep -qE '(^|[[:space:]])\.init\.data([[:space:]]|$|[[:space:]]*[-+])' ||
> +	fail "patched object has no reference into .init.data; the fixture tests nothing"

[Severity: Low]
Does this strict check abort the test unconditionally instead of skipping it?

The check relies on a section-relative relocation and lack of constant folding.
If the compiler does constant-fold the variable or use symbol-relative
relocations, this breaks the CI/build instead of gracefully skipping via
probe_skip.

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