From ebdb20fc06feb0513aa18dfa7fae05dd7c5b48dc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 7 May 2020 11:41:44 -0700 Subject: [PATCH] Remove the `arrayvec` dependency This commit removes the `arrayvec` dependency from `gimli` to continue the work started in #494. The goal here is to absolutely minimize the number of dependencies (ideally zero) when using only the bare bones of the crate. --- Cargo.toml | 3 +- src/read/cfi.rs | 173 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 132 insertions(+), 44 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0b3ee6f3c..a9743db05 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,6 @@ travis-ci = { repository = "gimli-rs/gimli" } coveralls = { repository = "gimli-rs/gimli" } [dependencies] -arrayvec = { version = "0.5.0", default-features = false, optional = true } fallible-iterator = { version = "0.2.0", default-features = false, optional = true } indexmap = { version = "1.0.2", optional = true } smallvec = { version = "1.1.0", default-features = false, optional = true } @@ -35,7 +34,7 @@ test-assembler = "0.1.3" typed-arena = "2" [features] -read = ["arrayvec", "smallvec", "stable_deref_trait"] +read = ["smallvec", "stable_deref_trait"] write = ["indexmap"] std = ["fallible-iterator/std", "stable_deref_trait/std"] default = ["read", "write", "std", "fallible-iterator"] diff --git a/src/read/cfi.rs b/src/read/cfi.rs index 5071b064a..72ff53081 100644 --- a/src/read/cfi.rs +++ b/src/read/cfi.rs @@ -1,9 +1,8 @@ use alloc::boxed::Box; -use arrayvec::ArrayVec; use core::cmp::{Ord, Ordering}; -use core::fmt::Debug; +use core::fmt::{self, Debug}; use core::iter::FromIterator; -use core::mem; +use core::mem::{self, MaybeUninit}; use crate::common::{DebugFrameOffset, EhFrameOffset, Encoding, Format, Register, SectionId}; use crate::constants::{self, DwEhPe}; @@ -1804,15 +1803,15 @@ impl UninitializedUnwindContext { } const MAX_UNWIND_STACK_DEPTH: usize = 4; -type UnwindContextStack = ArrayVec<[UnwindTableRow; MAX_UNWIND_STACK_DEPTH]>; /// An unwinding context. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Eq)] pub struct UnwindContext { // Stack of rows. The last row is the row currently being built by the // program. There is always at least one row. The vast majority of CFI // programs will only ever have one row on the stack. - stack: UnwindContextStack, + stack_storage: [UnwindTableRow; MAX_UNWIND_STACK_DEPTH], + stack_len: usize, // If we are evaluating an FDE's instructions, then `is_initialized` will be // `true` and `initial_rules` will contain the initial register rules @@ -1831,7 +1830,8 @@ pub struct UnwindContext { impl UnwindContext { fn new() -> UnwindContext { let mut ctx = UnwindContext { - stack: Default::default(), + stack_storage: Default::default(), + stack_len: 0, is_initialized: false, initial_rules: Default::default(), }; @@ -1839,10 +1839,18 @@ impl UnwindContext { ctx } + fn stack(&self) -> &[UnwindTableRow] { + &self.stack_storage[..self.stack_len] + } + + fn stack_mut(&mut self) -> &mut [UnwindTableRow] { + &mut self.stack_storage[..self.stack_len] + } + fn reset(&mut self) { - self.stack.clear(); - let res = self.stack.try_push(UnwindTableRow::default()); - debug_assert!(res.is_ok()); + self.stack_len = 0; + let res = self.try_push(UnwindTableRow::default()); + debug_assert!(res); self.initial_rules.clear(); self.is_initialized = false; @@ -1855,23 +1863,23 @@ impl UnwindContext { #[inline] fn assert_fully_uninitialized(&self) { assert_eq!(self.is_initialized, false); - assert_eq!(self.initial_rules.rules.len(), 0); - assert_eq!(self.stack.len(), 1); - assert!(self.stack[0].is_default()); + assert_eq!(self.initial_rules.rules().len(), 0); + assert_eq!(self.stack().len(), 1); + assert!(self.stack()[0].is_default()); } fn row(&self) -> &UnwindTableRow { - self.stack.last().unwrap() + self.stack().last().unwrap() } fn row_mut(&mut self) -> &mut UnwindTableRow { - self.stack.last_mut().unwrap() + self.stack_mut().last_mut().unwrap() } fn save_initial_rules(&mut self) { assert_eq!(self.is_initialized, false); - self.initial_rules - .clone_from(&self.stack.last().unwrap().registers); + let registers = &self.stack_storage[self.stack_len - 1].registers; + self.initial_rules.clone_from(®isters); self.is_initialized = true; } @@ -1909,14 +1917,34 @@ impl UnwindContext { fn push_row(&mut self) -> Result<()> { let new_row = self.row().clone(); - self.stack - .try_push(new_row) - .map_err(|_| Error::CfiStackFull) + if self.try_push(new_row) { + Ok(()) + } else { + Err(Error::CfiStackFull) + } + } + + fn try_push(&mut self, row: UnwindTableRow) -> bool { + if self.stack_len < self.stack_storage.len() { + self.stack_storage[self.stack_len] = row; + self.stack_len += 1; + true + } else { + false + } } fn pop_row(&mut self) { - assert!(self.stack.len() > 1); - self.stack.pop(); + assert!(self.stack().len() > 1); + self.stack_len -= 1; + } +} + +impl PartialEq for UnwindContext { + fn eq(&self, other: &UnwindContext) -> bool { + self.stack() == other.stack() + && self.initial_rules == other.initial_rules + && self.is_initialized == other.is_initialized } } @@ -2010,7 +2038,7 @@ impl<'a, R: Reader> UnwindTable<'a, R> { ctx: &'a mut UnwindContext, fde: &FrameDescriptionEntry, ) -> UnwindTable<'a, R> { - assert!(ctx.stack.len() >= 1); + assert!(ctx.stack().len() >= 1); UnwindTable { code_alignment_factor: fde.cie().code_alignment_factor(), data_alignment_factor: fde.cie().data_alignment_factor(), @@ -2028,7 +2056,7 @@ impl<'a, R: Reader> UnwindTable<'a, R> { ctx: &'a mut UnwindContext, cie: &CommonInformationEntry, ) -> UnwindTable<'a, R> { - assert!(ctx.stack.len() >= 1); + assert!(ctx.stack().len() >= 1); UnwindTable { code_alignment_factor: cie.code_alignment_factor(), data_alignment_factor: cie.data_alignment_factor(), @@ -2046,7 +2074,7 @@ impl<'a, R: Reader> UnwindTable<'a, R> { /// Unfortunately, this cannot be used with `FallibleIterator` because of /// the restricted lifetime of the yielded item. pub fn next_row(&mut self) -> Result>> { - assert!(self.ctx.stack.len() >= 1); + assert!(self.ctx.stack().len() >= 1); self.ctx.set_start_address(self.next_start_address); loop { @@ -2232,8 +2260,8 @@ impl<'a, R: Reader> UnwindTable<'a, R> { self.ctx.push_row()?; } RestoreState => { - assert!(self.ctx.stack.len() > 0); - if self.ctx.stack.len() == 1 { + assert!(self.ctx.stack().len() > 0); + if self.ctx.stack().len() == 1 { return Err(Error::PopWithEmptyStack); } // Pop state while preserving current location. @@ -2279,16 +2307,37 @@ impl<'a, R: Reader> UnwindTable<'a, R> { // - https://github.com/libunwind/libunwind/blob/11fd461095ea98f4b3e3a361f5a8a558519363fa/include/tdep-mips/dwarf-config.h#L31 // // TODO: Consider using const generics for the array size. -#[derive(Clone, Debug)] struct RegisterRuleMap { - rules: ArrayVec<[(Register, RegisterRule); 192]>, + rules_storage: MaybeUninit<[(Register, RegisterRule); MAX_RULES]>, + rules_len: usize, +} + +const MAX_RULES: usize = 192; + +impl Debug for RegisterRuleMap { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("RegisterRuleMap") + .field("rules", &self.rules()) + .finish() + } } impl Default for RegisterRuleMap { fn default() -> Self { RegisterRuleMap { - rules: Default::default(), + rules_storage: MaybeUninit::uninit(), + rules_len: 0, + } + } +} + +impl Clone for RegisterRuleMap { + fn clone(&self) -> Self { + let mut new = RegisterRuleMap::default(); + for (register, rule) in self.rules() { + new.push(register.clone(), rule.clone()).unwrap(); } + return new; } } @@ -2298,11 +2347,31 @@ impl Default for RegisterRuleMap { /// other signal-unsafe operations. impl RegisterRuleMap { fn is_default(&self) -> bool { - self.rules.len() == 0 + self.rules_len == 0 + } + + fn rules(&self) -> &[(Register, RegisterRule)] { + // Note that the unsafety here relies on the mutation of + // `self.rules_len` in `self.push` below, which guarantees that once + // we've pushed something the `rules_storage` is valid for that many + // elements. + unsafe { + core::slice::from_raw_parts(self.rules_storage.as_ptr() as *const _, self.rules_len) + } + } + + fn rules_mut(&mut self) -> &mut [(Register, RegisterRule)] { + // See the note in `rules` on safety here. + unsafe { + core::slice::from_raw_parts_mut( + self.rules_storage.as_mut_ptr() as *mut _, + self.rules_len, + ) + } } fn get(&self, register: Register) -> RegisterRule { - self.rules + self.rules() .iter() .find(|rule| rule.0 == register) .map(|r| { @@ -2315,18 +2384,22 @@ impl RegisterRuleMap { fn set(&mut self, register: Register, rule: RegisterRule) -> Result<()> { if !rule.is_defined() { let idx = self - .rules + .rules() .iter() .enumerate() .find(|&(_, r)| r.0 == register) .map(|(i, _)| i); if let Some(idx) = idx { - self.rules.swap_remove(idx); + let (a, b) = self.rules_mut().split_at_mut(idx + 1); + if b.len() > 0 { + mem::swap(&mut a[a.len() - 1], &mut b[b.len() - 1]); + } + self.rules_len -= 1; } return Ok(()); } - for &mut (reg, ref mut old_rule) in &mut self.rules { + for &mut (reg, ref mut old_rule) in self.rules_mut() { debug_assert!(old_rule.is_defined()); if reg == register { mem::replace(old_rule, rule); @@ -2334,17 +2407,33 @@ impl RegisterRuleMap { } } - self.rules - .try_push((register, rule)) - .map_err(|_| Error::TooManyRegisterRules) + self.push(register, rule) } fn clear(&mut self) { - self.rules.clear(); + self.rules_len = 0; } fn iter(&self) -> RegisterRuleIter { - RegisterRuleIter(self.rules.iter()) + RegisterRuleIter(self.rules().iter()) + } + + fn push(&mut self, register: Register, rule: RegisterRule) -> Result<()> { + if self.rules_len >= MAX_RULES { + return Err(Error::TooManyRegisterRules); + } + // The unsafety here comes from working with `MaybeUninit` to initialize + // our 192-element array. We're pushing a new element onto that array + // here. Just above we did a bounds check to make sure we actually have + // space to push something and here we're doing the actual memory write, + // along with an increment of `rules_len` which will affect the unsafe + // implementations of `rules` and `rules_mut` above. + unsafe { + let ptr = self.rules_storage.as_mut_ptr() as *mut (Register, RegisterRule); + *ptr.add(self.rules_len) = (register, rule); + self.rules_len += 1; + } + Ok(()) } } @@ -2373,14 +2462,14 @@ where R: Reader + PartialEq, { fn eq(&self, rhs: &Self) -> bool { - for &(reg, ref rule) in &self.rules { + for &(reg, ref rule) in self.rules() { debug_assert!(rule.is_defined()); if *rule != rhs.get(reg) { return false; } } - for &(reg, ref rhs_rule) in &rhs.rules { + for &(reg, ref rhs_rule) in rhs.rules() { debug_assert!(rhs_rule.is_defined()); if *rhs_rule != self.get(reg) { return false;