Skip to content

Commit

Permalink
libbpf-cargo: Handle duplicated types in BTF more gracefully
Browse files Browse the repository at this point in the history
Our SkeletonBuilder currently only support building a skeleton from a
single .bpf.c file. However, users may very well compile multiple .bpf.c
files into .bpf.o and link them into a single object. In such a
scenario, it is easily possible that we end up with multiple types of
the same name in the corresponding BTF. Currently, this results in
compilation errors.
With this change we handle this case more gracefully, by enumerating
these types, similar to what libbpf does in its dumping logic.

Signed-off-by: Daniel Müller <deso@posteo.net>
  • Loading branch information
d-e-s-o committed Jan 21, 2025
1 parent 7c22ef4 commit dbc1f06
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 5 deletions.
2 changes: 2 additions & 0 deletions libbpf-cargo/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Unreleased
- Replaced `--debug` option of `libbpf` sub-command with `-v` /
`--verbose`
- Removed `--quiet` option of `libbpf make` sub-command
- Fixed handling of multiple types of same name in BTF by enumerating
them in the generated skeleton


0.25.0-beta.1
Expand Down
31 changes: 26 additions & 5 deletions libbpf-cargo/src/gen/btf.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::borrow::Cow;
use std::cell::RefCell;
use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt::Debug;
Expand Down Expand Up @@ -368,9 +369,14 @@ fn escape_reserved_keyword(identifier: Cow<'_, str>) -> Cow<'_, str> {

#[derive(Debug, Default)]
pub(crate) struct AnonTypes {
/// A mapping from type to number, allowing us to assign numbers to types
/// consistently.
/// A mapping from type to number, allowing us to assign numbers to
/// anonymous types consistently.
types: RefCell<HashMap<TypeId, usize>>,
/// Mapping from type to name.
names: RefCell<HashMap<TypeId, String>>,
/// Mapping from type name to the number of times we have seen this
/// name already.
names_count: RefCell<HashMap<String, u8>>,
}

impl AnonTypes {
Expand All @@ -382,7 +388,23 @@ impl AnonTypes {
let anon_id = anon_table.entry(ty.type_id()).or_insert(len);
format!("{ANON_PREFIX}{anon_id}").into()
}
Some(n) => n.to_string_lossy(),
Some(n) => match self.names.borrow_mut().entry(ty.type_id()) {
Entry::Occupied(entry) => Cow::Owned(entry.get().clone()),
Entry::Vacant(vacancy) => {
let name = n.to_string_lossy();
let mut names_count = self.names_count.borrow_mut();
let cnt = names_count
.entry(name.to_string())
.and_modify(|cnt| *cnt += 1)
.or_default();
if *cnt == 0 {
vacancy.insert(name.to_string());
name
} else {
Cow::Owned(vacancy.insert(format!("{name}_{cnt}")).clone())
}
}
},
}
}
}
Expand Down Expand Up @@ -819,8 +841,7 @@ impl<'s> GenBtf<'s> {
writeln!(def, r#"#[repr(C{packed_repr})]"#)?;
writeln!(
def,
r#"pub {agg_type} {name} {{"#,
agg_type = aggregate_type,
r#"pub {aggregate_type} {name} {{"#,
name = self.anon_types.type_name_or_anon(&t),
)?;

Expand Down
93 changes: 93 additions & 0 deletions libbpf-cargo/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,99 @@ fn test_skeleton_builder_deterministic() {
assert_eq!(skel1, skel2);
}

/// Check that we generate a valid skeleton in the presence of duplicate
/// structs in the BTF.
#[test]
fn test_skeleton_duplicate_struct() {
let (_dir, proj_dir, cargo_toml) = setup_temp_project();

create_dir(proj_dir.join("src/bpf")).expect("failed to create prog dir");
let prog1 = proj_dir.join("src/bpf/prog1.bpf.c");
write(
&prog1,
r#"
#include "vmlinux.h"
struct foobar {
int bar;
} foo;
"#,
)
.expect("failed to write prog.bpf.c");
let prog2 = proj_dir.join("src/bpf/prog2.bpf.c");
write(
&prog2,
r#"
#include "vmlinux.h"
typedef int nonsensicaltypedeftoint;
struct foobar {
nonsensicaltypedeftoint bar;
} baz;
"#,
)
.expect("failed to write prog.bpf.c");

add_vmlinux_header(&proj_dir);

let bpf_o = proj_dir.join("prog.bpf.o");
let _output = BpfObjBuilder::default().build_many([&prog1, &prog2], &bpf_o).unwrap();

let skel = proj_dir.join("src/bpf/skel.rs");
let () = SkeletonBuilder::new()
.obj(&bpf_o)
.generate(&skel)
.unwrap();

let mut cargo = OpenOptions::new()
.append(true)
.open(&cargo_toml)
.expect("failed to open Cargo.toml");

// Make test project use our development libbpf-rs version
writeln!(
cargo,
r#"
libbpf-rs = {{ path = "{}" }}
"#,
get_libbpf_rs_path().as_path().display()
)
.expect("failed to write to Cargo.toml");

let mut source = OpenOptions::new()
.write(true)
.truncate(true)
.open(proj_dir.join("src/main.rs"))
.expect("failed to open main.rs");

write!(
source,
r#"
#[path = "{skel_path}"]
mod skel;
use skel::*;
fn main() {{
let _foobar0 = types::foobar::default();
let _foobar1 = types::foobar_1::default();
}}
"#,
skel_path = skel.display(),
)
.expect("failed to write to main.rs");

let status = Command::new("cargo")
.arg("build")
.arg("--quiet")
.arg("--manifest-path")
.arg(cargo_toml.into_os_string())
.env("RUSTFLAGS", "-Dwarnings")
.status()
.expect("failed to spawn cargo-build");
assert!(status.success());
}

// -- TEST RUST GENERATION OF BTF PROGRAMS --

/// Searches the Btf struct for a BtfType
Expand Down

0 comments on commit dbc1f06

Please sign in to comment.