* Re: [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks
2026-01-09 10:13 [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks Alan Maguire
@ 2026-01-09 10:33 ` bot+bpf-ci
2026-01-09 12:24 ` Alan Maguire
2026-01-09 16:28 ` Yonghong Song
2026-01-09 17:28 ` Andrii Nakryiko
2 siblings, 1 reply; 8+ messages in thread
From: bot+bpf-ci @ 2026-01-09 10:33 UTC (permalink / raw)
To: alan.maguire, andrii
Cc: ast, yonghong.song, jolsa, daniel, martin.lau, eddyz87, song,
john.fastabend, kpsingh, sdf, haoluo, nilay, bvanassche, bpf,
alan.maguire, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2311 bytes --]
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 84a4b0abc8be..8ef0354e66cc 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
[ ... ]
> @@ -4688,7 +4686,24 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
> return fwd_kind == real_kind;
> }
>
> - if (cand_kind != canon_kind)
> + /*
> + * Types are considered equivalent if modifiers (const, volatile,
> + * restrict) are present for one but not the other.
> + */
> + if (cand_kind != canon_kind) {
> + __u32 next_cand_id = cand_id;
> + __u32 next_canon_id = canon_id;
> +
> + if (btf_is_mod(cand_type) && !btf_is_type_tag(cand_type))
> + next_cand_id = cand_type->type;
> + if (btf_is_mod(canon_type) && !btf_is_type_tag(canon_type))
> + next_canon_id = canon_type->type;
> + if (cand_id == next_cand_id && canon_id == next_canon_id)
> + return 0;
> + return btf_dedup_is_equiv(d, next_cand_id, next_canon_id);
> + }
Can this recursive call fail when there are multiple levels of modifiers?
Consider comparing "volatile const int" (cand_id=20) against "int" (canon_id=5):
1. The first call adds hypot_map[5] = 20 via btf_dedup_hypot_map_add()
earlier in btf_dedup_is_equiv() before the kinds are checked.
2. btf_is_mod(VOLATILE) is true, so next_cand_id becomes the underlying
"const int" (ID=15), while next_canon_id stays at 5 since INT is not
a modifier.
3. The recursive call btf_dedup_is_equiv(d, 15, 5) is made.
4. In the recursive call, hypot_map[5] is already set to 20 (from step 1).
Since 20 != 15, the code falls through to btf_dedup_identical_types().
5. btf_dedup_identical_types(d, 20, 15) compares VOLATILE vs CONST kinds,
which differ, so it returns false.
6. The function returns 0, incorrectly reporting the types as not equivalent.
This appears to break the intended behavior for types with nested modifiers
like "volatile const X" vs "X", even though single modifiers like
"volatile X" vs "X" would work correctly (because next_cand_id and
next_canon_id would both resolve to the same underlying type).
---
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/20848783658
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks
2026-01-09 10:33 ` bot+bpf-ci
@ 2026-01-09 12:24 ` Alan Maguire
0 siblings, 0 replies; 8+ messages in thread
From: Alan Maguire @ 2026-01-09 12:24 UTC (permalink / raw)
To: bot+bpf-ci, andrii
Cc: ast, yonghong.song, jolsa, daniel, martin.lau, eddyz87, song,
john.fastabend, kpsingh, sdf, haoluo, nilay, bvanassche, bpf,
martin.lau, clm, ihor.solodrai
On 09/01/2026 10:33, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>> index 84a4b0abc8be..8ef0354e66cc 100644
>> --- a/tools/lib/bpf/btf.c
>> +++ b/tools/lib/bpf/btf.c
>
> [ ... ]
>
>> @@ -4688,7 +4686,24 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
>> return fwd_kind == real_kind;
>> }
>>
>> - if (cand_kind != canon_kind)
>> + /*
>> + * Types are considered equivalent if modifiers (const, volatile,
>> + * restrict) are present for one but not the other.
>> + */
>> + if (cand_kind != canon_kind) {
>> + __u32 next_cand_id = cand_id;
>> + __u32 next_canon_id = canon_id;
>> +
>> + if (btf_is_mod(cand_type) && !btf_is_type_tag(cand_type))
>> + next_cand_id = cand_type->type;
>> + if (btf_is_mod(canon_type) && !btf_is_type_tag(canon_type))
>> + next_canon_id = canon_type->type;
>> + if (cand_id == next_cand_id && canon_id == next_canon_id)
>> + return 0;
>> + return btf_dedup_is_equiv(d, next_cand_id, next_canon_id);
>> + }
>
> Can this recursive call fail when there are multiple levels of modifiers?
>
> Consider comparing "volatile const int" (cand_id=20) against "int" (canon_id=5):
>
> 1. The first call adds hypot_map[5] = 20 via btf_dedup_hypot_map_add()
> earlier in btf_dedup_is_equiv() before the kinds are checked.
>
> 2. btf_is_mod(VOLATILE) is true, so next_cand_id becomes the underlying
> "const int" (ID=15), while next_canon_id stays at 5 since INT is not
> a modifier.
>
> 3. The recursive call btf_dedup_is_equiv(d, 15, 5) is made.
>
> 4. In the recursive call, hypot_map[5] is already set to 20 (from step 1).
> Since 20 != 15, the code falls through to btf_dedup_identical_types().
>
> 5. btf_dedup_identical_types(d, 20, 15) compares VOLATILE vs CONST kinds,
> which differ, so it returns false.
>
> 6. The function returns 0, incorrectly reporting the types as not equivalent.
>
> This appears to break the intended behavior for types with nested modifiers
> like "volatile const X" vs "X", even though single modifiers like
> "volatile X" vs "X" would work correctly (because next_cand_id and
> next_canon_id would both resolve to the same underlying type).
>
>
The above is all true, but I _think_ it neglects an aspect of deduplication;
that it is symmetric; i.e. in one case a type id is canonical, and in another
it is a candidate. So for the above case, consider what happens when
cand_id=5 ("int") and canon_id=20 ("volatile const int"):
1. we add hypot_map[20] = 5 (i.e hypot_map["volatile const int"] = "int";
2. btf_is_mod(VOLATILE) is true so we recurse to btf_dedup_is_equiv(d, 5, 15)
(where 15 is "const int")
3. hypot_map[15] is not set so we do equivalence checking and recurse to
btf_dedup_is_equiv(d, 5, 5) ("int") and return 1.
4. As a result the original hypot_map["volatile const int"] = "int" is
verified.
So unless I'm missing something we end up deduplicating successfully in
this case, but the deduplication is weighted towards making the more complex
type canonical. That's what we want I think since these equivalence checks
will be just used to validate struct/union equivalence.
> ---
> 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/20848783658
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks
2026-01-09 10:13 [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks Alan Maguire
2026-01-09 10:33 ` bot+bpf-ci
@ 2026-01-09 16:28 ` Yonghong Song
2026-01-09 17:28 ` Andrii Nakryiko
2 siblings, 0 replies; 8+ messages in thread
From: Yonghong Song @ 2026-01-09 16:28 UTC (permalink / raw)
To: Alan Maguire, andrii
Cc: ast, jolsa, daniel, martin.lau, eddyz87, song, john.fastabend,
kpsingh, sdf, haoluo, nilay, bvanassche, bpf
On 1/9/26 2:13 AM, Alan Maguire wrote:
> We see identical type problems in [1] as a result of an occasionally
> applied volatile modifier to kernel data structures. Such things can
> result from different header include patterns, explicit Makefile
> rules etc. As a result consider types with modifiers const, volatile
> and restrict as equivalent for dedup equivalence testing purposes.
>
> Type tag is excluded from modifier equivalence as it would be possible
> we would end up with the type without the type tag annotations in the
> final BTF, which could potentially lead to information loss.
>
> [1] https://lore.kernel.org/bpf/42a1b4b0-83d0-4dda-b1df-15a1b7c7638d@linux.ibm.com/
>
> Reported-by: Nilay Shroff <nilay@linux.ibm.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Tested with llvm22 and gcc15 for config including CONFIG_DEBUG_INFO_BTF
and CONFIG_KCSAN. Both work properly.
Tested-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks
2026-01-09 10:13 [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks Alan Maguire
2026-01-09 10:33 ` bot+bpf-ci
2026-01-09 16:28 ` Yonghong Song
@ 2026-01-09 17:28 ` Andrii Nakryiko
2026-01-10 14:05 ` Alan Maguire
2 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2026-01-09 17:28 UTC (permalink / raw)
To: Alan Maguire
Cc: andrii, ast, yonghong.song, jolsa, daniel, martin.lau, eddyz87,
song, john.fastabend, kpsingh, sdf, haoluo, nilay, bvanassche,
bpf
On Fri, Jan 9, 2026 at 2:14 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> We see identical type problems in [1] as a result of an occasionally
> applied volatile modifier to kernel data structures. Such things can
> result from different header include patterns, explicit Makefile
> rules etc. As a result consider types with modifiers const, volatile
> and restrict as equivalent for dedup equivalence testing purposes.
>
> Type tag is excluded from modifier equivalence as it would be possible
> we would end up with the type without the type tag annotations in the
> final BTF, which could potentially lead to information loss.
Hold on... I'm not a fan of just randomly ignoring modifiers in BTF
dedup. If we think volatile is not important, let pahole just drop it.
I think BTF dedup itself shouldn't be randomly ignoring information
like this.
Better yet, of course, is to fix kernel headers to not have mismatched
type definitions, no?
pw-bot: cr
>
> [1] https://lore.kernel.org/bpf/42a1b4b0-83d0-4dda-b1df-15a1b7c7638d@linux.ibm.com/
>
> Reported-by: Nilay Shroff <nilay@linux.ibm.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> tools/lib/bpf/btf.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index b136572e889a..89fbeed948a8 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -4677,12 +4677,10 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
> cand_kind = btf_kind(cand_type);
> canon_kind = btf_kind(canon_type);
>
> - if (cand_type->name_off != canon_type->name_off)
> - return 0;
> -
> /* FWD <--> STRUCT/UNION equivalence check, if enabled */
> - if ((cand_kind == BTF_KIND_FWD || canon_kind == BTF_KIND_FWD)
> - && cand_kind != canon_kind) {
> + if ((cand_kind == BTF_KIND_FWD || canon_kind == BTF_KIND_FWD) &&
> + cand_type->name_off == canon_type->name_off &&
> + cand_kind != canon_kind) {
> __u16 real_kind;
> __u16 fwd_kind;
>
> @@ -4699,7 +4697,24 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
> return fwd_kind == real_kind;
> }
>
> - if (cand_kind != canon_kind)
> + /*
> + * Types are considered equivalent if modifiers (const, volatile,
> + * restrict) are present for one but not the other.
> + */
> + if (cand_kind != canon_kind) {
> + __u32 next_cand_id = cand_id;
> + __u32 next_canon_id = canon_id;
> +
> + if (btf_is_mod(cand_type) && !btf_is_type_tag(cand_type))
> + next_cand_id = cand_type->type;
> + if (btf_is_mod(canon_type) && !btf_is_type_tag(canon_type))
> + next_canon_id = canon_type->type;
> + if (cand_id == next_cand_id && canon_id == next_canon_id)
> + return 0;
> + return btf_dedup_is_equiv(d, next_cand_id, next_canon_id);
> + }
> +
> + if (cand_type->name_off != canon_type->name_off)
> return 0;
>
> switch (cand_kind) {
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks
2026-01-09 17:28 ` Andrii Nakryiko
@ 2026-01-10 14:05 ` Alan Maguire
2026-01-13 21:48 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Alan Maguire @ 2026-01-10 14:05 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: andrii, ast, yonghong.song, jolsa, daniel, martin.lau, eddyz87,
song, john.fastabend, kpsingh, sdf, haoluo, nilay, bvanassche,
bpf
On 09/01/2026 17:28, Andrii Nakryiko wrote:
> On Fri, Jan 9, 2026 at 2:14 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> We see identical type problems in [1] as a result of an occasionally
>> applied volatile modifier to kernel data structures. Such things can
>> result from different header include patterns, explicit Makefile
>> rules etc. As a result consider types with modifiers const, volatile
>> and restrict as equivalent for dedup equivalence testing purposes.
>>
>> Type tag is excluded from modifier equivalence as it would be possible
>> we would end up with the type without the type tag annotations in the
>> final BTF, which could potentially lead to information loss.
>
> Hold on... I'm not a fan of just randomly ignoring modifiers in BTF
> dedup. If we think volatile is not important, let pahole just drop it.
It's important to stress that the final BTF representation doesn't ignore
the volatile modifier; in fact it is included in the final BTF for the two
cases where __data_racy is used in a structure (in structs backing_dev_info
and request_queue). See my response to the AI bot for the reason we weight
towards choosing the more complete type as canonical.
> I think BTF dedup itself shouldn't be randomly ignoring information
> like this.
>
> Better yet, of course, is to fix kernel headers to not have mismatched
> type definitions, no?
>
Of course, but these are not mutually exclusive activities. Some issues
like [1] admit to such a fix fairly easily.
In this specific case however the __data_racy annotation definition depends
on __SANITIZE_THREAD__ which is set via compiler flag, and there are cases
where KCSAN is deliberately disabled; from scripts/Makefile.lib:
#
# Enable KCSAN flags except some files or directories we don't want to check
# (depends on variables KCSAN_SANITIZE_obj.o, KCSAN_SANITIZE)
#
ifeq ($(CONFIG_KCSAN),y)
_c_flags += $(if $(patsubst n%,, \
$(KCSAN_SANITIZE_$(target-stem).o)$(KCSAN_SANITIZE)$(is-kernel-object)), \
$(CFLAGS_KCSAN))
# Some uninstrumented files provide implied barriers required to avoid false
# positives: set KCSAN_INSTRUMENT_BARRIERS for barrier instrumentation only.
_c_flags += $(if $(patsubst n%,, \
$(KCSAN_INSTRUMENT_BARRIERS_$(target-stem).o)$(KCSAN_INSTRUMENT_BARRIERS)n), \
-D__KCSAN_INSTRUMENT_BARRIERS__)
endif
So there's nothing to fix for such cases; for some objects, disabling KCSAN is
intentional. Since some core .o like mm slab/slub files disable KCSAN, the
non-volatile fields proliferate widely.
[1] https://lore.kernel.org/netdev/20251121181231.64337-1-alan.maguire@oracle.com/
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks
2026-01-10 14:05 ` Alan Maguire
@ 2026-01-13 21:48 ` Andrii Nakryiko
2026-01-14 18:41 ` Alan Maguire
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2026-01-13 21:48 UTC (permalink / raw)
To: Alan Maguire
Cc: andrii, ast, yonghong.song, jolsa, daniel, martin.lau, eddyz87,
song, john.fastabend, kpsingh, sdf, haoluo, nilay, bvanassche,
bpf
On Sat, Jan 10, 2026 at 6:05 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 09/01/2026 17:28, Andrii Nakryiko wrote:
> > On Fri, Jan 9, 2026 at 2:14 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >>
> >> We see identical type problems in [1] as a result of an occasionally
> >> applied volatile modifier to kernel data structures. Such things can
> >> result from different header include patterns, explicit Makefile
> >> rules etc. As a result consider types with modifiers const, volatile
> >> and restrict as equivalent for dedup equivalence testing purposes.
> >>
> >> Type tag is excluded from modifier equivalence as it would be possible
> >> we would end up with the type without the type tag annotations in the
> >> final BTF, which could potentially lead to information loss.
> >
> > Hold on... I'm not a fan of just randomly ignoring modifiers in BTF
> > dedup. If we think volatile is not important, let pahole just drop it.
>
> It's important to stress that the final BTF representation doesn't ignore
> the volatile modifier; in fact it is included in the final BTF for the two
> cases where __data_racy is used in a structure (in structs backing_dev_info
> and request_queue). See my response to the AI bot for the reason we weight
> towards choosing the more complete type as canonical.
I'm probably very slow, but I don't see how we actually consciously
pick a fuller type definition as canonical. You have symmetrical
single-level modifier stripping for both canon and cand types. So I
don't see what prevents us from saying that `int` is the canonical
type for `volatile int` and never really have `volatile int` in a type
chain anymore.
Symmetry that you mentioned doesn't help here either, because we will
declare success on the first lucky match, regardless of which side is
volatile on (candidate or canonical).
So if we at all do this, we need to only strip modifiers on the
canonical side, so that cand=`volatile int` never matches canon=`int`,
but eventually they flip order and cand=`int` does match to `volatile
int`.
But even that gives me pause, because hypot_map on successful type
graph match will stop being "hypothetical" and will be real
(btf_dedup_merge_hypot_map), and so all `int`'s within that
compilation unit will suddenly become "volatile int".
It's just a mess. If volatile is not important, I'd rather have pahole
strip it out completely for the kernel (as an extra option). *That* I
can at least reason about in terms of consequences. While with your
patch I can't convince myself we are not introducing subtle problems.
>
> > I think BTF dedup itself shouldn't be randomly ignoring information
> > like this.
> >
> > Better yet, of course, is to fix kernel headers to not have mismatched
> > type definitions, no?
> >
>
> Of course, but these are not mutually exclusive activities. Some issues
> like [1] admit to such a fix fairly easily.
>
> In this specific case however the __data_racy annotation definition depends
> on __SANITIZE_THREAD__ which is set via compiler flag, and there are cases
> where KCSAN is deliberately disabled; from scripts/Makefile.lib:
>
> #
> # Enable KCSAN flags except some files or directories we don't want to check
> # (depends on variables KCSAN_SANITIZE_obj.o, KCSAN_SANITIZE)
> #
> ifeq ($(CONFIG_KCSAN),y)
> _c_flags += $(if $(patsubst n%,, \
> $(KCSAN_SANITIZE_$(target-stem).o)$(KCSAN_SANITIZE)$(is-kernel-object)), \
> $(CFLAGS_KCSAN))
> # Some uninstrumented files provide implied barriers required to avoid false
> # positives: set KCSAN_INSTRUMENT_BARRIERS for barrier instrumentation only.
> _c_flags += $(if $(patsubst n%,, \
> $(KCSAN_INSTRUMENT_BARRIERS_$(target-stem).o)$(KCSAN_INSTRUMENT_BARRIERS)n), \
> -D__KCSAN_INSTRUMENT_BARRIERS__)
> endif
>
> So there's nothing to fix for such cases; for some objects, disabling KCSAN is
> intentional. Since some core .o like mm slab/slub files disable KCSAN, the
> non-volatile fields proliferate widely.
>
> [1] https://lore.kernel.org/netdev/20251121181231.64337-1-alan.maguire@oracle.com/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] libbpf: BTF dedup should ignore modifiers in type equivalence checks
2026-01-13 21:48 ` Andrii Nakryiko
@ 2026-01-14 18:41 ` Alan Maguire
0 siblings, 0 replies; 8+ messages in thread
From: Alan Maguire @ 2026-01-14 18:41 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: andrii, ast, yonghong.song, jolsa, daniel, martin.lau, eddyz87,
song, john.fastabend, kpsingh, sdf, haoluo, nilay, bvanassche,
bpf
On 13/01/2026 21:48, Andrii Nakryiko wrote:
> On Sat, Jan 10, 2026 at 6:05 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> On 09/01/2026 17:28, Andrii Nakryiko wrote:
>>> On Fri, Jan 9, 2026 at 2:14 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>>
>>>> We see identical type problems in [1] as a result of an occasionally
>>>> applied volatile modifier to kernel data structures. Such things can
>>>> result from different header include patterns, explicit Makefile
>>>> rules etc. As a result consider types with modifiers const, volatile
>>>> and restrict as equivalent for dedup equivalence testing purposes.
>>>>
>>>> Type tag is excluded from modifier equivalence as it would be possible
>>>> we would end up with the type without the type tag annotations in the
>>>> final BTF, which could potentially lead to information loss.
>>>
>>> Hold on... I'm not a fan of just randomly ignoring modifiers in BTF
>>> dedup. If we think volatile is not important, let pahole just drop it.
>>
>> It's important to stress that the final BTF representation doesn't ignore
>> the volatile modifier; in fact it is included in the final BTF for the two
>> cases where __data_racy is used in a structure (in structs backing_dev_info
>> and request_queue). See my response to the AI bot for the reason we weight
>> towards choosing the more complete type as canonical.
>
> I'm probably very slow, but I don't see how we actually consciously
> pick a fuller type definition as canonical. You have symmetrical
> single-level modifier stripping for both canon and cand types. So I
> don't see what prevents us from saying that `int` is the canonical
> type for `volatile int` and never really have `volatile int` in a type
> chain anymore.
>
> Symmetry that you mentioned doesn't help here either, because we will
> declare success on the first lucky match, regardless of which side is
> volatile on (candidate or canonical).
>
> So if we at all do this, we need to only strip modifiers on the
> canonical side, so that cand=`volatile int` never matches canon=`int`,
> but eventually they flip order and cand=`int` does match to `volatile
> int`.
I belatedly realized we need to handle the module case too; so we really
need two-way matching. Check out the test in [1]; it demonstrates the problem
and it failed with my original patch. By not populating the hypot match
we have proper two-way matching which we need for edge cases like module/base
BTF dedup and cases where one type has a modifier and non-modifier field while
another has another set of field modifiers; in such a case neither is more
complete than the other. The only answer is to match in both directions to
handle such cases.
>
> But even that gives me pause, because hypot_map on successful type
> graph match will stop being "hypothetical" and will be real
> (btf_dedup_merge_hypot_map), and so all `int`'s within that
> compilation unit will suddenly become "volatile int".
>
> It's just a mess. If volatile is not important, I'd rather have pahole
> strip it out completely for the kernel (as an extra option). *That* I
> can at least reason about in terms of consequences. While with your
> patch I can't convince myself we are not introducing subtle problems.
>
I've tried adding a test exercising complicated edge cases; let me know if
there's additional cases you can think of that might help convice the
approach is safe. Thanks!
Alan
[1] https://lore.kernel.org/bpf/20260114183808.2946395-1-alan.maguire@oracle.com/
>>
>>> I think BTF dedup itself shouldn't be randomly ignoring information
>>> like this.
>>>
>>> Better yet, of course, is to fix kernel headers to not have mismatched
>>> type definitions, no?
>>>
>>
>> Of course, but these are not mutually exclusive activities. Some issues
>> like [1] admit to such a fix fairly easily.
>>
>> In this specific case however the __data_racy annotation definition depends
>> on __SANITIZE_THREAD__ which is set via compiler flag, and there are cases
>> where KCSAN is deliberately disabled; from scripts/Makefile.lib:
>>
>> #
>> # Enable KCSAN flags except some files or directories we don't want to check
>> # (depends on variables KCSAN_SANITIZE_obj.o, KCSAN_SANITIZE)
>> #
>> ifeq ($(CONFIG_KCSAN),y)
>> _c_flags += $(if $(patsubst n%,, \
>> $(KCSAN_SANITIZE_$(target-stem).o)$(KCSAN_SANITIZE)$(is-kernel-object)), \
>> $(CFLAGS_KCSAN))
>> # Some uninstrumented files provide implied barriers required to avoid false
>> # positives: set KCSAN_INSTRUMENT_BARRIERS for barrier instrumentation only.
>> _c_flags += $(if $(patsubst n%,, \
>> $(KCSAN_INSTRUMENT_BARRIERS_$(target-stem).o)$(KCSAN_INSTRUMENT_BARRIERS)n), \
>> -D__KCSAN_INSTRUMENT_BARRIERS__)
>> endif
>>
>> So there's nothing to fix for such cases; for some objects, disabling KCSAN is
>> intentional. Since some core .o like mm slab/slub files disable KCSAN, the
>> non-volatile fields proliferate widely.
>>
>> [1] https://lore.kernel.org/netdev/20251121181231.64337-1-alan.maguire@oracle.com/
^ permalink raw reply [flat|nested] 8+ messages in thread