[BPF] Handle nested wrapper structs in BPF map definition traversal

[vadorovsky]

In Aya/Rust, BPF map definitions are nested in two nested types:

• A struct representing the map type (e.g., HashMap, RingBuf) that provides methods for interacting with the map type (e.g. HashMap::get, RingBuf::reserve).
• An UnsafeCell, which informs the Rust compiler that the type is thread-safe and can be safely mutated even as a global variable. The kernel guarantees map operation safety.

This leads to a type hierarchy like:

    pub struct HashMap<K, V, const M: usize, const F: usize = 0>(
        core::cell::UnsafeCell<HashMapDef<K, V, M, F>>,
    );
    const BPF_MAP_TYPE_HASH: usize = 1;
    pub struct HashMapDef<K, V, const M: usize, const F: usize = 0> {
        r#type: *const [i32; BPF_MAP_TYPE_HASH],
        key: *const K,
        value: *const V,
        max_entries: *const [i32; M],
        map_flags: *const [i32; F],
    }

Then used in the BPF program code as a global variable:

    #[link_section = ".maps"]
    static HASH_MAP: HashMap<u32, u32, 1337> = HashMap::new();

Which is an equivalent of the following BPF map definition in C:

    #define BPF_MAP_TYPE_HASH 1
    struct {
        int (*type)[BPF_MAP_TYPE_HASH];
        typeof(int) *key;
        typeof(int) *value;
        int (*max_entries)[1337];
    } map_1 __attribute__((section(".maps")));

Accessing the actual map definition requires traversing:

  HASH_MAP -> __0 -> value

Previously, the BPF backend only visited the pointee types of the outermost struct, and didn’t descend into inner wrappers. This caused issues when the key/value types were custom structs:

    // Define custom structs for key and values.
    pub struct MyKey(u32);
    pub struct MyValue(u32);

    #[link_section = ".maps"]
    #[export_name = "HASH_MAP"]
    pub static HASH_MAP: HashMap<MyKey, MyValue, 10> = HashMap::new();

These types weren’t fully visited and appeared in BTF as forward declarations:

    #30: <FWD> 'MyKey' kind:struct
    #31: <FWD> 'MyValue' kind:struct

The fix is to enhance visitMapDefType to recursively visit inner composite members. If a member is a composite type (likely a wrapper), it is now also visited using visitMapDefType, ensuring that the pointee types of the innermost stuct members, like MyKey and MyValue, are fully resolved in BTF.

With this fix, the correct BTF entries are emitted:

    #6: <STRUCT> 'MyKey' sz:4 n:1
            #00 '__0' off:0 --> [7]
    #7: <INT> 'u32' bits:32 off:0
    #8: <PTR> --> [9]
    #9: <STRUCT> 'MyValue' sz:4 n:1
            #00 '__0' off:0 --> [7]

Fixes: #143361

[tamird]

doing this check inside the loop is a bit odd, since it will only ever happen if the loop runs for exactly one iteration. at the very least I think this should move outside the loop body, but also I wonder if there's a more sensible way to do this. What if we decide to have more than one element in the wrapper struct in the future?

[vadorovsky]

but also I wonder if there's a more sensible way to do this.

Good point. Assuming that BPF map definitions should have only pointers and arrays, maybe we could assume that any DICompositeType element is a wrapper?

What if we decide to have more than one element in the wrapper struct in the future?

Hard to imagine that happening, but the idea of recursively looking into DICompositeType elements should solve it.

[vadorovsky]

Well, with the new logic (detecting whether a given field holds DICompositeType), the if statement has to be inside the loop. PTAL

[Copilot]

Pull Request Overview

This PR enhances the BTF map-definition traversal in visitMapDefType to correctly descend through nested wrapper structs and ensure inner pointee types are fully visited.

• Detect composite wrapper members and recurse with visitMapDefType instead of visitTypeEntry.
• Preserve generic type visitation for non-composite members.
• Updated comments to explain the new traversal logic.

Comments suppressed due to low confidence (1)

llvm/lib/Target/BPF/BTFDebug.cpp:991

• [nitpick] The variable name MemberCTy is abbreviated; consider renaming it to MemberCompositeType for improved clarity.

const auto *MemberCTy = dyn_cast<DICompositeType>(MemberBaseType);

[eddyz87]

Could you please take a look at the test case llvm/test/CodeGen/BPF/BTF/unreachable.ll?
There and in a few other places we started using a small python script, print_btf.py, to print out BTF in tests in a readable form.

[vadorovsky]

Will do that, I completely missed the print_btf.py utility.

I wrote this test in the same style as the other map-def*.ll tests, which unfortunately don't use print_btf.py and perform manual assembly assertions. Would you be open for switching them as well? I could create a new PR for that.

[eddyz87]

Could you please try to minimize your test case?
As far as I understand, the intended use case looks as follows in C:

$ cat test_struct_dbg.c
struct key { int i; };
struct val { int j; };

#define __uint(name, val) int (*name)[val]
#define __type(name, val) typeof(val) *name

struct {
        __uint(type, 1);
        __uint(max_entries, 1);
        __type(key, struct key);
        __type(value, struct val);
} map __attribute__((section(".maps")));

And it generates a much smaller amount of IR:

$ clang -g -emit-llvm -S test_struct_dbg.c -o - | wc -l
45

[vadorovsky]

Such test wouldn't be really relevant to the issue that this PR is trying to fix. The simple, not nested map structs in C, like the one you wrote, always worked fine.

What doesn't work is wrapping such map structs in nested structs, which is necessary in Rust. I tried my best explaining that in the PR description and commit message, please let me know if something is unclear there.

The C code, capable of reproducing the issue, would look like:

struct hash_map {
    struct {
        __uint(type, 1);
        __uint(max_entries, 1);
        __type(key, struct key);
        __type(value, struct val);
    } __0;
}:

struct hash_map map __attribute__((section(".maps")));

where the actual map definition is wrapped in some other type.

[eddyz87]

Accessing the actual map definition requires traversing:

 HASH_MAP -> __0 -> value

I don't understand where the __0 comes from? Is it a field of a structure generated for pub struct MyKey(u32)?

[eddyz87]

A side note: is there a way to kick Copilot out of reviewers? It just produces random noise.

[yonghong-song]

IIUC, rust compiler will generate a struct chain like 'HashMap.__0.UnsafeCall.value.HashMapDef' and the last one 'HashMapDef' is the actual hash map definition based on C code.

From that perspective, yes, your patch MAY make sense since all previous structs like 'HashMap', '__0', 'UnsafeCall' and 'value' are all internally created by rust for some safety checking so recursively going through visitMapDefType() does make sense for this case.

Please provide a simpler test with only necessary part so it can be checked easily. Also please use BTF/print_btf.py as @eddyz87 suggested.

[vadorovsky]

@eddyz87

Accessing the actual map definition requires traversing:

 HASH_MAP -> __0 -> value

I don't understand where the __0 comes from? Is it a field of a structure generated for pub struct MyKey(u32)?

It comes from the HashMap struct:

pub struct HashMap<K, V, const M: usize, const F: usize = 0>(
    core::cell::UnsafeCell<HashMapDef<K, V, M, F>>,
);

Such struct, without named fields, is a "tuple struct". Rust assigns the field names __n for each tuple element, starting from 0. That's why the HashMap has a field __0 of UnsafeCell type.

We define the HashMap struct as the end type that developers using Aya are supposed to use. That struct has methods related to the given map type.

UnsafeCell is a struct coming from core library in Rust (a minified version of standard library) with the following definition:

pub struct UnsafeCell<T: ?Sized> {
    value: T,
}

We hold the actual BPF map definition in that value field. The purpose of UnsafeCell is telling the Rust compiles "hey, the wrapped type is safe to mutate by multiple, concurrent users". Without using UnsafeCell, it would be impossible to interact with a map (defined as a global static variable in program code) without writing unsafe code.

Given all of that, as @yonghong-song said, the chain is HashMap.__0.UnsafeCell.value.HashMapDef.

[vadorovsky]

@yonghong-song

IIUC, rust compiler will generate a struct chain like 'HashMap.__0.UnsafeCall.value.HashMapDef' and the last one 'HashMapDef' is the actual hash map definition based on C code.

That's exactly right.

From that perspective, yes, your patch MAY make sense since all previous structs like 'HashMap', '__0', 'UnsafeCall' and 'value' are all internally created by rust for some safety checking so recursively going through visitMapDefType() does make sense for this case.

Yes, that was my way of thinking.

Please provide a simpler test with only necessary part so it can be checked easily.

Unfortunately, the Rust code I used for generating the IR is the simpliest one which reproduces the issue and actually compiles. To be precise, this is the code:

#![no_std]
#![no_main]
#![allow(dead_code)]

pub const BPF_MAP_TYPE_HASH: usize = 1;

// The real map definition.
pub struct HashMapDef<K, V, const M: usize, const F: usize> {
    r#type: *const [i32; BPF_MAP_TYPE_HASH],
    key: *const K,
    value: *const V,
    max_entries: *const [i32; M],
    map_flags: *const [i32; F],
}
impl<K, V, const M: usize, const F: usize> HashMapDef<K, V, M, F> {
    pub const fn new() -> Self {
        Self {
            r#type: &[0i32; BPF_MAP_TYPE_HASH],
            key: ::core::ptr::null(),
            value: ::core::ptr::null(),
            max_entries: &[0i32; M],
            map_flags: &[0i32; F],
        }
    }
}
// Use `UnsafeCell` to allow the mutability by multiple threads.
pub struct HashMap<K, V, const M: usize, const F: usize = 0>(
    core::cell::UnsafeCell<HashMapDef<K, V, M, F>>,
);
impl<K, V, const M: usize, const F: usize> HashMap<K, V, M, F> {
    pub const fn new() -> Self {
        Self(core::cell::UnsafeCell::new(HashMapDef::new()))
    }
}
/// Tell Rust that `HashMap` is thread-safe.
unsafe impl<K: Sync, V: Sync, const M: usize, const F: usize> Sync for HashMap<K, V, M, F> {}

// Define custom structs for key and values.
pub struct MyKey(u32);
pub struct MyValue(u32);

#[link_section = ".maps"]
#[export_name = "HASH_MAP"]
pub static HASH_MAP: HashMap<MyKey, MyValue, 1337> = HashMap::new();

#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
    loop {}
}

Everything except the #[panic_handler], is essential for the map type to be complete. #[panic_handler] is mandatory in #![no_std] programs. Without it, the code wouldn't compile.

All the types present in LLVM DebugInfo which don't look obviously related to the map types, are either dependencies of PanicInfo or UnsafeCell.

Also please use BTF/print_btf.py as @eddyz87 suggested.

Sure! Thanks for letting me know about the tool. I was mostly looking at other map*.ll test, which don't use this script. I'm happy to file PRs porting the other map tests to use the script, if you are open to that.

[vadorovsky]

Hmm... when using print_btf.py, I'm getting bunch of name_off should be 0 but is N warnings:

$ ./build/bin/llc -mtriple=bpfel -mcpu=v3 -filetype=obj -o output llvm/test/CodeGen/BPF/BTF/map-def-nested.ll

$ ./build/bin/llvm-objcopy --dump-section='.BTF'=output_btf output

$ python llvm/test/CodeGen/BPF/BTF/print_btf.py output_btf
[1] PTR '*const [i32; 1]' type_id=3
<1> name_off should be 0 but is 1
[2] INT 'i32' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
[3] ARRAY '(anon)' type_id=2 index_type_id=4 nr_elems=1
[4] INT '__ARRAY_SIZE_TYPE__' size=4 bits_offset=0 nr_bits=32 encoding=(none)
[5] PTR '*const map_def::MyKey' type_id=6
<5> name_off should be 0 but is 41
[6] STRUCT 'MyKey' size=4 vlen=1
        '__0' type_id=7 bits_offset=0
[7] INT 'u32' size=4 bits_offset=0 nr_bits=32 encoding=(none)
[8] PTR '*const map_def::MyValue' type_id=9
<8> name_off should be 0 but is 77
[9] STRUCT 'MyValue' size=4 vlen=1
        '__0' type_id=7 bits_offset=0
[10] PTR '*const [i32; 1337]' type_id=11
<10> name_off should be 0 but is 109
[11] ARRAY '(anon)' type_id=2 index_type_id=4 nr_elems=1337
[12] PTR '*const [i32; 0]' type_id=13
<12> name_off should be 0 but is 128
[13] ARRAY '(anon)' type_id=2 index_type_id=4 nr_elems=0
[14] STRUCT 'HashMapDef<map_def::MyKey, map_def::MyValue, 1337, 0>' size=40 vlen=5
        'type' type_id=1 bits_offset=0
        'key' type_id=5 bits_offset=64
        'value' type_id=8 bits_offset=128
        'max_entries' type_id=10 bits_offset=192
        'map_flags' type_id=12 bits_offset=256
[15] STRUCT 'UnsafeCell<map_def::HashMapDef<map_def::MyKey, map_def::MyValue, 1337, 0>>' size=40 vlen=1
        'value' type_id=14 bits_offset=0
[16] STRUCT 'HashMap<map_def::MyKey, map_def::MyValue, 1337, 0>' size=40 vlen=1
        '__0' type_id=15 bits_offset=0
[17] VAR 'HASH_MAP' type_id=16, linkage=global
[18] PTR '&core::panic::panic_info::PanicInfo' type_id=19
<18> name_off should be 0 but is 370
[19] STRUCT 'PanicInfo' size=24 vlen=4
        'message' type_id=20 bits_offset=0
        'location' type_id=21 bits_offset=64
        'can_unwind' type_id=22 bits_offset=128
        'force_no_backtrace' type_id=22 bits_offset=136
[20] PTR '&core::fmt::Arguments' type_id=27
<20> name_off should be 0 but is 463
[21] PTR '&core::panic::location::Location' type_id=28
<21> name_off should be 0 but is 485
[22] INT 'bool' size=1 bits_offset=0 nr_bits=8 encoding=BOOL
[23] FUNC_PROTO '(anon)' ret_type_id=0 vlen=1
        '_info' type_id=18
[24] FUNC 'panic' type_id=23 linkage=static
[25] DATASEC '.maps' size=0 vlen=1
        type_id=17 offset=0 size=40
[26] DATASEC '.rodata' size=0 vlen=0
[27] FWD 'Arguments' fwd_kind=struct
[28] FWD 'Location' fwd_kind=struct

I haven't seen anything like that when using btfdump on the same object file, generated from the same Rust code.

Do we have a strict requirement of PTR types being anonymous?

I will try to figure that out on my own, but if you have any ideas what's wrong here, please let me know.

[vadorovsky]

Nevermind, I figured that out. Named pointer types are totally normal in Rust and the print_bpf.py script was the only place in the entire monorepo enforcing their anonimity. I pushed a separate commit fixing that, please let me know if you agree with the change.

Linux kernel might have some requirement around that, given that we already have a fixup for that in bpf-linker:

https://github.com/aya-rs/bpf-linker/blob/79503ff5daccf4137576fb094f1da264066367a2/src/llvm/di.rs#L197-L200

But I think we should stop enforcing that. If the kernel really has such a requirement, I'm happy to prepare a fix and propose it to ML. I will take a look this week.

What I can say for now, is that on LLVM's side, emitting BTF with named pointers works just fine.

[eddyz87]

@vadorovsky

But I think we should stop enforcing that. If the kernel really has such a requirement, I'm happy to prepare a fix and propose it to ML. I will take a look this week.

I took a quick look at kernel/bpf/{verifier,btf}.c and did not see any places where anonymity of pointers is enforced, did not test, though. I think I added this check in print_btf.py only because C backend generates anonymous pointers. You can either adjust print_btf.py to remove the warning, or proceed with your other change that makes rust pointers anonymous. Note, however, that pointer names serve no purpose on kernel side, so this is just a bloat in the BTF strings section.

[eddyz87]

@vadorovsky ,

Unfortunately, the Rust code I used for generating the IR is the simpliest one which reproduces the issue and actually compiles. To be precise, this is the code:

I mean, you can use my C example, it tests the same thing:

struct key { int i; };
struct val { int j; };

#define __uint(name, val) int (*name)[val]
#define __type(name, val) typeof(val) *name

struct {
        __uint(type, 1);
        __uint(max_entries, 1);
        __type(key, struct key);
        __type(value, struct val);
} map __attribute__((section(".maps")));

[eddyz87]

@vadorovsky

But I think we should stop enforcing that. If the kernel really has such a requirement, I'm happy to prepare a fix and propose it to ML. I will take a look this week.

I took a quick look at kernel/bpf/{verifier,btf}.c and did not see any places where anonymity of pointers is enforced, did not test, though. I think I added this check in print_btf.py only because C backend generates anonymous pointers. You can either adjust print_btf.py to remove the warning, or proceed with your other change that makes rust pointers anonymous. Note, however, that pointer names serve no purpose on kernel side, so this is just a bloat in the BTF strings section.

Disregard my previous comment, here is a point in the kernel source code that checks for pointers to be w/o names:

• https://elixir.bootlin.com/linux/v6.15.2/source/kernel/bpf/btf.c#L2817
• https://elixir.bootlin.com/linux/v6.15.2/source/kernel/bpf/btf.c#L2568

Function btf.c:btf_ref_type_check_meta().

[vadorovsky]

@eddyz87

I mean, you can use my C example, it tests the same thing:

struct key { int i; };
struct val { int j; };

#define __uint(name, val) int (*name)[val]
#define __type(name, val) typeof(val) *name

struct {
        __uint(type, 1);
        __uint(max_entries, 1);
        __type(key, struct key);
        __type(value, struct val);
} map __attribute__((section(".maps")));

OK, I can write an example in C which is capable of reproducing the issue.

To be precise, the map struct itself needs to be nested, so I will try something like:

struct key { int i; };
struct val { int j; };

#define __uint(name, val) int (*name)[val]
#define __type(name, val) typeof(val) *name

struct {
   struct {
        __uint(type, 1);
        __uint(max_entries, 1);
        __type(key, struct key);
        __type(value, struct val);
    } map_def;
} map __attribute__((section(".maps")));

On a side note, I know that such nested definition doesn't work on libbpf. I made it work in Aya by modyfing the logic for parsing BTF maps. It works fine and the kernel is able to work with such program.

https://github.com/vadorovsky/aya/blob/8d6e12107ad0a0c4d72459c56fcc9c8d78fb6c90/aya-obj/src/obj.rs#L1275-L1367

I'm happy to port that to libbpf eventually, if there is interest.

Disregard my previous comment, here is a point in the kernel source code that checks for pointers to be w/o names:

* https://elixir.bootlin.com/linux/v6.15.2/source/kernel/bpf/btf.c#L2817

* https://elixir.bootlin.com/linux/v6.15.2/source/kernel/bpf/btf.c#L2568

Function btf.c:btf_ref_type_check_meta().

I see. Would you be still open to relaxing print_btf.py (which I already pushed as a prerequisite commit) and me sending a patch relaxing that requirement in the kernel as well? Even though we currently do a fixup in bpf-linker, I think the right thing to do is to teach kernel to accept BTF produced by Rust and gradually remove the necessity of fxups on Aya/bpf-linker's side. I believe that approach would be helpful also for Rust-for-Linux folks, as they would eventually like to generate BTF for kernel modules written in Rust. It would be great if they don't have to do such fixups on their side.

[eddyz87]

@vadorovsky,

On a side note, I know that such nested definition doesn't work on libbpf. I made it work in Aya by modyfing the logic for parsing BTF maps. It works fine and the kernel is able to work with such program.

Noted, I think it's fine, this is a unit test for some backend functionality it doesn't need to be something that fully works end-to-end from Rust/C to kernel.

I'm happy to port that to libbpf eventually, if there is interest.

libbpf is C oriented and I don't think there is a use-case, but you can always ask on the mailing list.

I see. Would you be still open to relaxing print_btf.py (which I already pushed as a prerequisite commit)

The name is printed anyways, so signal is not lost => removing the warning from print_btf.py is fine.

and me sending a patch relaxing that requirement in the kernel as well?

As I said, there is no functional reason for pointer types to have names in BTF, any strings encoding such names would unnecessarily use space in the .BTF strings section. Hence, I think upstream kernel might object, but who knows.

By the way, I encourage you to make a small test for yourself, there might be some differences between BTF validation rules applied to kernel/module BTF and program BTF.

[vadorovsky]

@eddyz87

As I said, there is no functional reason for pointer types to have names in BTF, any strings encoding such names would unnecessarily use space in the .BTF strings section. Hence, I think upstream kernel might object, but who knows.

OK, the point about no functional reason for named pointers sounds reasonable. Named pointer types in Rust are convenient for debuggers, but are indeed of no use in the kernel/BPF world. And the goal of BTF is to be small.

That makes me think of a completely opposite idea - what if we remove pointer names here, in the BPF backend in LLVM? Since named pointers are correct from the point of view of LLVM debug info, but we don't want them to bloat BTF, then the LLVM backend seems like a correct place for stripping unnecessary information.

[vadorovsky]

We can disregard my last comment, at least in the context of this PR. After rewriting the codegen test in C, there are no named pointers, so print_btf.py doesn't complain.

The main issue (supporting nested maps) is anyway fixed. @eddyz87 @yonghong-song Please let me know if the changes look good to you now.

[eddyz87]

I did a sanity check with BPF selftests, all are passing. Also there are no significant changes in the generated BTF (there is some slight reordering, but that's not important).

[eddyz87]

@vadorovsky ,

That makes me think of a completely opposite idea - what if we remove pointer names here, in the BPF backend in LLVM?

This can be done, makes sense to me.

[vadorovsky]

I don't have commit access, so somebody else needs to merge

[tamird]

I don't have commit access, so somebody else needs to merge

I don't anymore either.

[tamird]

some paths leaked into this, is that typical?

[vadorovsky]

It is if you use clang built from local sources - usually the git URI gets applied to the version string. I can sanitize it - maybe by changing it to llvm/llvm-project?

[tamird]

llvm-project/llvm/test/CodeGen/BPF/BTF/annot-dup-ptr-struct.ll

Line 57 in c3929fb

!7 = !{!"clang version 20.1.6 (https://github.com/llvm/llvm-project.git aa804fd3e624cb92c6e7665182504c6049387f35)"}

looks like that's more common.

not also the occurrences on line 84 and 85.

[vadorovsky]

OK, done. I also sanitized the source dir to /tmp.

[yonghong-song]

Ok, the change looks okay to me. Echoing to the previous comment from @eddyz87, nested wrapper structs won't work for libbpf, but libbpf is mostly for C. We have libbpf-rs but that is just a wrapper for C version of libbpf. Since your nested wrapper is only for rust based libbpf, so it should be okay for now.