From f6bf45bb9351d4c4ad47530323f90f8500164439 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20M=C3=BCller?= <deso@posteo.net>
Date: Tue, 19 Mar 2024 15:42:24 -0700
Subject: [PATCH] libbpf-cargo: Make sure to escape reserve keywords used for
 identifiers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <deso@posteo.net>
---
 libbpf-cargo/CHANGELOG.md   |  2 ++
 libbpf-cargo/src/gen/btf.rs | 29 +++++++++++++++++++++++++++-
 libbpf-cargo/src/test.rs    | 38 +++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/libbpf-cargo/CHANGELOG.md b/libbpf-cargo/CHANGELOG.md
index 68fabe414..e97a3e15f 100644
--- a/libbpf-cargo/CHANGELOG.md
+++ b/libbpf-cargo/CHANGELOG.md
@@ -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
diff --git a/libbpf-cargo/src/gen/btf.rs b/libbpf-cargo/src/gen/btf.rs
index 4bdefedbb..e7eb5086d 100644
--- a/libbpf-cargo/src/gen/btf.rs
+++ b/libbpf-cargo/src/gen/btf.rs
@@ -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
@@ -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
diff --git a/libbpf-cargo/src/test.rs b/libbpf-cargo/src/test.rs
index f51dfaff4..4279bce2b 100644
--- a/libbpf-cargo/src/test.rs
+++ b/libbpf-cargo/src/test.rs
@@ -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#"