> commit e7a800a771162d1c8e3c4ae6c2032fa58e5ba5d9
> Author: Alan Maguire <[email protected]>
>
> selftests/bpf: Add LOC_PARAM, LOC_PROTO, LOCSEC to dedup split tests
>
> Ensure that location params/protos are deduplicated and location
> sections are not, and that references to deduplicated locations within
> location prototypes and sections are updated after deduplication.
>
> Signed-off-by: Alan Maguire <[email protected]>
The commit message states that location sections are not deduplicated,
but the test contains only a single LOCSEC and no second LOCSEC to
merge against.
A single LOCSEC survives dedup regardless of whether the
btf_is_locsec() exemption exists in btf_dedup_new(), since there's
nothing to dedup it with. Adding a second .locs LOCSEC with identical
contents (or an identical btf_loc record) and asserting both survive
would directly pin the self-canonical branch for BTF_KIND_LOCSEC.
The test does still catch total breakage, because BTF_KIND_LOCSEC has
no case in btf_dedup_ref_type() and would hit its default return
-EINVAL if it were not self-canonical, which would trip the
ASSERT_OK(err, "btf_dedup"). However, that doesn't distinguish "LOCSEC
is exempt from dedup" from "LOCSEC had nothing to dedup against."
Would a second identical LOCSEC better verify the "location sections
are not [deduplicated]" claim?
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c
> b/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c
> index 9d6161151593..34ca20f88afd 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_dedup_split.c
[ ... ]
> @@ -554,6 +554,113 @@ static void test_split_module(void)
> btf__free(vmlinux_btf);
> }
>
> +static void test_split_loc(void)
> +{
[ ... ]
> + btf2 = btf__new_empty_split(btf1);
> + if (!ASSERT_OK_PTR(btf2, "empty_split_btf"))
> + goto cleanup;
> + btf__add_loc_param(btf2, 8, BTF_LOC_PARAM_REG);
> + btf__add_loc_param_value(btf2, 1); /* [8] loc reg 1 */
> + btf__add_loc_proto(btf2); /* [9] loc proto */
> + btf__add_loc_proto_param(btf2, 5); /* param value */
> + btf__add_loc_proto_param(btf2, 8); /* param reg 1 */
> + btf__add_locsec(btf2, ".locs"); /* [9] locsec ".locs" */
^^^^
The type-ID annotation on this line is off by one. The comment says
/* [9] locsec ".locs" */ but [9] is the LOC_PROTO added three lines
earlier (also annotated /* [9] loc proto */). The LOCSEC actually gets
ID 10, as the VALIDATE_RAW_BTF block immediately below states:
"[10] LOCSEC '.locs' vlen=1\n"
"\tfunc_type_id=4 loc_proto_type_id=9 offset=128"
Traced against the builders: btf2 is split on top of btf1 (7 base
types), so btf__add_loc_param() creates [8], btf__add_loc_proto()
creates [9], and btf__add_locsec() creates [10].
In this file the /* [N] ... */ trailing comments are the only mapping
from builder calls to the expected raw-dump lines (see test_split_simple),
so a duplicated index misleads anyone updating the expectations.
Should this read /* [10] locsec ".locs" */?
> + btf__add_locsec_loc(btf2, 4, 9, 128);
> +
> + VALIDATE_RAW_BTF(
> + btf2,
[ ... ]
> + "[10] LOCSEC '.locs' vlen=1\n"
> + "\tfunc_type_id=4 loc_proto_type_id=9 offset=128");
> +
> + err = btf__dedup(btf2, NULL);
> + if (!ASSERT_OK(err, "btf_dedup"))
> + goto cleanup;
[ ... ]
> +cleanup:
> + btf__free(btf2);
> + btf__free(btf1);
> +}
> +
> void test_btf_dedup_split()
> {
> if (test__start_subtest("split_simple"))
> @@ -566,4 +673,6 @@ void test_btf_dedup_split()
> test_split_dup_struct_in_cu();
> if (test__start_subtest("split_module"))
> test_split_module();
> + if (test__start_subtest("split_loc"))
> + test_split_loc();
> }
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33537080133
