Skip to content

Commit

Permalink
libbpf-cargo: Fix generation of duplicate anon types
Browse files Browse the repository at this point in the history
Our skeleton generation code instantiates a new GenBtf object in many of
the individual sub-steps of the generation procedure. However, that is a
broken pattern: each GenBtf instance starts the counter that determines
numbering of Rust types generated for anonymous C types anew and, hence,
we can run into duplicate names.
This change fixes the issue by using a "global" GenBtf instance instead.

Signed-off-by: Daniel Müller <[email protected]>
  • Loading branch information
d-e-s-o authored and danielocfb committed Jul 18, 2024
1 parent 802e864 commit 31f8583
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 22 deletions.
41 changes: 19 additions & 22 deletions libbpf-cargo/src/gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,16 +731,14 @@ fn gen_skel_prog_defs(skel: &mut String, progs: &ProgsData, raw_obj_name: &str)

fn gen_skel_datasec_types(
skel: &mut String,
object: &Object,
btf: Option<&GenBtf<'_>>,
processed: &mut HashSet<TypeId>,
) -> Result<()> {
let btf =
if let Some(btf) = Btf::from_bpf_object(unsafe { object.as_libbpf_object().as_ref() })? {
btf
} else {
return Ok(());
};
let btf = GenBtf::from(btf);
let btf = if let Some(btf) = btf {
btf
} else {
return Ok(());
};

for ty in btf.type_by_kind::<types::DataSec<'_>>() {
let name = match ty.name() {
Expand All @@ -763,12 +761,10 @@ fn gen_skel_datasec_types(

fn gen_skel_struct_ops_types(
skel: &mut String,
object: &Object,
btf: Option<&GenBtf<'_>>,
processed: &mut HashSet<TypeId>,
) -> Result<()> {
if let Some(btf) = Btf::from_bpf_object(unsafe { object.as_libbpf_object().as_ref() })? {
let btf = GenBtf::from(btf);

if let Some(btf) = btf {
let def = btf.struct_ops_type_definition(processed)?;
write!(skel, "{def}")?;
} else {
Expand All @@ -787,15 +783,14 @@ pub struct struct_ops {{}}
fn gen_skel_map_types(
skel: &mut String,
object: &Object,
btf: Option<&GenBtf<'_>>,
processed: &mut HashSet<TypeId>,
) -> Result<()> {
let btf =
if let Some(btf) = Btf::from_bpf_object(unsafe { object.as_libbpf_object().as_ref() })? {
btf
} else {
return Ok(());
};
let btf = GenBtf::from(btf);
let btf = if let Some(btf) = btf {
btf
} else {
return Ok(());
};

for map in maps(object) {
let map_ptr = map.as_libbpf_object().as_ptr();
Expand Down Expand Up @@ -1002,6 +997,8 @@ fn gen_skel_contents(_debug: bool, raw_obj_name: &str, obj_file_path: &Path) ->
.with_context(|| format!("failed to open BPF object `{}`", obj_file_path.display()))?;
let mmap = unsafe { Mmap::map(&file)? };
let object = open_bpf_object(&libbpf_obj_name, &mmap)?;
let btf =
Btf::from_bpf_object(unsafe { object.as_libbpf_object().as_ref() })?.map(GenBtf::from);
let maps = MapsData::new(&object)?;
let progs = ProgsData::new(&object)?;

Expand Down Expand Up @@ -1089,9 +1086,9 @@ fn gen_skel_contents(_debug: bool, raw_obj_name: &str, obj_file_path: &Path) ->
)?;

let mut processed = HashSet::new();
gen_skel_datasec_types(&mut skel, &object, &mut processed)?;
gen_skel_struct_ops_types(&mut skel, &object, &mut processed)?;
gen_skel_map_types(&mut skel, &object, &mut processed)?;
gen_skel_datasec_types(&mut skel, btf.as_ref(), &mut processed)?;
gen_skel_struct_ops_types(&mut skel, btf.as_ref(), &mut processed)?;
gen_skel_map_types(&mut skel, &object, btf.as_ref(), &mut processed)?;
writeln!(skel, "}}")?;

write!(
Expand Down
128 changes: 128 additions & 0 deletions libbpf-cargo/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,134 @@ fn test_skeleton_generate_struct_with_pointer() {
assert!(status.success());
}

/// Generate a skeleton that includes multiple "anon" type definitions.
#[test]
fn test_skeleton_builder_multiple_anon() {
let (_dir, proj_dir, cargo_toml) = setup_temp_project();

// Add prog dir
create_dir(proj_dir.join("src/bpf")).expect("failed to create prog dir");

// Add a prog
let mut prog = OpenOptions::new()
.write(true)
.create(true)
.truncate(true)
.open(proj_dir.join("src/bpf/prog.bpf.c"))
.expect("failed to open prog.bpf.c");

write!(
prog,
r#"
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
struct unique_key {{
struct {{
u32 foo;
u64 bar;
}} foobar;
}};
struct {{
__uint(type, BPF_MAP_TYPE_HASH);
__uint(max_entries, 1024);
__type(key, struct unique_key);
__type(value, u64);
}} mymap SEC(".maps");
struct Foo {{
int x;
struct {{
u8 y[10];
u16 z[16];
}} bar;
struct {{
u32 w;
u64 *u;
}} baz;
int w;
}};
struct Foo foo;
SEC("kprobe/foo")
int this_is_my_prog(u64 *ctx)
{{
return 0;
}}
"#,
)
.expect("failed to write prog.bpf.c");

// Lay down the necessary header files
add_vmlinux_header(&proj_dir);

// Generate skeleton file
let skel = NamedTempFile::new().unwrap();
SkeletonBuilder::new()
.source(proj_dir.join("src/bpf/prog.bpf.c"))
.debug(true)
.build_and_generate(skel.path())
.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 std::mem::MaybeUninit;
use skel::*;
use libbpf_rs::skel::SkelBuilder;
use libbpf_rs::skel::OpenSkel;
fn main() {{
let builder = ProgSkelBuilder::default();
let mut open_object = MaybeUninit::uninit();
let open_skel = builder
.open(&mut open_object)
.expect("failed to open skel");
let _skel = open_skel.load().expect("failed to load skel");
let _key = prog_types::unique_key::default();
}}
"#,
skel_path = skel.path().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());
}

/// Check that skeleton creation is deterministic, i.e., that no temporary paths
/// change the output between invocations.
#[test]
Expand Down

0 comments on commit 31f8583

Please sign in to comment.