Skip to content

Commit

Permalink
libbpf-cargo: Make sure to escape reserve keywords used for identifiers
Browse files Browse the repository at this point in the history
C generally has fewer reserved keywords than Rust. That means that more
liberal naming can be applied to variables and types. However, when we
generate Rust code we may end up accidentally colliding with a Rust
keyword as a consequence.
With this change we make sure to escape such identifiers accordingly.

Signed-off-by: Daniel Müller <[email protected]>
  • Loading branch information
d-e-s-o committed Mar 19, 2024
1 parent 44d0750 commit f6bf45b
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 1 deletion.
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
- Added `--clang-args` argument to `make` and `build` sub-commands
- Put all generated types into single `<project>_types` module as opposed to
having multiple modules for various sections (`.bss`, `.rodata`, etc.)
- Fixed potential naming issues by escaping reserved keywords used in
identifiers
- Fixed potential unsoundness issues in generated skeletons by wrapping "unsafe"
type in `MaybeUninit`
- Added pointer based ("raw") access to datasec type to generated skeletons
Expand Down
29 changes: 28 additions & 1 deletion libbpf-cargo/src/gen/btf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,33 @@ fn is_unsafe(ty: BtfType<'_>) -> bool {
})
}

fn escape_reserved_keyword(identifier: Cow<'_, str>) -> Cow<'_, str> {
// A list of keywords that need to be escaped in Rust when used for variable
// names or similar (from https://doc.rust-lang.org/reference/keywords.html#keywords,
// minus keywords that are already reserved in C).
let reserved = [
"Self", "abstract", "as", "async", "await", "become", "box", "const", "crate", "dyn",
"enum", "final", "fn", "impl", "in", "let", "loop", "macro", "match", "mod", "move", "mut",
"override", "priv", "pub", "ref", "self", "super", "trait", "try", "type", "typeof",
"unsafe", "unsized", "use", "virtual", "where", "yield",
];
debug_assert_eq!(
reserved.as_slice(),
{
let mut vec = reserved.to_vec();
vec.sort();
vec
},
"please keep reserved keywoards sorted",
);

if reserved.binary_search(&identifier.as_ref()).is_ok() {
Cow::Owned(format!("r#{identifier}"))
} else {
identifier
}
}

pub struct GenBtf<'s> {
btf: Btf<'s>,
// We use refcell here because the design of this type unfortunately causes a lot of borrowing
Expand Down Expand Up @@ -349,7 +376,7 @@ impl<'s> GenBtf<'s> {
dependent_types.push(next_ty_id);
}
let field_name = if let Some(name) = member.name {
name.to_string_lossy()
escape_reserved_keyword(name.to_string_lossy())
} else {
// Only anonymous unnamed unions should ever have no name set.
// We just name them the same as their anonymous type. As there
Expand Down
38 changes: 38 additions & 0 deletions libbpf-cargo/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,44 @@ fn assert_definition(btf: &GenBtf<'_>, btf_item: &BtfType<'_>, expected_output:
assert!(eo == ao);
}

#[test]
fn test_btf_dump_reserved_keyword_escaping() {
let prog_text = r#"
#include "vmlinux.h"
#include <bpf/bpf_helpers.h>
struct Foo {
u64 type;
void* mod;
};
struct Foo foo = {{0}};
"#;

let expected_output = r#"
#[derive(Debug, Copy, Clone)]
#[repr(C)]
pub struct Foo {
pub r#type: u64,
pub r#mod: *mut std::ffi::c_void,
}
impl Default for Foo {
fn default() -> Self {
Foo {
r#type: u64::default(),
r#mod: 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_basic() {
let prog_text = r#"
Expand Down

0 comments on commit f6bf45b

Please sign in to comment.