From e2c0ca5110b0e7767f2bec942bee54fb118b1761 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Sep 2017 14:11:18 -0700 Subject: [PATCH 1/5] Update bindings. MozReview-Commit-ID: 8hyaJxfSi9L --- components/style/gecko/generated/bindings.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/components/style/gecko/generated/bindings.rs b/components/style/gecko/generated/bindings.rs index db302b197f7..7b8e300183b 100644 --- a/components/style/gecko/generated/bindings.rs +++ b/components/style/gecko/generated/bindings.rs @@ -1560,6 +1560,17 @@ extern "C" { extern "C" { pub fn Gecko_ShouldCreateStyleThreadPool() -> bool; } +extern "C" { + pub fn Gecko_GetSystemPageSize() -> usize; +} +extern "C" { + pub fn Gecko_ProtectBuffer(buffer: *mut ::std::os::raw::c_void, + size: usize); +} +extern "C" { + pub fn Gecko_UnprotectBuffer(buffer: *mut ::std::os::raw::c_void, + size: usize); +} extern "C" { pub fn Gecko_Construct_Default_nsStyleFont(ptr: *mut nsStyleFont, pres_context: @@ -2989,6 +3000,10 @@ extern "C" { *const ServoRawOffsetArc) -> ServoRawOffsetArc; } +extern "C" { + pub fn Servo_CorruptRuleHashAndCrash(set: RawServoStyleSetBorrowed, + index: usize); +} extern "C" { pub fn Gecko_CreateCSSErrorReporter(sheet: *mut ServoStyleSheet, loader: *mut Loader, uri: *mut nsIURI) From 98f370130dd0c41e84bbf07b288784da3bb811ce Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 26 Sep 2017 12:33:21 -0700 Subject: [PATCH 2/5] Semantics for ProtectedHashMap. MozReview-Commit-ID: K0m65uZi7iw --- Cargo.lock | 1 + components/hashglobe/src/lib.rs | 1 + components/hashglobe/src/protected.rs | 180 ++++++++++++++++++ components/malloc_size_of/lib.rs | 19 ++ components/style/hash.rs | 7 +- .../invalidation/element/invalidation_map.rs | 18 ++ components/style/selector_map.rs | 49 ++++- components/style/selector_parser.rs | 9 + components/style/stylist.rs | 35 +++- 9 files changed, 311 insertions(+), 8 deletions(-) create mode 100644 components/hashglobe/src/protected.rs diff --git a/Cargo.lock b/Cargo.lock index 5b9c3ec5bed..7e6808bbfcf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1061,6 +1061,7 @@ dependencies = [ "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "cssparser 0.21.2 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", + "hashglobe 0.1.0", "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "malloc_size_of 0.0.1", diff --git a/components/hashglobe/src/lib.rs b/components/hashglobe/src/lib.rs index cbbdacb4c8d..8a2d070e0e2 100644 --- a/components/hashglobe/src/lib.rs +++ b/components/hashglobe/src/lib.rs @@ -13,6 +13,7 @@ extern crate heapsize; mod alloc; pub mod hash_map; pub mod hash_set; +pub mod protected; mod shim; mod table; diff --git a/components/hashglobe/src/protected.rs b/components/hashglobe/src/protected.rs new file mode 100644 index 00000000000..99b0879c276 --- /dev/null +++ b/components/hashglobe/src/protected.rs @@ -0,0 +1,180 @@ +use hash_map::{Entry, HashMap, Iter, IterMut, Keys, RandomState, Values}; +use std::borrow::Borrow; +use std::hash::{BuildHasher, Hash}; + +use FailedAllocationError; + +#[derive(Clone, Debug)] +pub struct ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher +{ + map: HashMap, + readonly: bool, +} + +impl ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher +{ + #[inline(always)] + pub fn inner(&self) -> &HashMap { + &self.map + } + + #[inline(always)] + pub fn begin_mutation(&mut self) { + assert!(self.readonly); + self.readonly = false; + } + + #[inline(always)] + pub fn end_mutation(&mut self) { + assert!(!self.readonly); + self.readonly = true; + } + + #[inline(always)] + pub fn with_hasher(hash_builder: S) -> Self { + Self { + map: HashMap::::with_hasher(hash_builder), + readonly: true, + } + } + + #[inline(always)] + pub fn len(&self) -> usize { + self.map.len() + } + + #[inline(always)] + pub fn is_empty(&self) -> bool { + self.map.is_empty() + } + + #[inline(always)] + pub fn contains_key(&self, k: &Q) -> bool + where K: Borrow, + Q: Hash + Eq + { + self.map.contains_key(k) + } + + #[inline(always)] + pub fn keys(&self) -> Keys { + self.map.keys() + } + + #[inline(always)] + pub fn values(&self) -> Values { + self.map.values() + } + + #[inline(always)] + pub fn get(&self, k: &Q) -> Option<&V> + where K: Borrow, + Q: Hash + Eq + { + self.map.get(k) + } + + #[inline(always)] + pub fn iter(&self) -> Iter { + self.map.iter() + } + + #[inline(always)] + pub fn iter_mut(&mut self) -> IterMut { + assert!(!self.readonly); + self.map.iter_mut() + } + + #[inline(always)] + pub fn entry(&mut self, key: K) -> Entry { + assert!(!self.readonly); + self.map.entry(key) + } + + #[inline(always)] + pub fn try_entry(&mut self, key: K) -> Result, FailedAllocationError> { + assert!(!self.readonly); + self.map.try_entry(key) + } + + #[inline(always)] + pub fn insert(&mut self, k: K, v: V) -> Option { + assert!(!self.readonly); + self.map.insert(k, v) + } + + #[inline(always)] + pub fn try_insert(&mut self, k: K, v: V) -> Result, FailedAllocationError> { + assert!(!self.readonly); + self.map.try_insert(k, v) + } + + #[inline(always)] + pub fn remove(&mut self, k: &Q) -> Option + where K: Borrow, + Q: Hash + Eq + { + assert!(!self.readonly); + self.map.remove(k) + } + + #[inline(always)] + pub fn clear(&mut self) where K: 'static, V: 'static { + // We handle scoped mutations for the caller here, since callsites that + // invoke clear() don't benefit from the coalescing we do around insertion. + self.begin_mutation(); + self.map.clear(); + self.end_mutation(); + } +} + +impl ProtectedHashMap + where K: Eq + Hash, +{ + pub fn new() -> Self { + Self { + map: HashMap::new(), + readonly: true, + } + } + + pub fn with_capacity(capacity: usize) -> Self { + Self { + map: HashMap::with_capacity(capacity), + readonly: true, + } + } +} + +impl PartialEq for ProtectedHashMap + where K: Eq + Hash, + V: PartialEq, + S: BuildHasher +{ + fn eq(&self, other: &Self) -> bool { + self.map.eq(&other.map) + } +} + +impl Eq for ProtectedHashMap + where K: Eq + Hash, + V: Eq, + S: BuildHasher +{ +} + +impl Default for ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher + Default +{ + fn default() -> Self { + Self { + map: HashMap::default(), + readonly: true, + } + } +} diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 413942e4322..fc27de0320a 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -339,6 +339,25 @@ impl MallocSizeOf for hashglobe::hash_map::HashMap } } +impl MallocShallowSizeOf for hashglobe::protected::ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher +{ + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + self.inner().shallow_size_of(ops) + } +} + +impl MallocSizeOf for hashglobe::protected::ProtectedHashMap + where K: Eq + Hash + MallocSizeOf, + V: MallocSizeOf, + S: BuildHasher, +{ + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + self.inner().size_of(ops) + } +} + // XXX: we don't want MallocSizeOf to be defined for Rc and Arc. If negative // trait bounds are ever allowed, this code should be uncommented. // (We do have a compile-fail test for this: diff --git a/components/style/hash.rs b/components/style/hash.rs index 305357a85b2..89b7e1979a2 100644 --- a/components/style/hash.rs +++ b/components/style/hash.rs @@ -13,11 +13,16 @@ use fnv; pub use hashglobe::hash_map::HashMap; #[cfg(feature = "gecko")] pub use hashglobe::hash_set::HashSet; - +#[cfg(feature = "gecko")] +pub use hashglobe::protected::ProtectedHashMap; #[cfg(feature = "servo")] pub use hashglobe::fake::{HashMap, HashSet}; +/// Alias to use regular HashMaps everywhere in Servo. +#[cfg(feature = "servo")] +pub type ProtectedHashMap = HashMap; + /// Appropriate reexports of hash_map types pub mod map { #[cfg(feature = "gecko")] diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 301cd2e25a9..2531e0f8f23 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -299,6 +299,24 @@ impl InvalidationMap { Ok(()) } + + /// Allows mutation of this InvalidationMap. + #[cfg(feature = "gecko")] + pub fn begin_mutation(&mut self) { + self.class_to_selector.begin_mutation(); + self.id_to_selector.begin_mutation(); + self.state_affecting_selectors.begin_mutation(); + self.other_attribute_affecting_selectors.begin_mutation(); + } + + /// Disallows mutation of this InvalidationMap. + #[cfg(feature = "gecko")] + pub fn end_mutation(&mut self) { + self.class_to_selector.end_mutation(); + self.id_to_selector.end_mutation(); + self.state_affecting_selectors.end_mutation(); + self.other_attribute_affecting_selectors.end_mutation(); + } } /// A struct that collects invalidations for a given compound selector. diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 3b14431c06f..b151a36632b 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -10,7 +10,7 @@ use applicable_declarations::ApplicableDeclarationBlock; use context::QuirksMode; use dom::TElement; use fallible::FallibleVec; -use hash::{HashMap, HashSet}; +use hash::{HashMap, HashSet, ProtectedHashMap}; use hash::map as hash_map; use hashglobe::FailedAllocationError; use pdqsort::sort_by; @@ -38,6 +38,9 @@ impl Default for PrecomputedHasher { /// A simple alias for a hashmap using PrecomputedHasher. pub type PrecomputedHashMap = HashMap>; +/// A simple alias for a hashmap using PrecomputedHasher. +pub type PrecomputedProtectedHashMap = ProtectedHashMap>; + /// A simple alias for a hashset using PrecomputedHasher. pub type PrecomputedHashSet = HashSet>; @@ -102,7 +105,7 @@ pub struct SelectorMap { /// A hash from a class name to rules which contain that class selector. pub class_hash: MaybeCaseInsensitiveHashMap>, /// A hash from local name to rules which contain that local name selector. - pub local_name_hash: PrecomputedHashMap>, + pub local_name_hash: PrecomputedProtectedHashMap>, /// Rules that don't have ID, class, or element selectors. pub other: SmallVec<[T; 1]>, /// The number of entries in this map. @@ -123,7 +126,7 @@ impl SelectorMap { SelectorMap { id_hash: MaybeCaseInsensitiveHashMap::new(), class_hash: MaybeCaseInsensitiveHashMap::new(), - local_name_hash: HashMap::default(), + local_name_hash: ProtectedHashMap::default(), other: SmallVec::new(), count: 0, } @@ -147,6 +150,30 @@ impl SelectorMap { pub fn len(&self) -> usize { self.count } + + /// Allows mutation of this SelectorMap. + #[cfg(feature = "gecko")] + pub fn begin_mutation(&mut self) { + self.id_hash.begin_mutation(); + self.class_hash.begin_mutation(); + self.local_name_hash.begin_mutation(); + } + + /// Allows mutation of this SelectorMap. Not enforced in Servo. + #[cfg(feature = "servo")] + pub fn begin_mutation(&mut self) {} + + /// Disallows mutation of this SelectorMap. + #[cfg(feature = "gecko")] + pub fn end_mutation(&mut self) { + self.id_hash.end_mutation(); + self.class_hash.end_mutation(); + self.local_name_hash.end_mutation(); + } + + /// Disallows mutation of this SelectorMap. Not enforced in Servo. + #[cfg(feature = "servo")] + pub fn end_mutation(&mut self) {} } impl SelectorMap { @@ -463,7 +490,7 @@ fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> { #[derive(Debug)] #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -pub struct MaybeCaseInsensitiveHashMap(PrecomputedHashMap); +pub struct MaybeCaseInsensitiveHashMap(PrecomputedProtectedHashMap); // FIXME(Manishearth) the 'static bound can be removed when // our HashMap fork (hashglobe) is able to use NonZero, @@ -471,7 +498,7 @@ pub struct MaybeCaseInsensitiveHashMap MaybeCaseInsensitiveHashMap { /// Empty map pub fn new() -> Self { - MaybeCaseInsensitiveHashMap(PrecomputedHashMap::default()) + MaybeCaseInsensitiveHashMap(PrecomputedProtectedHashMap::default()) } /// HashMap::entry @@ -512,5 +539,17 @@ impl MaybeCaseInsensitiveHashMap { self.0.get(key) } } + + /// ProtectedHashMap::begin_mutation + #[cfg(feature = "gecko")] + pub fn begin_mutation(&mut self) { + self.0.begin_mutation(); + } + + /// ProtectedHashMap::end_mutation + #[cfg(feature = "gecko")] + pub fn end_mutation(&mut self) { + self.0.end_mutation(); + } } diff --git a/components/style/selector_parser.rs b/components/style/selector_parser.rs index f55161a9cc4..316f360daf8 100644 --- a/components/style/selector_parser.rs +++ b/components/style/selector_parser.rs @@ -159,6 +159,15 @@ impl PerPseudoElementMap { *self = Self::default(); } + /// Invokes a callback on each non-None entry. + pub fn for_each(&mut self, mut f: F) { + for entry in self.entries.iter_mut() { + if entry.is_some() { + f(entry.as_mut().unwrap()); + } + } + } + /// Set an entry value. /// /// Returns an error if the element is not a simple pseudo. diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 7bf735955b9..0b0b020cf19 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1870,6 +1870,32 @@ impl CascadeData { } } + #[cfg(feature = "gecko")] + fn begin_mutation(&mut self, rebuild_kind: &SheetRebuildKind) { + self.element_map.begin_mutation(); + self.pseudos_map.for_each(|m| m.begin_mutation()); + if rebuild_kind.should_rebuild_invalidation() { + self.invalidation_map.begin_mutation(); + self.selectors_for_cache_revalidation.begin_mutation(); + } + } + + #[cfg(feature = "servo")] + fn begin_mutation(&mut self, _: &SheetRebuildKind) {} + + #[cfg(feature = "gecko")] + fn end_mutation(&mut self, rebuild_kind: &SheetRebuildKind) { + self.element_map.end_mutation(); + self.pseudos_map.for_each(|m| m.end_mutation()); + if rebuild_kind.should_rebuild_invalidation() { + self.invalidation_map.end_mutation(); + self.selectors_for_cache_revalidation.end_mutation(); + } + } + + #[cfg(feature = "servo")] + fn end_mutation(&mut self, _: &SheetRebuildKind) {} + /// Collects all the applicable media query results into `results`. /// /// This duplicates part of the logic in `add_stylesheet`, which is @@ -1933,6 +1959,7 @@ impl CascadeData { self.effective_media_query_results.saw_effective(stylesheet); } + self.begin_mutation(&rebuild_kind); for rule in stylesheet.effective_rules(device, guard) { match *rule { CssRule::Style(ref locked) => { @@ -1969,8 +1996,11 @@ impl CascadeData { None => &mut self.element_map, Some(pseudo) => { self.pseudos_map - .get_or_insert_with(&pseudo.canonical(), || Box::new(SelectorMap::new())) - .expect("Unexpected tree pseudo-element?") + .get_or_insert_with(&pseudo.canonical(), || { + let mut map = Box::new(SelectorMap::new()); + map.begin_mutation(); + map + }).expect("Unexpected tree pseudo-element?") } }; @@ -2064,6 +2094,7 @@ impl CascadeData { _ => {} } } + self.end_mutation(&rebuild_kind); Ok(()) } From ef042899d25428fc675525ad2b3f81da86f61849 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 26 Sep 2017 20:57:20 -0700 Subject: [PATCH 3/5] Round hashglobe allocations up to the nearest page size. MozReview-Commit-ID: 34KFtcwCkBB --- Cargo.lock | 1 + components/hashglobe/src/lib.rs | 3 +++ components/hashglobe/src/table.rs | 18 +++++++++++++++++- ports/geckolib/Cargo.toml | 1 + ports/geckolib/glue.rs | 4 ++++ ports/geckolib/lib.rs | 1 + tests/unit/stylo/Cargo.toml | 1 + tests/unit/stylo/lib.rs | 1 + 8 files changed, 29 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 7e6808bbfcf..685e1b7f821 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3231,6 +3231,7 @@ dependencies = [ "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.3 (registry+https://github.com/rust-lang/crates.io-index)", "geckoservo 0.0.1", + "hashglobe 0.1.0", "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "malloc_size_of 0.0.1", diff --git a/components/hashglobe/src/lib.rs b/components/hashglobe/src/lib.rs index 8a2d070e0e2..cdb98ad2003 100644 --- a/components/hashglobe/src/lib.rs +++ b/components/hashglobe/src/lib.rs @@ -52,3 +52,6 @@ impl fmt::Display for FailedAllocationError { self.reason.fmt(f) } } + +// The size of memory pages on this system. Set when initializing geckolib. +pub static SYSTEM_PAGE_SIZE: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::ATOMIC_USIZE_INIT; diff --git a/components/hashglobe/src/table.rs b/components/hashglobe/src/table.rs index 45dcb2673fa..602cd2131b4 100644 --- a/components/hashglobe/src/table.rs +++ b/components/hashglobe/src/table.rs @@ -777,7 +777,7 @@ impl RawTable { // FORK NOTE: Uses alloc shim instead of Heap.alloc - let buffer = alloc(size, alignment); + let buffer = alloc(round_up_to_page_size(size), alignment); if buffer.is_null() { @@ -1201,3 +1201,19 @@ impl Drop for RawTable { } } } + +// Force all allocations to fill their pages for the duration of the mprotect +// experiment. +#[inline] +fn round_up_to_page_size(size: usize) -> usize { + let page_size = ::SYSTEM_PAGE_SIZE.load(::std::sync::atomic::Ordering::Relaxed); + debug_assert!(page_size != 0); + let mut result = size; + let remainder = size % page_size; + if remainder != 0 { + result += page_size - remainder; + } + debug_assert!(result % page_size == 0); + debug_assert!(result - size < page_size); + result +} diff --git a/ports/geckolib/Cargo.toml b/ports/geckolib/Cargo.toml index 4043e9beb56..4f6b6fa670f 100644 --- a/ports/geckolib/Cargo.toml +++ b/ports/geckolib/Cargo.toml @@ -17,6 +17,7 @@ gecko_debug = ["style/gecko_debug"] atomic_refcell = "0.1" cssparser = "0.21.1" env_logger = {version = "0.4", default-features = false} # disable `regex` to reduce code size +hashglobe = {path = "../../components/hashglobe"} libc = "0.2" log = {version = "0.3.5", features = ["release_max_level_info"]} malloc_size_of = {path = "../../components/malloc_size_of"} diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 69c039a9817..d79b2ca6496 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -181,6 +181,10 @@ pub extern "C" fn Servo_Initialize(dummy_url_data: *mut URLExtraData) { // Initialize the dummy url data unsafe { DUMMY_URL_DATA = dummy_url_data; } + + // Set the system page size. + let page_size = unsafe { bindings::Gecko_GetSystemPageSize() }; + ::hashglobe::SYSTEM_PAGE_SIZE.store(page_size, ::std::sync::atomic::Ordering::Relaxed); } #[no_mangle] diff --git a/ports/geckolib/lib.rs b/ports/geckolib/lib.rs index ca57307c245..7bfc0525059 100644 --- a/ports/geckolib/lib.rs +++ b/ports/geckolib/lib.rs @@ -6,6 +6,7 @@ extern crate cssparser; extern crate env_logger; +extern crate hashglobe; extern crate libc; #[macro_use] extern crate log; extern crate malloc_size_of; diff --git a/tests/unit/stylo/Cargo.toml b/tests/unit/stylo/Cargo.toml index 40248fb0420..bbe601c56b8 100644 --- a/tests/unit/stylo/Cargo.toml +++ b/tests/unit/stylo/Cargo.toml @@ -17,6 +17,7 @@ cssparser = "0.21.1" env_logger = "0.4" euclid = "0.15" geckoservo = {path = "../../../ports/geckolib"} +hashglobe = {path = "../../../components/hashglobe"} libc = "0.2" log = {version = "0.3.5", features = ["release_max_level_info"]} malloc_size_of = {path = "../../../components/malloc_size_of"} diff --git a/tests/unit/stylo/lib.rs b/tests/unit/stylo/lib.rs index 6b460558953..ac7c61f587c 100644 --- a/tests/unit/stylo/lib.rs +++ b/tests/unit/stylo/lib.rs @@ -6,6 +6,7 @@ extern crate atomic_refcell; extern crate cssparser; extern crate env_logger; extern crate geckoservo; +extern crate hashglobe; #[macro_use] extern crate log; extern crate malloc_size_of; extern crate selectors; From 039fe176b9bee0e78a685aab6f1d4ceb796b82d1 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 27 Sep 2017 17:01:34 -0700 Subject: [PATCH 4/5] Protect the hashmaps outside of rebuilds. MozReview-Commit-ID: KACmfw4pZY2 --- components/hashglobe/src/hash_map.rs | 6 +++ components/hashglobe/src/protected.rs | 59 ++++++++++++++++++++++++++- components/hashglobe/src/table.rs | 18 ++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/components/hashglobe/src/hash_map.rs b/components/hashglobe/src/hash_map.rs index 69bd06344e0..c1c4db4d33e 100644 --- a/components/hashglobe/src/hash_map.rs +++ b/components/hashglobe/src/hash_map.rs @@ -1027,6 +1027,12 @@ impl HashMap self.table.size() } + /// Access to the raw buffer backing this hashmap. + pub fn raw_buffer(&self) -> (*const (), usize) { + assert!(self.raw_capacity() != 0); + self.table.raw_buffer() + } + /// Returns true if the map contains no elements. /// /// # Examples diff --git a/components/hashglobe/src/protected.rs b/components/hashglobe/src/protected.rs index 99b0879c276..cd23aed78fe 100644 --- a/components/hashglobe/src/protected.rs +++ b/components/hashglobe/src/protected.rs @@ -25,12 +25,14 @@ impl ProtectedHashMap #[inline(always)] pub fn begin_mutation(&mut self) { assert!(self.readonly); + self.unprotect(); self.readonly = false; } #[inline(always)] pub fn end_mutation(&mut self) { assert!(!self.readonly); + self.protect(); self.readonly = true; } @@ -130,6 +132,36 @@ impl ProtectedHashMap self.map.clear(); self.end_mutation(); } + + fn protect(&mut self) { + if self.map.capacity() == 0 { + return; + } + let buff = self.map.raw_buffer(); + if buff.0 as usize % ::SYSTEM_PAGE_SIZE.load(::std::sync::atomic::Ordering::Relaxed) != 0 { + // Safely handle weird allocators like ASAN that return + // non-page-aligned buffers to page-sized allocations. + return; + } + unsafe { + Gecko_ProtectBuffer(buff.0 as *mut _, buff.1); + } + } + + fn unprotect(&mut self) { + if self.map.capacity() == 0 { + return; + } + let buff = self.map.raw_buffer(); + if buff.0 as usize % ::SYSTEM_PAGE_SIZE.load(::std::sync::atomic::Ordering::Relaxed) != 0 { + // Safely handle weird allocators like ASAN that return + // non-page-aligned buffers to page-sized allocations. + return; + } + unsafe { + Gecko_UnprotectBuffer(buff.0 as *mut _, buff.1); + } + } } impl ProtectedHashMap @@ -143,10 +175,12 @@ impl ProtectedHashMap } pub fn with_capacity(capacity: usize) -> Self { - Self { + let mut result = Self { map: HashMap::with_capacity(capacity), readonly: true, - } + }; + result.protect(); + result } } @@ -178,3 +212,24 @@ impl Default for ProtectedHashMap } } } + +impl Drop for ProtectedHashMap + where K: Eq + Hash, + S: BuildHasher +{ + fn drop(&mut self) { + debug_assert!(self.readonly, "Dropped while mutating"); + self.unprotect(); + } +} + +// Manually declare the FFI functions since we don't depend on the crate with +// the bindings. +extern "C" { + pub fn Gecko_ProtectBuffer(buffer: *mut ::std::os::raw::c_void, + size: usize); +} +extern "C" { + pub fn Gecko_UnprotectBuffer(buffer: *mut ::std::os::raw::c_void, + size: usize); +} diff --git a/components/hashglobe/src/table.rs b/components/hashglobe/src/table.rs index 602cd2131b4..23b2ea00f16 100644 --- a/components/hashglobe/src/table.rs +++ b/components/hashglobe/src/table.rs @@ -813,6 +813,24 @@ impl RawTable { } } + /// Access to the raw buffer backing this table. + pub fn raw_buffer(&self) -> (*const (), usize) { + debug_assert!(self.capacity() != 0); + + let buffer = self.hashes.ptr() as *const (); + let size = { + let hashes_size = self.capacity() * size_of::(); + let pairs_size = self.capacity() * size_of::<(K, V)>(); + let (_, _, size, _) = calculate_allocation(hashes_size, + align_of::(), + pairs_size, + align_of::<(K, V)>()); + round_up_to_page_size(size) + }; + (buffer, size) + } + + /// Creates a new raw table from a given capacity. All buckets are /// initially empty. pub fn new(capacity: usize) -> Result, FailedAllocationError> { From abd408433f1e95423bcf536d789bb29cda4e012c Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 27 Sep 2017 23:20:47 -0700 Subject: [PATCH 5/5] Add a testing API. This will allow us to verify the entire detection pipeline in real nightly builds, which will give us confidence that real heap corruption will be detected and reported properly. MozReview-Commit-ID: 43Fp2HT8RYy --- components/style/stylist.rs | 11 +++++++++++ ports/geckolib/glue.rs | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 0b0b020cf19..47dfcad1aa5 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1508,6 +1508,17 @@ impl Stylist { pub fn shutdown() { UA_CASCADE_DATA_CACHE.lock().unwrap().clear() } + + /// Temporary testing method. See bug 1403397. + pub fn corrupt_rule_hash_and_crash(&self, index: usize) { + let mut origin_iter = self.cascade_data.iter_origins(); + let d = origin_iter.next().unwrap().0; + let mut it = d.element_map.local_name_hash.iter(); + let nth = index % it.len(); + let entry = it.nth(nth).unwrap(); + let ptr = entry.0 as *const _ as *const usize as *mut usize; + unsafe { *ptr = 0; } + } } /// This struct holds data which users of Stylist may want to extract diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index d79b2ca6496..432ed4a73c6 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -4046,3 +4046,9 @@ pub extern "C" fn Servo_HasPendingRestyleAncestor(element: RawGeckoElementBorrow } false } + +#[no_mangle] +pub extern "C" fn Servo_CorruptRuleHashAndCrash(set: RawServoStyleSetBorrowed, index: usize) { + let per_doc_data = PerDocumentStyleData::from_ffi(set).borrow(); + per_doc_data.stylist.corrupt_rule_hash_and_crash(index); +}