Re: [PATCH 36/58] objtool/klp: Add test for position-independent checksums

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The file header comment in checksum_position.c incorrectly claims that target() contains a loop, contradicting the implementation and commit message.
--

commit 142ca8573cda7b552fbe79f475cedfd122ac600a
Author: Song Liu <song@kernel.org>

objtool/klp: Add test for position-independent checksums

This commit introduces a test fixture and script to verify that a function's
checksum remains consistent when its position shifts due to other code changes.
The test uses a fixture built without function sections to verify the behavior
of position-dependent checksums for non-relocated calls.

> diff --git a/tools/objtool/tests/generic/fixtures/checksum_position.c b/tools/objtool/tests/generic/fixtures/checksum_position.c
> new file mode 100644
> index 0000000000000..e1362e58422b7
> --- /dev/null
> +++ b/tools/objtool/tests/generic/fixtures/checksum_position.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * A function whose position in the section changes between the two builds,
> + * without the function itself changing.
> + *
> + * PATCHED adds a function ahead of it, so target() moves.  It must be built
> + * without -ffunction-sections, or every function sits at offset 0 of its own
> + * section and nothing ever moves -- which is why the test passes
> + * -fno-function-sections.
> + *
> + * target() contains a loop, so it has a relative jump with no relocation.  The
> + * offset encoded in that instruction depends on where the function sits, and
> + * hashing those bytes makes the checksum move when the function does.
> + */

[Severity: Low]
Is this file header comment accurate regarding target() containing a loop?

The implementation of target() lower down in this file actually performs
relative calls to callee(), rather than containing a loop. Furthermore, the
commit message explicitly notes that a loop branch would keep the same
displacement wherever the function goes, making it unsuitable for this test.

Could this comment be updated to accurately reflect that target() uses
relative calls across to another function instead of an internal loop?

[ ... ]

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