From 3a9e1d517eb883cb31e3630bbb4188cc027d540c Mon Sep 17 00:00:00 2001 From: Vadim Vetrov Date: Thu, 22 May 2025 19:16:30 +0300 Subject: [PATCH 1/3] libbpf-cargo: Relative anonymous type naming for struct fields Previously, anonymous types were named as __anon_{id} with id as a global counter. This approach caused cascading renaming of all subsequent anonymous definitions after any C anonymous struct order/amount change. This was a major design problem not for only development but even portability, since here may be the case when something was changed in vmlinux.h leading to compilation errors. This commit addresses the issue by localizing cascading effects to the local struct layer rather than the global layer. While the solution is not fully ideal - cascading still occurs within individual structs - it eliminates the need for wide Rust code modifications when the order of anonymous struct definitions changes. Note, that the commit does not fully eliminates __anon_{id}, but the most important part of it. The commit is breaking and should be included in major version update. --- libbpf-cargo/src/gen/btf.rs | 64 +++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/libbpf-cargo/src/gen/btf.rs b/libbpf-cargo/src/gen/btf.rs index a79c6903..85c5f0f4 100644 --- a/libbpf-cargo/src/gen/btf.rs +++ b/libbpf-cargo/src/gen/btf.rs @@ -10,6 +10,7 @@ use std::fmt::Write; use std::mem::size_of; use std::num::NonZeroUsize; use std::ops::Deref; +use std::rc::Rc; use anyhow::anyhow; use anyhow::bail; @@ -367,6 +368,13 @@ fn escape_reserved_keyword(identifier: Cow<'_, str>) -> Cow<'_, str> { } } +#[derive(Debug, Clone)] +pub struct BtfDependency { + pub name: Option, + pub dep_id: i32, + pub child_counter: Rc>, +} + #[derive(Debug, Default)] pub(crate) struct TypeMap { /// A mapping from type to number, allowing us to assign numbers to @@ -377,15 +385,70 @@ pub(crate) struct TypeMap { /// Mapping from type name to the number of times we have seen this /// name already. names_count: RefCell>, + + dependencies: RefCell>, } impl TypeMap { + pub fn derive_parent<'s>(&self, ty: &BtfType<'s>, parent: &BtfType<'s>) { + let mut deps = self.dependencies.borrow_mut(); + if deps.get(&ty.type_id()).is_some() { + return; + } + + let parent_dep = deps.get(&parent.type_id()); + if let Some(pdep) = parent_dep { + let mut dep = pdep.clone(); + + if let Some(n) = parent.name() { + dep.name = Some(n.to_string_lossy().to_string()); + } + if ty.name().is_some() { + dep.child_counter = Rc::new(RefCell::new(0)); + } + + let parent_counter = Rc::>::clone(&pdep.child_counter); + *parent_counter.borrow_mut() += 1; + dep.dep_id = *parent_counter.borrow(); + + deps.insert(ty.type_id(), dep); + } else { + let mut dep = BtfDependency { + name: None, + dep_id: 0, + child_counter: Rc::new(RefCell::new(1)), + }; + deps.insert(parent.type_id(), dep.clone()); + + if let Some(n) = parent.name() { + dep.name = Some(n.to_string_lossy().to_string()); + } + if ty.name().is_some() { + dep.child_counter = Rc::new(RefCell::new(0)); + } + dep.dep_id = 1; + deps.insert(ty.type_id(), dep); + } + } + + pub fn lookup_parent<'s>(&self, ty: &BtfType<'s>) -> Option { + self.dependencies.borrow().get(&ty.type_id()).cloned() + } + pub fn type_name_or_anon<'s>(&self, ty: &BtfType<'s>) -> Cow<'s, str> { match ty.name() { None => { let mut anon_table = self.types.borrow_mut(); let len = anon_table.len() + 1; // use 1 index anon ids for backwards compat let anon_id = anon_table.entry(ty.type_id()).or_insert(len); + + if let Some(parent) = self.lookup_parent(ty) { + if let Some(name) = parent.name { + if !name.is_empty() { + return format!("{ANON_PREFIX}{}_{}", name, parent.dep_id).into(); + } + } + } format!("{ANON_PREFIX}{anon_id}").into() } Some(n) => match self.names.borrow_mut().entry(ty.type_id()) { @@ -726,6 +789,7 @@ impl<'s> GenBtf<'s> { } if let Some(next_ty_id) = next_type(field_ty)? { + self.type_map.derive_parent(&next_ty_id, &t); dependent_types.push(next_ty_id); } let field_name = if let Some(name) = member.name { From 4f4c5f93d94ee5801a626ec5f6ba792d02134789 Mon Sep 17 00:00:00 2001 From: Vadim Vetrov Date: Thu, 22 May 2025 20:10:22 +0300 Subject: [PATCH 2/3] libbpf-cargo: Fix broken tests for 3a9e1d5 --- libbpf-cargo/src/test.rs | 98 ++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/libbpf-cargo/src/test.rs b/libbpf-cargo/src/test.rs index cc249314..a0253407 100644 --- a/libbpf-cargo/src/test.rs +++ b/libbpf-cargo/src/test.rs @@ -2514,24 +2514,24 @@ struct Foo foo; #[repr(C)] pub struct Foo { pub x: i32, - pub bar: __anon_1, + pub bar: __anon_Foo_1, pub __pad_36: [u8; 4], - pub baz: __anon_2, + pub baz: __anon_Foo_2, pub w: i32, pub __pad_52: [u8; 4], } #[derive(Copy, Clone)] #[repr(C)] -pub union __anon_1 { +pub union __anon_Foo_1 { pub y: [u8; 10], pub z: [u16; 16], } -impl std::fmt::Debug for __anon_1 { +impl std::fmt::Debug for __anon_Foo_1 { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "(???)") } } -impl Default for __anon_1 { +impl Default for __anon_Foo_1 { fn default() -> Self { Self { y: [u8::default(); 10], @@ -2540,16 +2540,16 @@ impl Default for __anon_1 { } #[derive(Copy, Clone)] #[repr(C)] -pub union __anon_2 { +pub union __anon_Foo_2 { pub w: u32, pub u: *mut u64, } -impl std::fmt::Debug for __anon_2 { +impl std::fmt::Debug for __anon_Foo_2 { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "(???)") } } -impl Default for __anon_2 { +impl Default for __anon_Foo_2 { fn default() -> Self { Self { w: u32::default(), @@ -2594,25 +2594,25 @@ struct Foo foo; #[repr(C)] pub struct Foo { pub x: i32, - pub bar: __anon_1, - pub baz: __anon_2, + pub bar: __anon_Foo_1, + pub baz: __anon_Foo_2, pub w: i32, pub __pad_68: [u8; 4], } #[derive(Debug, Default, Copy, Clone)] #[repr(C)] -pub struct __anon_1 { +pub struct __anon_Foo_1 { pub y: [u8; 10], pub z: [u16; 16], } #[derive(Debug, Copy, Clone)] #[repr(C)] -pub struct __anon_2 { +pub struct __anon_Foo_2 { pub w: u32, pub __pad_4: [u8; 4], pub u: *mut u64, } -impl Default for __anon_2 { +impl Default for __anon_Foo_2 { fn default() -> Self { Self { w: u32::default(), @@ -2667,31 +2667,31 @@ struct Foo foo; #[repr(C)] pub struct Foo { pub x: i32, - pub bar: __anon_1, - pub zerg: __anon_2, - pub baz: __anon_3, + pub bar: __anon_Foo_1, + pub zerg: __anon_Foo_2, + pub baz: __anon_Foo_3, pub w: i32, pub __pad_76: [u8; 4], - pub flarg: __anon_4, + pub flarg: __anon_Foo_4, } #[derive(Debug, Default, Copy, Clone)] #[repr(C)] -pub struct __anon_1 { +pub struct __anon_Foo_1 { pub y: [u8; 10], pub z: [u16; 16], } #[derive(Copy, Clone)] #[repr(C)] -pub union __anon_2 { +pub union __anon_Foo_2 { pub a: *mut i8, pub b: i32, } -impl std::fmt::Debug for __anon_2 { +impl std::fmt::Debug for __anon_Foo_2 { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "(???)") } } -impl Default for __anon_2 { +impl Default for __anon_Foo_2 { fn default() -> Self { Self { a: std::ptr::null_mut(), @@ -2700,12 +2700,12 @@ impl Default for __anon_2 { } #[derive(Debug, Copy, Clone)] #[repr(C)] -pub struct __anon_3 { +pub struct __anon_Foo_3 { pub w: u32, pub __pad_4: [u8; 4], pub u: *mut u64, } -impl Default for __anon_3 { +impl Default for __anon_Foo_3 { fn default() -> Self { Self { w: u32::default(), @@ -2716,16 +2716,16 @@ impl Default for __anon_3 { } #[derive(Copy, Clone)] #[repr(C)] -pub union __anon_4 { +pub union __anon_Foo_4 { pub c: u8, pub d: [u64; 5], } -impl std::fmt::Debug for __anon_4 { +impl std::fmt::Debug for __anon_Foo_4 { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "(???)") } } -impl Default for __anon_4 { +impl Default for __anon_Foo_4 { fn default() -> Self { Self { c: u8::default(), @@ -2763,20 +2763,20 @@ struct Foo foo = {{0}}; #[derive(Debug, Default, Copy, Clone)] #[repr(C)] pub struct Foo { - pub __anon_1: __anon_1, + pub __anon_Foo_1: __anon_Foo_1, } #[derive(Copy, Clone)] #[repr(C)] -pub union __anon_1 { +pub union __anon_Foo_1 { pub name: *mut i8, pub tp: *mut std::ffi::c_void, } -impl std::fmt::Debug for __anon_1 { +impl std::fmt::Debug for __anon_Foo_1 { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "(???)") } } -impl Default for __anon_1 { +impl Default for __anon_Foo_1 { fn default() -> Self { Self { name: std::ptr::null_mut(), @@ -2811,17 +2811,17 @@ struct Foo foo; #[derive(Debug, Default, Copy, Clone)] #[repr(C)] pub struct Foo { - pub test: __anon_1, + pub test: __anon_Foo_1, } #[derive(Debug, Copy, Clone, Eq, PartialEq)] #[repr(transparent)] -pub struct __anon_1(pub u32); +pub struct __anon_Foo_1(pub u32); #[allow(non_upper_case_globals)] -impl __anon_1 { - pub const FOO: __anon_1 = __anon_1(1); +impl __anon_Foo_1 { + pub const FOO: __anon_Foo_1 = __anon_Foo_1(1); } -impl Default for __anon_1 { - fn default() -> Self { __anon_1::FOO } +impl Default for __anon_Foo_1 { + fn default() -> Self { __anon_Foo_1::FOO } } "#; @@ -2918,40 +2918,40 @@ struct bpf_sock_tuple_5_15 tup; #[derive(Debug, Default, Copy, Clone)] #[repr(C)] pub struct bpf_sock_tuple_5_15 { - pub __anon_1: __anon_1, + pub __anon_bpf_sock_tuple_5_15_1: __anon_bpf_sock_tuple_5_15_1, pub __pad_36: [u8; 4], - pub __anon_2: __anon_2, + pub __anon_bpf_sock_tuple_5_15_2: __anon_bpf_sock_tuple_5_15_2, } #[derive(Copy, Clone)] #[repr(C)] -pub union __anon_1 { - pub ipv4: __anon_3, - pub ipv6: __anon_4, +pub union __anon_bpf_sock_tuple_5_15_1 { + pub ipv4: __anon_bpf_sock_tuple_5_15_3, + pub ipv6: __anon_bpf_sock_tuple_5_15_4, } -impl std::fmt::Debug for __anon_1 { +impl std::fmt::Debug for __anon_bpf_sock_tuple_5_15_1 { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "(???)") } } -impl Default for __anon_1 { +impl Default for __anon_bpf_sock_tuple_5_15_1 { fn default() -> Self { Self { - ipv4: __anon_3::default(), + ipv4: __anon_bpf_sock_tuple_5_15_3::default(), } } } #[derive(Copy, Clone)] #[repr(C)] -pub union __anon_2 { +pub union __anon_bpf_sock_tuple_5_15_2 { pub a: i32, pub b: *mut i8, } -impl std::fmt::Debug for __anon_2 { +impl std::fmt::Debug for __anon_bpf_sock_tuple_5_15_2 { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "(???)") } } -impl Default for __anon_2 { +impl Default for __anon_bpf_sock_tuple_5_15_2 { fn default() -> Self { Self { a: i32::default(), @@ -2960,7 +2960,7 @@ impl Default for __anon_2 { } #[derive(Debug, Default, Copy, Clone)] #[repr(C)] -pub struct __anon_3 { +pub struct __anon_bpf_sock_tuple_5_15_3 { pub saddr: u32, pub daddr: u32, pub sport: u16, @@ -2968,7 +2968,7 @@ pub struct __anon_3 { } #[derive(Debug, Default, Copy, Clone)] #[repr(C)] -pub struct __anon_4 { +pub struct __anon_bpf_sock_tuple_5_15_4 { pub saddr: [u32; 4], pub daddr: [u32; 4], pub sport: u16, From 19d5ff492d7e77d25e6504b060cbcec1f30ad161 Mon Sep 17 00:00:00 2001 From: Vadim Vetrov Date: Fri, 23 May 2025 02:37:12 +0300 Subject: [PATCH 3/3] libbpf-cargo: Relative anonymous type naming --- libbpf-cargo/src/gen/btf.rs | 102 +++++++++++++++------------- libbpf-cargo/src/test.rs | 128 ++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 45 deletions(-) diff --git a/libbpf-cargo/src/gen/btf.rs b/libbpf-cargo/src/gen/btf.rs index 85c5f0f4..f5eea6c9 100644 --- a/libbpf-cargo/src/gen/btf.rs +++ b/libbpf-cargo/src/gen/btf.rs @@ -369,10 +369,17 @@ fn escape_reserved_keyword(identifier: Cow<'_, str>) -> Cow<'_, str> { } #[derive(Debug, Clone)] -pub struct BtfDependency { - pub name: Option, - pub dep_id: i32, - pub child_counter: Rc>, +struct BtfDependency { + /// Name of the dependency parent + parent_name: Option, + + /// Dependency id relative to the parent's `child_counter` + dep_id: i32, + + /// The `child_counter` for the dependency if it is intended to be + /// a parent itself. + /// For an anonymous unit this should be a pointer to the parent's `child_counter` + child_counter: Rc>, } #[derive(Debug, Default)] @@ -386,53 +393,47 @@ pub(crate) struct TypeMap { /// name already. names_count: RefCell>, - dependencies: RefCell>, + /// Mapping from type to it's parent. Used in anonymous members naming + dependency_tree: RefCell>, } impl TypeMap { - pub fn derive_parent<'s>(&self, ty: &BtfType<'s>, parent: &BtfType<'s>) { - let mut deps = self.dependencies.borrow_mut(); + fn register_parent<'s>(&self, ty: &BtfType<'s>, parent: &BtfType<'s>) { + let mut deps = self.dependency_tree.borrow_mut(); if deps.get(&ty.type_id()).is_some() { return; } - let parent_dep = deps.get(&parent.type_id()); - if let Some(pdep) = parent_dep { - let mut dep = pdep.clone(); - - if let Some(n) = parent.name() { - dep.name = Some(n.to_string_lossy().to_string()); - } - if ty.name().is_some() { - dep.child_counter = Rc::new(RefCell::new(0)); - } + let pdep = deps.entry(parent.type_id()).or_insert(BtfDependency { + parent_name: None, + dep_id: 0, + child_counter: Rc::new(RefCell::new(0)), + }); - let parent_counter = Rc::>::clone(&pdep.child_counter); - *parent_counter.borrow_mut() += 1; - dep.dep_id = *parent_counter.borrow(); + let mut dep = pdep.clone(); - deps.insert(ty.type_id(), dep); - } else { - let mut dep = BtfDependency { - name: None, - dep_id: 0, - child_counter: Rc::new(RefCell::new(1)), - }; - deps.insert(parent.type_id(), dep.clone()); + // If parent is named, derive it. + // Otherwise derive parent's parent + if let Some(n) = parent.name() { + dep.parent_name = Some(n.to_string_lossy().to_string()); + } - if let Some(n) = parent.name() { - dep.name = Some(n.to_string_lossy().to_string()); - } - if ty.name().is_some() { - dep.child_counter = Rc::new(RefCell::new(0)); - } - dep.dep_id = 1; - deps.insert(ty.type_id(), dep); + // If the current unit is named, self-assign the child_counter. + // Otherwise derive a parent's one + if ty.name().is_some() { + dep.child_counter = Rc::new(RefCell::new(0)); } + + // Increment parent's `child_counter` and assign the new value to dep_id + let parent_counter = Rc::clone(&pdep.child_counter); + *parent_counter.borrow_mut() += 1; + dep.dep_id = *parent_counter.borrow(); + + deps.insert(ty.type_id(), dep); } - pub fn lookup_parent<'s>(&self, ty: &BtfType<'s>) -> Option { - self.dependencies.borrow().get(&ty.type_id()).cloned() + fn lookup_parent<'s>(&self, ty: &BtfType<'s>) -> Option { + self.dependency_tree.borrow().get(&ty.type_id()).cloned() } pub fn type_name_or_anon<'s>(&self, ty: &BtfType<'s>) -> Cow<'s, str> { @@ -440,15 +441,26 @@ impl TypeMap { None => { let mut anon_table = self.types.borrow_mut(); let len = anon_table.len() + 1; // use 1 index anon ids for backwards compat - let anon_id = anon_table.entry(ty.type_id()).or_insert(len); - if let Some(parent) = self.lookup_parent(ty) { - if let Some(name) = parent.name { - if !name.is_empty() { - return format!("{ANON_PREFIX}{}_{}", name, parent.dep_id).into(); + let anon_id = match anon_table.entry(ty.type_id()) { + Entry::Occupied(anon_id) => { + let anon_id = anon_id.get(); + *anon_id + } + Entry::Vacant(anon_id) => { + if let Some(dep) = self.lookup_parent(ty) { + if let Some(name) = dep.parent_name { + if !name.is_empty() { + return format!("{ANON_PREFIX}{}_{}", name, dep.dep_id).into(); + } + } } + + let anon_id = anon_id.insert(len); + *anon_id } - } + }; + format!("{ANON_PREFIX}{anon_id}").into() } Some(n) => match self.names.borrow_mut().entry(ty.type_id()) { @@ -789,7 +801,7 @@ impl<'s> GenBtf<'s> { } if let Some(next_ty_id) = next_type(field_ty)? { - self.type_map.derive_parent(&next_ty_id, &t); + self.type_map.register_parent(&next_ty_id, &t); dependent_types.push(next_ty_id); } let field_name = if let Some(name) = member.name { diff --git a/libbpf-cargo/src/test.rs b/libbpf-cargo/src/test.rs index a0253407..f678d4c2 100644 --- a/libbpf-cargo/src/test.rs +++ b/libbpf-cargo/src/test.rs @@ -2792,6 +2792,134 @@ impl Default for __anon_Foo_1 { assert_definition(&btf, &struct_foo, expected_output); } +#[test] +fn test_btf_dump_anon_member_tree() { + let prog_text = r#" +#include "vmlinux.h" +#include + +struct Foo { + union { + struct { + char *name; + void *tp; + }; + struct Bar { + union { + struct { + char *name; + void *trp; + }; + struct Baz { + char *name; + void *trp; + } baz; + }; + } bar; + }; +}; + +struct Foo foo = {0}; +"#; + + let expected_output = r#" +#[derive(Debug, Default, Copy, Clone)] +#[repr(C)] +pub struct Foo { + pub __anon_Foo_1: __anon_Foo_1, +} +#[derive(Copy, Clone)] +#[repr(C)] +pub union __anon_Foo_1 { + pub __anon_Foo_2: __anon_Foo_2, + pub bar: Bar, +} +impl std::fmt::Debug for __anon_Foo_1 { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "(???)") + } +} +impl Default for __anon_Foo_1 { + fn default() -> Self { + Self { + __anon_Foo_2: __anon_Foo_2::default(), + } + } +} +#[derive(Debug, Copy, Clone)] +#[repr(C)] +pub struct __anon_Foo_2 { + pub name: *mut i8, + pub tp: *mut std::ffi::c_void, +} +impl Default for __anon_Foo_2 { + fn default() -> Self { + Self { + name: std::ptr::null_mut(), + tp: std::ptr::null_mut(), + } + } +} +#[derive(Debug, Default, Copy, Clone)] +#[repr(C)] +pub struct Bar { + pub __anon_Bar_1: __anon_Bar_1, +} +#[derive(Copy, Clone)] +#[repr(C)] +pub union __anon_Bar_1 { + pub __anon_Bar_2: __anon_Bar_2, + pub baz: Baz, +} +impl std::fmt::Debug for __anon_Bar_1 { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "(???)") + } +} +impl Default for __anon_Bar_1 { + fn default() -> Self { + Self { + __anon_Bar_2: __anon_Bar_2::default(), + } + } +} +#[derive(Debug, Copy, Clone)] +#[repr(C)] +pub struct __anon_Bar_2 { + pub name: *mut i8, + pub trp: *mut std::ffi::c_void, +} +impl Default for __anon_Bar_2 { + fn default() -> Self { + Self { + name: std::ptr::null_mut(), + trp: std::ptr::null_mut(), + } + } +} +#[derive(Debug, Copy, Clone)] +#[repr(C)] +pub struct Baz { + pub name: *mut i8, + pub trp: *mut std::ffi::c_void, +} +impl Default for Baz { + fn default() -> Self { + Self { + name: std::ptr::null_mut(), + trp: std::ptr::null_mut(), + } + } +} +"#; + + let mmap = build_btf_mmap(prog_text); + let btf = btf_from_mmap(&mmap); + let struct_foo = find_type_in_btf!(btf, types::Struct<'_>, "Foo"); + + assert_definition(&btf, &struct_foo, expected_output); +} + #[test] fn test_btf_dump_definition_anon_enum() { let prog_text = r#"