Cleanup (#547)

* General cleanup

* Adds deny(clippy::ptr_as_ptr).

src/aligned_memory.rs

@@ -27,7 +27,6 @@ pub struct AlignedMemory<const ALIGN: usize> {
 impl<const ALIGN: usize> AlignedMemory<ALIGN> {
     fn get_mem(max_len: usize) -> (Vec<u8>, usize) {
         let mut mem: Vec<u8> = Vec::with_capacity(max_len.saturating_add(ALIGN));
-        mem.push(0);
         let align_offset = mem.as_ptr().align_offset(ALIGN);
         mem.resize(align_offset, 0);
         (mem, align_offset)
@@ -122,7 +121,7 @@ impl<const ALIGN: usize> AlignedMemory<ALIGN> {
             _ => {
                 return Err(std::io::Error::new(
                     std::io::ErrorKind::InvalidInput,
-                    "aligned memory resize failed",
+                    "aligned memory fill_write failed",
                 ))
             }
         };

src/assembler.rs

@@ -254,6 +254,17 @@ fn insn(opc: u8, dst: i64, src: i64, off: i64, imm: i64) -> Result<Insn, String>
     })
 }
 
+fn resolve_label(
+    insn_ptr: usize,
+    labels: &HashMap<&str, usize>,
+    label: &str,
+) -> Result<i64, String> {
+    labels
+        .get(label)
+        .map(|target_pc| *target_pc as i64 - insn_ptr as i64 - 1)
+        .ok_or_else(|| format!("Label not found {label}"))
+}
+
 /// Parse assembly source and translate to binary.
 ///
 /// # Examples
@@ -299,16 +310,6 @@ pub fn assemble<C: ContextObject>(
     } else {
         SBPFVersion::V1
     };
-    fn resolve_label(
-        insn_ptr: usize,
-        labels: &HashMap<&str, usize>,
-        label: &str,
-    ) -> Result<i64, String> {
-        labels
-            .get(label)
-            .map(|target_pc| *target_pc as i64 - insn_ptr as i64 - 1)
-            .ok_or_else(|| format!("Label not found {label}"))
-    }
 
     let statements = parse(src)?;
     let instruction_map = make_instruction_map();
@@ -321,7 +322,7 @@ pub fn assemble<C: ContextObject>(
             Statement::Label { name } => {
                 if name.starts_with("function_") || name == "entrypoint" {
                     function_registry
-                        .register_function(insn_ptr as u32, name.as_bytes().to_vec(), insn_ptr)
+                        .register_function(insn_ptr as u32, name.as_bytes(), insn_ptr)
                         .map_err(|_| format!("Label hash collision {name}"))?;
                 }
                 labels.insert(name.as_str(), insn_ptr);
@@ -372,7 +373,7 @@ pub fn assemble<C: ContextObject>(
                             function_registry
                                 .register_function(
                                     target_pc as u32,
-                                    label.as_bytes().to_vec(),
+                                    label.as_bytes(),
                                     target_pc as usize,
                                 )
                                 .map_err(|_| format!("Label hash collision {name}"))?;

src/elf.rs

@@ -1219,7 +1219,7 @@ mod test {
 
         let write_header = |header: Elf64Ehdr| unsafe {
             let mut bytes = elf_bytes.clone();
-            std::ptr::write(bytes.as_mut_ptr() as *mut Elf64Ehdr, header);
+            std::ptr::write(bytes.as_mut_ptr().cast::<Elf64Ehdr>(), header);
             bytes
         };
 
@@ -1322,7 +1322,7 @@ mod test {
 
         let write_header = |header: Elf64Ehdr| unsafe {
             let mut bytes = elf_bytes.clone();
-            std::ptr::write(bytes.as_mut_ptr() as *mut Elf64Ehdr, header);
+            std::ptr::write(bytes.as_mut_ptr().cast::<Elf64Ehdr>(), header);
             bytes
         };
 

src/elf_parser/mod.rs

@@ -135,7 +135,7 @@ impl<'a> Elf64<'a> {
         let section_header_table =
             slice_from_bytes::<Elf64Shdr>(elf_bytes, section_header_table_range.clone())?;
         section_header_table
-            .get(0)
+            .first()
             .filter(|section_header| section_header.sh_type == SHT_NULL)
             .ok_or(ElfParserError::InvalidSectionHeader)?;
 

src/error.rs

@@ -4,18 +4,7 @@
 // the MIT license <http://opensource.org/licenses/MIT>, at your option. This file may not be
 // copied, modified, or distributed except according to those terms.
 
-//! This module contains all the definitions related to eBPF, and some functions permitting to
-//! manipulate eBPF instructions.
-//!
-//! The number of bytes in an instruction, the maximum number of instructions in a program, and
-//! also all operation codes are defined here as constants.
-//!
-//! The structure for an instruction used by this crate, as well as the function to extract it from
-//! a program, is also defined in the module.
-//!
-//! To learn more about these instructions, see the Linux kernel documentation:
-//! <https://www.kernel.org/doc/Documentation/networking/filter.txt>, or for a shorter version of
-//! the list of the operation codes: <https://github.com/iovisor/bpf-docs/blob/master/eBPF.md>
+//! This module contains error and result types
 
 use {
     crate::{elf::ElfError, memory_region::AccessType, verifier::VerifierError},
@@ -156,7 +145,7 @@ impl<T: std::fmt::Debug, E: std::fmt::Debug> StableResult<T, E> {
         allow(dead_code)
     )]
     pub(crate) fn discriminant(&self) -> u64 {
-        unsafe { *(self as *const _ as *const u64) }
+        unsafe { *std::ptr::addr_of!(*self).cast::<u64>() }
     }
 }
 

src/jit.rs

@@ -47,7 +47,7 @@ impl JitProgram {
             let raw = allocate_pages(pc_loc_table_size + over_allocated_code_size)?;
             Ok(Self {
                 page_size,
-                pc_section: std::slice::from_raw_parts_mut(raw as *mut usize, pc),
+                pc_section: std::slice::from_raw_parts_mut(raw.cast::<usize>(), pc),
                 text_section: std::slice::from_raw_parts_mut(
                     raw.add(pc_loc_table_size),
                     over_allocated_code_size,
@@ -80,7 +80,7 @@ impl JitProgram {
             self.text_section =
                 std::slice::from_raw_parts_mut(raw.add(pc_loc_table_size), text_section_usage);
             protect_pages(
-                self.pc_section.as_mut_ptr() as *mut u8,
+                self.pc_section.as_mut_ptr().cast::<u8>(),
                 pc_loc_table_size,
                 false,
             )?;
@@ -119,7 +119,7 @@ impl JitProgram {
                 "pop rbp",
                 "pop rbx",
                 host_stack_pointer = in(reg) &mut vm.host_stack_pointer,
-                inlateout("rdi") (vm as *mut _ as *mut u64).offset(get_runtime_environment_key() as isize) => _,
+                inlateout("rdi") std::ptr::addr_of_mut!(*vm).cast::<u64>().offset(get_runtime_environment_key() as isize) => _,
                 inlateout("rax") (vm.previous_instruction_meter as i64).wrapping_add(registers[11] as i64) => _,
                 inlateout("r10") self.pc_section[registers[11] as usize] => _,
                 inlateout("r11") &registers => _,
@@ -1269,7 +1269,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
     }
 
     fn emit_set_exception_kind(&mut self, err: EbpfError) {
-        let err_kind = unsafe { *(&err as *const _ as *const u64) };
+        let err_kind = unsafe { *std::ptr::addr_of!(err).cast::<u64>() };
         let err_discriminant = ProgramResult::Err(err).discriminant();
         self.emit_ins(X86Instruction::lea(OperandSize::S64, REGISTER_PTR_TO_VM, REGISTER_OTHER_SCRATCH, Some(X86IndirectAccess::Offset(self.slot_in_vm(RuntimeEnvironmentSlot::ProgramResult)))));
         self.emit_ins(X86Instruction::store_immediate(OperandSize::S64, REGISTER_OTHER_SCRATCH, X86IndirectAccess::Offset(0), err_discriminant as i64)); // result.discriminant = err_discriminant;
@@ -1615,8 +1615,9 @@ mod tests {
             ($env:expr, $entry:ident, $slot:ident) => {
                 assert_eq!(
                     unsafe {
-                        (&$env.$entry as *const _ as *const u64)
-                            .offset_from(&$env as *const _ as *const u64) as usize
+                        std::ptr::addr_of!($env.$entry)
+                            .cast::<u64>()
+                            .offset_from(std::ptr::addr_of!($env).cast::<u64>()) as usize
                     },
                     RuntimeEnvironmentSlot::$slot as usize,
                 );

src/lib.rs

@@ -16,6 +16,7 @@
     html_favicon_url = "https://raw.githubusercontent.com/qmonnet/rbpf/master/misc/rbpf.ico"
 )]
 #![deny(clippy::arithmetic_side_effects)]
+#![deny(clippy::ptr_as_ptr)]
 
 extern crate byteorder;
 extern crate combine;

src/memory_management.rs

@@ -113,16 +113,16 @@ pub unsafe fn allocate_pages(size_in_bytes: usize) -> Result<*mut u8, EbpfError>
         winnt::MEM_RESERVE | winnt::MEM_COMMIT,
         winnt::PAGE_READWRITE,
     );
-    Ok(raw as *mut u8)
+    Ok(raw.cast::<u8>())
 }
 
 pub unsafe fn free_pages(raw: *mut u8, size_in_bytes: usize) -> Result<(), EbpfError> {
     #[cfg(not(target_os = "windows"))]
-    libc_error_guard!(munmap, raw as *mut _, size_in_bytes);
+    libc_error_guard!(munmap, raw.cast::<c_void>(), size_in_bytes);
     #[cfg(target_os = "windows")]
     winapi_error_guard!(
         VirtualFree,
-        raw as *mut _,
+        raw.cast::<c_void>(),
         size_in_bytes,
         winnt::MEM_RELEASE, // winnt::MEM_DECOMMIT
     );
@@ -138,7 +138,7 @@ pub unsafe fn protect_pages(
     {
         libc_error_guard!(
             mprotect,
-            raw as *mut _,
+            raw.cast::<c_void>(),
             size_in_bytes,
             if executable_flag {
                 libc::PROT_EXEC | libc::PROT_READ
@@ -153,7 +153,7 @@ pub unsafe fn protect_pages(
         let ptr_old: *mut minwindef::DWORD = &mut old;
         winapi_error_guard!(
             VirtualProtect,
-            raw as *mut _,
+            raw.cast::<c_void>(),
             size_in_bytes,
             if executable_flag {
                 winnt::PAGE_EXECUTE_READ

src/memory_region.rs

@@ -104,12 +104,7 @@ impl MemoryRegion {
 
     /// Creates a new writable MemoryRegion from a mutable slice
     pub fn new_writable(slice: &mut [u8], vm_addr: u64) -> Self {
-        Self::new(
-            unsafe { std::mem::transmute::<&mut [u8], &[u8]>(slice) },
-            vm_addr,
-            0,
-            MemoryState::Writable,
-        )
+        Self::new(&*slice, vm_addr, 0, MemoryState::Writable)
     }
 
     /// Creates a new copy on write MemoryRegion.
@@ -121,12 +116,7 @@ impl MemoryRegion {
 
     /// Creates a new writable gapped MemoryRegion from a mutable slice
     pub fn new_writable_gapped(slice: &mut [u8], vm_addr: u64, vm_gap_size: u64) -> Self {
-        Self::new(
-            unsafe { std::mem::transmute::<&mut [u8], &[u8]>(slice) },
-            vm_addr,
-            vm_gap_size,
-            MemoryState::Writable,
-        )
+        Self::new(&*slice, vm_addr, vm_gap_size, MemoryState::Writable)
     }
 
     /// Convert a virtual machine address into a host address
@@ -399,7 +389,7 @@ impl<'a> UnalignedMemoryMapping<'a> {
         let initial_len = len;
         let initial_vm_addr = vm_addr;
         let mut value = 0u64;
-        let mut ptr = &mut value as *mut _ as *mut u8;
+        let mut ptr = std::ptr::addr_of_mut!(value).cast::<u8>();
 
         while len > 0 {
             let load_len = len.min(region.vm_addr_end.saturating_sub(vm_addr));
@@ -450,7 +440,7 @@ impl<'a> UnalignedMemoryMapping<'a> {
         // guaranteed to be unique.
         let cache = unsafe { &mut *self.cache.get() };
 
-        let mut src = &value as *const _ as *const u8;
+        let mut src = std::ptr::addr_of!(value).cast::<u8>();
 
         let mut region = match self.find_region(cache, vm_addr) {
             Some(region) if ensure_writable_region(region, &self.cow_cb) => {

src/program.rs

@@ -167,7 +167,7 @@ impl<T: Copy + PartialEq> FunctionRegistry<T> {
 
     /// Iterate over all keys
     pub fn keys(&self) -> impl Iterator<Item = u32> + '_ {
-        self.map.keys().cloned()
+        self.map.keys().copied()
     }
 
     /// Iterate over all entries
@@ -324,7 +324,7 @@ macro_rules! declare_builtin_function {
             ) {
                 use $crate::vm::ContextObject;
                 let vm = unsafe {
-                    &mut *(($vm as *mut u64).offset(-($crate::vm::get_runtime_environment_key() as isize)) as *mut $crate::vm::EbpfVm<$ContextObject>)
+                    &mut *($vm.cast::<u64>().offset(-($crate::vm::get_runtime_environment_key() as isize)).cast::<$crate::vm::EbpfVm<$ContextObject>>())
                 };
                 let config = vm.loader.get_config();
                 if config.enable_instruction_meter {

src/static_analysis.rs

@@ -553,8 +553,7 @@ impl<'a> Analysis<'a> {
                         format!("<tr><td align=\"left\">{}</td></tr>", html_escape(&desc))
                     }
                 })
-                .collect::<Vec<String>>()
-                .join("")
+                .collect::<String>()
             )?;
             if let Some(dynamic_analysis) = dynamic_analysis {
                 if let Some(recorded_edges) = dynamic_analysis.edges.get(&cfg_node_start) {

src/syscalls.rs

@@ -158,16 +158,10 @@ declare_builtin_function!(
         let host_addr: Result<u64, EbpfError> =
             memory_mapping.map(AccessType::Load, vm_addr, len).into();
         let host_addr = host_addr?;
-        let c_buf: *const i8 = host_addr as *const i8;
         unsafe {
-            for i in 0..len {
-                let c = std::ptr::read(c_buf.offset(i as isize));
-                if c == 0 {
-                    break;
-                }
-            }
-            let message = from_utf8(from_raw_parts(host_addr as *const u8, len as usize))
-                .unwrap_or("Invalid UTF-8 String");
+            let c_buf = from_raw_parts(host_addr as *const u8, len as usize);
+            let len = c_buf.iter().position(|c| *c == 0).unwrap_or(len as usize);
+            let message = from_utf8(&c_buf[0..len]).unwrap_or("Invalid UTF-8 String");
             println!("log: {message}");
         }
         Ok(0)

src/vm.rs

@@ -426,8 +426,10 @@ impl<'a, C: ContextObject> EbpfVm<'a, C> {
     pub fn invoke_function(&mut self, function: BuiltinFunction<C>) {
         function(
             unsafe {
-                (self as *mut _ as *mut u64).offset(get_runtime_environment_key() as isize)
-                    as *mut _
+                std::ptr::addr_of_mut!(*self)
+                    .cast::<u64>()
+                    .offset(get_runtime_environment_key() as isize)
+                    .cast::<Self>()
             },
             self.registers[1],
             self.registers[2],