From fe047d79a3f501631ae6406444769f6d5f6fed24 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 9 Aug 2023 09:58:39 -0400 Subject: [PATCH] aya-log-common: Simplify - Remove `TagLenValue`; this type has a single method, which is now a function. - Remove generics from `TagLenValue::write` (now `write`). The tag is always `u8`, and the value is always a sequence of bytes. - Replace slicing operations which can panic with calls to `get` which explicit check bounds. --- aya-log-common/src/lib.rs | 107 +++++++++++----------------- xtask/public-api/aya-log-common.txt | 4 ++ 2 files changed, 44 insertions(+), 67 deletions(-) diff --git a/aya-log-common/src/lib.rs b/aya-log-common/src/lib.rs index 0d789e3b..c634c51e 100644 --- a/aya-log-common/src/lib.rs +++ b/aya-log-common/src/lib.rs @@ -1,6 +1,6 @@ #![no_std] -use core::{mem, num, ptr}; +use core::num; use num_enum::IntoPrimitive; @@ -86,7 +86,7 @@ pub trait UpperMacFormatter {} impl UpperMacFormatter for [u8; 6] {} #[repr(u8)] -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, IntoPrimitive)] pub enum RecordField { Target = 1, Level, @@ -99,7 +99,7 @@ pub enum RecordField { /// Types which are supported by aya-log and can be safely sent from eBPF /// programs to userspace. #[repr(u8)] -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, IntoPrimitive)] pub enum Argument { DisplayHint, @@ -147,53 +147,24 @@ pub enum DisplayHint { UpperMac, } -struct TagLenValue { - pub tag: T, - pub value: V, -} - -impl TagLenValue -where - V: IntoIterator, - ::IntoIter: ExactSizeIterator, -{ - pub(crate) fn write(self, mut buf: &mut [u8]) -> Result { - // Break the abstraction to please the verifier. - if buf.len() > LOG_BUF_CAPACITY { - buf = &mut buf[..LOG_BUF_CAPACITY]; - } - let Self { tag, value } = self; - let value = value.into_iter(); - let len = value.len(); - let wire_len: LogValueLength = value - .len() - .try_into() - .map_err(|num::TryFromIntError { .. }| ())?; - let size = mem::size_of_val(&tag) + mem::size_of_val(&wire_len) + len; - if size > buf.len() { - return Err(()); - } - - let tag_size = mem::size_of_val(&tag); - unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, tag) }; - buf = &mut buf[tag_size..]; - - unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, wire_len) }; - buf = &mut buf[mem::size_of_val(&wire_len)..]; - - buf.iter_mut().zip(value).for_each(|(dst, src)| { - *dst = src; - }); - - Ok(size) - } -} - -impl TagLenValue { - #[inline(always)] - pub(crate) fn new(tag: T, value: V) -> TagLenValue { - TagLenValue { tag, value } +pub(crate) fn write(tag: u8, value: &[u8], buf: &mut [u8]) -> Result { + let wire_len: LogValueLength = value + .len() + .try_into() + .map_err(|num::TryFromIntError { .. }| ())?; + let mut size = 0; + macro_rules! copy_from_slice { + ($value:expr) => {{ + let buf = buf.get_mut(size..).ok_or(())?; + let buf = buf.get_mut(..$value.len()).ok_or(())?; + buf.copy_from_slice($value); + size += $value.len(); + }}; } + copy_from_slice!(&[tag]); + copy_from_slice!(&wire_len.to_ne_bytes()); + copy_from_slice!(value); + Ok(size) } pub trait WriteToBuf { @@ -205,7 +176,7 @@ macro_rules! impl_write_to_buf { ($type:ident, $arg_type:expr) => { impl WriteToBuf for $type { fn write(self, buf: &mut [u8]) -> Result { - TagLenValue::new($arg_type, self.to_ne_bytes()).write(buf) + write($arg_type.into(), &self.to_ne_bytes(), buf) } } }; @@ -228,45 +199,45 @@ impl_write_to_buf!(f64, Argument::F64); impl WriteToBuf for [u8; 16] { fn write(self, buf: &mut [u8]) -> Result { - TagLenValue::new(Argument::ArrU8Len16, self).write(buf) + write(Argument::ArrU8Len16.into(), &self, buf) } } impl WriteToBuf for [u16; 8] { fn write(self, buf: &mut [u8]) -> Result { let bytes = unsafe { core::mem::transmute::<_, [u8; 16]>(self) }; - TagLenValue::new(Argument::ArrU16Len8, bytes).write(buf) + write(Argument::ArrU16Len8.into(), &bytes, buf) } } impl WriteToBuf for [u8; 6] { fn write(self, buf: &mut [u8]) -> Result { - TagLenValue::new(Argument::ArrU8Len6, self).write(buf) + write(Argument::ArrU8Len6.into(), &self, buf) } } impl WriteToBuf for &[u8] { fn write(self, buf: &mut [u8]) -> Result { - TagLenValue::new(Argument::Bytes, self.iter().copied()).write(buf) + write(Argument::Bytes.into(), self, buf) } } impl WriteToBuf for &str { fn write(self, buf: &mut [u8]) -> Result { - TagLenValue::new(Argument::Str, self.as_bytes().iter().copied()).write(buf) + write(Argument::Str.into(), self.as_bytes(), buf) } } impl WriteToBuf for DisplayHint { fn write(self, buf: &mut [u8]) -> Result { let v: u8 = self.into(); - TagLenValue::new(Argument::DisplayHint, v.to_ne_bytes()).write(buf) + write(Argument::DisplayHint.into(), &v.to_ne_bytes(), buf) } } #[allow(clippy::result_unit_err)] #[doc(hidden)] -#[inline(always)] +#[inline(always)] // This function takes too many arguments to not be inlined. pub fn write_record_header( buf: &mut [u8], target: &str, @@ -278,16 +249,18 @@ pub fn write_record_header( ) -> Result { let level: u8 = level.into(); let mut size = 0; - size += TagLenValue::new(RecordField::Target, target.as_bytes().iter().copied()) - .write(&mut buf[size..])?; - size += TagLenValue::new(RecordField::Level, level.to_ne_bytes()).write(&mut buf[size..])?; - size += TagLenValue::new(RecordField::Module, module.as_bytes().iter().copied()) - .write(&mut buf[size..])?; - size += TagLenValue::new(RecordField::File, file.as_bytes().iter().copied()) - .write(&mut buf[size..])?; - size += TagLenValue::new(RecordField::Line, line.to_ne_bytes()).write(&mut buf[size..])?; - size += - TagLenValue::new(RecordField::NumArgs, num_args.to_ne_bytes()).write(&mut buf[size..])?; + macro_rules! write { + ($tag:expr, $value:expr) => {{ + let buf = buf.get_mut(size..).ok_or(())?; + size += write($tag.into(), $value, buf)?; + }}; + } + write!(RecordField::Target, target.as_bytes()); + write!(RecordField::Level, &level.to_ne_bytes()); + write!(RecordField::Module, module.as_bytes()); + write!(RecordField::File, file.as_bytes()); + write!(RecordField::Line, &line.to_ne_bytes()); + write!(RecordField::NumArgs, &num_args.to_ne_bytes()); Ok(size) } diff --git a/xtask/public-api/aya-log-common.txt b/xtask/public-api/aya-log-common.txt index 7e28268a..1b3e48c1 100644 --- a/xtask/public-api/aya-log-common.txt +++ b/xtask/public-api/aya-log-common.txt @@ -18,6 +18,8 @@ pub aya_log_common::Argument::U32 pub aya_log_common::Argument::U64 pub aya_log_common::Argument::U8 pub aya_log_common::Argument::Usize +impl core::convert::From for u8 +pub fn u8::from(enum_value: aya_log_common::Argument) -> Self impl core::clone::Clone for aya_log_common::Argument pub fn aya_log_common::Argument::clone(&self) -> aya_log_common::Argument impl core::fmt::Debug for aya_log_common::Argument @@ -134,6 +136,8 @@ pub aya_log_common::RecordField::Line pub aya_log_common::RecordField::Module pub aya_log_common::RecordField::NumArgs pub aya_log_common::RecordField::Target = 1 +impl core::convert::From for u8 +pub fn u8::from(enum_value: aya_log_common::RecordField) -> Self impl core::clone::Clone for aya_log_common::RecordField pub fn aya_log_common::RecordField::clone(&self) -> aya_log_common::RecordField impl core::fmt::Debug for aya_log_common::RecordField