Merge pull request #293 from dave-tucker/getters-be-gone

Getters be gone

aya-gen/src/bin/aya-gen.rs

@@ -15,8 +15,6 @@ enum Command {
     BtfTypes {
         #[structopt(long, default_value = "/sys/kernel/btf/vmlinux")]
         btf: PathBuf,
-        #[structopt(long)]
-        probe_read_getters: bool,
         names: Vec<String>,
     },
 }
@@ -31,12 +29,8 @@ fn main() {
 fn try_main() -> Result<(), anyhow::Error> {
     let opts = Options::from_args();
     match opts.command {
-        Command::BtfTypes {
+        Command::BtfTypes { btf, names } => {
-            btf,
+            let bindings = btf_types::generate(&btf, &names)?;
-            probe_read_getters,
-            names,
-        } => {
-            let bindings = btf_types::generate(&btf, &names, probe_read_getters)?;
             println!("{}", bindings);
         }
     };

aya-gen/src/btf_types.rs

@@ -2,11 +2,7 @@ use std::{io, path::Path, process::Command, str::from_utf8};
 
 use thiserror::Error;
 
-use crate::{
+use crate::bindgen;
-    bindgen,
-    getters::{generate_getters_for_items, read_getter},
-    rustfmt,
-};
 
 #[derive(Error, Debug)]
 pub enum Error {
@@ -23,11 +19,7 @@ pub enum Error {
     Rustfmt(#[source] io::Error),
 }
 
-pub fn generate<T: AsRef<str>>(
+pub fn generate<T: AsRef<str>>(btf_file: &Path, types: &[T]) -> Result<String, Error> {
-    btf_file: &Path,
-    types: &[T],
-    probe_read_getters: bool,
-) -> Result<String, Error> {
     let mut bindgen = bindgen::bpf_builder();
 
     let c_header = c_header_from_btf(btf_file)?;
@@ -38,17 +30,6 @@ pub fn generate<T: AsRef<str>>(
     }
 
     let bindings = bindgen.generate().or(Err(Error::Bindgen))?.to_string();
-    if !probe_read_getters {
-        return Ok(bindings);
-    }
-
-    let tree = syn::parse_str::<syn::File>(&bindings).unwrap();
-    let bpf_probe_read = syn::parse_str::<syn::Path>("::aya_bpf::helpers::bpf_probe_read").unwrap();
-    let getters =
-        generate_getters_for_items(&tree.items, |getter| read_getter(getter, &bpf_probe_read));
-    let getters = rustfmt::format(&getters.to_string()).map_err(Error::Rustfmt)?;
-
-    let bindings = format!("{}\n{}", bindings, getters);
 
     Ok(bindings)
 }

aya-gen/src/getters.rs

@@ -1,151 +0,0 @@
-use indexmap::IndexMap;
-use proc_macro2::{Span, TokenStream};
-use quote::{quote, TokenStreamExt};
-use syn::{
-    self, Fields, FieldsNamed, Generics, Ident, Item, ItemStruct, ItemUnion, Path, Type, TypePath,
-    Visibility,
-};
-
-pub struct GetterList<'a> {
-    slf: Ident,
-    item_fields: IndexMap<Ident, (&'a Item, &'a FieldsNamed)>,
-}
-
-impl<'a> GetterList<'a> {
-    pub fn new(items: &'a [Item]) -> GetterList<'a> {
-        let item_fields = items
-            .iter()
-            .filter_map(|item| {
-                unpack_item(item).map(|(ident, _generics, fields)| (ident.clone(), (item, fields)))
-            })
-            .collect();
-        GetterList {
-            slf: Ident::new("self", Span::call_site()),
-            item_fields,
-        }
-    }
-
-    pub fn iter(&self) -> impl Iterator<Item = (&'a Item, Vec<Getter<'_>>)> {
-        self.item_fields
-            .values()
-            .map(move |(item, fields)| (*item, self.getters(&self.slf, fields)))
-    }
-
-    fn getters(&self, ident: &'a Ident, fields: &'a FieldsNamed) -> Vec<Getter<'a>> {
-        let mut getters = Vec::new();
-        for field in &fields.named {
-            if let Visibility::Inherited = field.vis {
-                continue;
-            }
-
-            let field_ident = field.ident.as_ref().unwrap();
-            let field_s = field_ident.to_string();
-
-            // FIXME: bindgen generates fields named `_bitfield_N` for bitfields. If a type T has
-            // two or more unions with bitfields, the getters for the bitfields - generated in impl
-            // T - will clash. To avoid that we skip getters for bitfields altogether for now.
-            // See sk_reuseport_md for an example where the clash happens.
-            if field_s.starts_with("_bitfield") {
-                continue;
-            }
-
-            if field_s.starts_with("__bindgen_anon") {
-                let field_ty_ident = match &field.ty {
-                    Type::Path(TypePath {
-                        path: Path { segments, .. },
-                        ..
-                    }) => &segments.first().unwrap().ident,
-                    _ => panic!(),
-                };
-                let sub_fields = self
-                    .item_fields
-                    .get(field_ty_ident)
-                    .unwrap_or_else(|| panic!("{}", field_ty_ident.to_string()))
-                    .1;
-                getters.extend(self.getters(field_ident, sub_fields).into_iter().map(
-                    |mut getter| {
-                        getter.prefix.insert(0, ident);
-                        getter
-                    },
-                ));
-            } else {
-                getters.push(Getter {
-                    ident: field_ident,
-                    prefix: vec![ident],
-                    ty: &field.ty,
-                });
-            }
-        }
-
-        getters
-    }
-}
-
-pub fn generate_getters_for_items(
-    items: &[Item],
-    gen_getter: impl Fn(&Getter<'_>) -> TokenStream,
-) -> TokenStream {
-    let mut tokens = TokenStream::new();
-    tokens.append_all(GetterList::new(items).iter().map(|(item, getters)| {
-        let getters = getters.iter().map(&gen_getter);
-        let (ident, generics, _) = unpack_item(item).unwrap();
-        quote! {
-            impl#generics #ident#generics {
-                #(#getters)*
-            }
-        }
-    }));
-
-    tokens
-}
-
-pub fn read_getter(getter: &Getter<'_>, read_fn: &Path) -> TokenStream {
-    let ident = getter.ident;
-    let ty = getter.ty;
-    let prefix = &getter.prefix;
-    match ty {
-        Type::Ptr(_) => {
-            quote! {
-                pub fn #ident(&self) -> Option<#ty> {
-                    let v = unsafe { #read_fn(&#(#prefix).*.#ident) }.ok()?;
-                    if v.is_null() {
-                        None
-                    } else {
-                        Some(v)
-                    }
-                }
-            }
-        }
-        _ => {
-            quote! {
-                pub fn #ident(&self) -> Option<#ty> {
-                    unsafe { #read_fn(&#(#prefix).*.#ident) }.ok()
-                }
-            }
-        }
-    }
-}
-
-pub struct Getter<'a> {
-    pub ident: &'a Ident,
-    pub prefix: Vec<&'a Ident>,
-    pub ty: &'a Type,
-}
-
-fn unpack_item(item: &Item) -> Option<(&Ident, &Generics, &FieldsNamed)> {
-    match item {
-        Item::Struct(ItemStruct {
-            ident,
-            generics,
-            fields: Fields::Named(fields),
-            ..
-        })
-        | Item::Union(ItemUnion {
-            ident,
-            generics,
-            fields,
-            ..
-        }) => Some((ident, generics, fields)),
-        _ => None,
-    }
-}

aya-gen/src/lib.rs

@@ -6,7 +6,6 @@ use std::{
 
 pub mod bindgen;
 pub mod btf_types;
-pub mod getters;
 pub mod rustfmt;
 
 pub fn write_to_file<T: AsRef<Path>>(path: T, code: &str) -> Result<(), io::Error> {

bpf/aya-bpf-bindings/src/aarch64/mod.rs

@@ -1,4 +1,3 @@
 #![allow(clippy::all, dead_code)]
 pub mod bindings;
-pub mod getters;
 pub mod helpers;

bpf/aya-bpf-bindings/src/armv7/mod.rs

@@ -1,4 +1,3 @@
 #![allow(clippy::all, dead_code)]
 pub mod bindings;
-pub mod getters;
 pub mod helpers;

bpf/aya-bpf-bindings/src/lib.rs

@@ -26,7 +26,7 @@ mod gen {
     #[cfg(bpf_target_arch = "riscv64")]
     pub use super::riscv64::*;
 }
-pub use gen::{getters, helpers};
+pub use gen::helpers;
 
 pub mod bindings {
     pub use crate::gen::bindings::*;
@@ -55,21 +55,3 @@ pub mod bindings {
         pub pinning: ::aya_bpf_cty::c_uint,
     }
 }
-
-use aya_bpf_cty::{c_long, c_void};
-use core::mem::{self, MaybeUninit};
-
-#[inline]
-unsafe fn bpf_probe_read<T>(src: *const T) -> Result<T, c_long> {
-    let mut v: MaybeUninit<T> = MaybeUninit::uninit();
-    let ret = helpers::bpf_probe_read(
-        v.as_mut_ptr() as *mut c_void,
-        mem::size_of::<T>() as u32,
-        src as *const c_void,
-    );
-    if ret < 0 {
-        return Err(ret);
-    }
-
-    Ok(v.assume_init())
-}

bpf/aya-bpf-bindings/src/riscv64/mod.rs

@@ -1,4 +1,3 @@
 #![allow(clippy::all, dead_code)]
 pub mod bindings;
-pub mod getters;
 pub mod helpers;

bpf/aya-bpf-bindings/src/x86_64/mod.rs

@@ -1,4 +1,3 @@
 #![allow(clippy::all, dead_code)]
 pub mod bindings;
-pub mod getters;
 pub mod helpers;

bpf/aya-bpf/src/args.rs

@@ -101,18 +101,18 @@ pub trait FromPtRegs: Sized {
 impl<T> FromPtRegs for *const T {
     fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
         match n {
-            0 => ctx.rdi().map(|v| v as _),
+            0 => Some(ctx.rdi as *const _),
-            1 => ctx.rsi().map(|v| v as _),
+            1 => Some(ctx.rsi as *const _),
-            2 => ctx.rdx().map(|v| v as _),
+            2 => Some(ctx.rdx as *const _),
-            3 => ctx.rcx().map(|v| v as _),
+            3 => Some(ctx.rcx as *const _),
-            4 => ctx.r8().map(|v| v as _),
+            4 => Some(ctx.r8 as *const _),
-            5 => ctx.r9().map(|v| v as _),
+            5 => Some(ctx.r9 as *const _),
             _ => None,
         }
     }
 
     fn from_retval(ctx: &pt_regs) -> Option<Self> {
-        ctx.rax().map(|v| v as _)
+        Some(ctx.rax as *const _)
     }
 }
 
@@ -120,14 +120,14 @@ impl<T> FromPtRegs for *const T {
 impl<T> FromPtRegs for *const T {
     fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
         if n <= 6 {
-            ctx.uregs().map(|regs| regs[n] as _)
+            Some(ctx.uregs.regs[n] as *const _)
         } else {
             None
         }
     }
 
     fn from_retval(ctx: &pt_regs) -> Option<Self> {
-        ctx.uregs().map(|regs| regs[0] as _)
+        Some(ctx.uregs.regs[0] as *const _)
     }
 }
 
@@ -135,7 +135,7 @@ impl<T> FromPtRegs for *const T {
 impl<T> FromPtRegs for *const T {
     fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
         if n <= 7 {
-            ctx.regs().map(|regs| regs[n] as _)
+            Some(ctx.regs.regs[n] as *const _)
         } else {
             None
         }
@@ -150,18 +150,18 @@ impl<T> FromPtRegs for *const T {
 impl<T> FromPtRegs for *mut T {
     fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
         match n {
-            0 => ctx.rdi().map(|v| v as _),
+            0 => Some(ctx.rdi as *mut _),
-            1 => ctx.rsi().map(|v| v as _),
+            1 => Some(ctx.rsi as *mut _),
-            2 => ctx.rdx().map(|v| v as _),
+            2 => Some(ctx.rdx as *mut _),
-            3 => ctx.rcx().map(|v| v as _),
+            3 => Some(ctx.rcx as *mut _),
-            4 => ctx.r8().map(|v| v as _),
+            4 => Some(ctx.r8 as *mut _),
-            5 => ctx.r9().map(|v| v as _),
+            5 => Some(ctx.r9 as *mut _),
             _ => None,
         }
     }
 
     fn from_retval(ctx: &pt_regs) -> Option<Self> {
-        ctx.rax().map(|v| v as _)
+        Some(ctx.rax as *mut _)
     }
 }
 
@@ -169,14 +169,14 @@ impl<T> FromPtRegs for *mut T {
 impl<T> FromPtRegs for *mut T {
     fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
         if n <= 6 {
-            ctx.uregs().map(|regs| regs[n] as _)
+            Some(ctx.uregs.regs[n] as *mut _)
         } else {
             None
         }
     }
 
     fn from_retval(ctx: &pt_regs) -> Option<Self> {
-        ctx.uregs().map(|regs| regs[0] as _)
+        Some(ctx.uregs.regs[0] as *mut _)
     }
 }
 
@@ -184,14 +184,14 @@ impl<T> FromPtRegs for *mut T {
 impl<T> FromPtRegs for *mut T {
     fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
         if n <= 7 {
-            ctx.regs().map(|regs| regs[n] as _)
+            Some(ctx.regs.regs[n] as *mut _)
         } else {
             None
         }
     }
 
     fn from_retval(ctx: &pt_regs) -> Option<Self> {
-        ctx.regs().map(|regs| regs[0] as _)
+        Some(ctx.regs.regs[0] as *mut _)
     }
 }
 
@@ -202,18 +202,18 @@ macro_rules! impl_from_pt_regs {
         impl FromPtRegs for $type {
             fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
                 match n {
-                    0 => ctx.rdi().map(|v| v as _),
+                    0 => Some(ctx.rdi as *const $type as _),
-                    1 => ctx.rsi().map(|v| v as _),
+                    1 => Some(ctx.rsi as *const $type as _),
-                    2 => ctx.rdx().map(|v| v as _),
+                    2 => Some(ctx.rdx as *const $type as _),
-                    3 => ctx.rcx().map(|v| v as _),
+                    3 => Some(ctx.rcx as *const $type as _),
-                    4 => ctx.r8().map(|v| v as _),
+                    4 => Some(ctx.r8 as *const $type as _),
-                    5 => ctx.r9().map(|v| v as _),
+                    5 => Some(ctx.r9 as *const $type as _),
                     _ => None,
                 }
             }
 
             fn from_retval(ctx: &pt_regs) -> Option<Self> {
-                ctx.rax().map(|v| v as _)
+                Some(ctx.rax as *const $type as _)
             }
         }
 
@@ -221,14 +221,14 @@ macro_rules! impl_from_pt_regs {
         impl FromPtRegs for $type {
             fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
                 if n <= 6 {
-                    ctx.uregs().map(|regs| regs[n] as _)
+                    Some(ctx.uregs.regs[n] as *const $type as _)
                 } else {
                     None
                 }
             }
 
             fn from_retval(ctx: &pt_regs) -> Option<Self> {
-                ctx.uregs().map(|regs| regs[0] as _)
+                Some(ctx.uregs.regs[0] as *const $type as _)
             }
         }
 
@@ -236,14 +236,14 @@ macro_rules! impl_from_pt_regs {
         impl FromPtRegs for $type {
             fn from_argument(ctx: &pt_regs, n: usize) -> Option<Self> {
                 if n <= 7 {
-                    ctx.regs().map(|regs| regs[n] as _)
+                    Some(ctx.regs.regs[n] as *const $type as _)
                 } else {
                     None
                 }
             }
 
             fn from_retval(ctx: &pt_regs) -> Option<Self> {
-                ctx.regs().map(|regs| regs[0] as _)
+                Some(ctx.regs.regs[0] as *const $type as _)
             }
         }
     };

xtask/src/codegen/aya_bpf_bindings.rs

@@ -3,11 +3,7 @@ use proc_macro2::TokenStream;
 use quote::ToTokens;
 use std::path::PathBuf;
 
-use aya_gen::{
+use aya_gen::{bindgen, write_to_file_fmt};
-    bindgen,
-    getters::{generate_getters_for_items, read_getter},
-    write_to_file_fmt,
-};
 use syn::{parse_str, Item};
 
 use crate::codegen::{
@@ -116,19 +112,6 @@ pub fn codegen(opts: &Options) -> Result<(), anyhow::Error> {
             &generated.join("helpers.rs"),
             &format!("use super::bindings::*; {}", helpers),
         )?;
-
-        // write the bpf_probe_read() getters
-        let bpf_probe_read = syn::parse_str("crate::bpf_probe_read").unwrap();
-        write_to_file_fmt(
-            &generated.join("getters.rs"),
-            &format!(
-                "use super::bindings::*; {}",
-                &generate_getters_for_items(&tree.items, |getter| {
-                    read_getter(getter, &bpf_probe_read)
-                })
-                .to_string()
-            ),
-        )?;
     }
 
     Ok(())