From 09143e627b602e34288876d48fc6ef60d5482b21 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 18 Sep 2017 13:37:53 +1000 Subject: [PATCH] Fix a panic in Stylo memory reporting. `MallocSizeOfOps::enclosing_size_of_op` is an `Option<>` type, and the panic in question is caused by not providing a value in a case where it's needed for measuring a HashSet. HashMaps and HashSets are common enough that it makes sense to make `enclosing_size_of_op` non-optional, which this patch does. --- components/malloc_size_of/lib.rs | 11 ++++------- components/style/gecko/generated/bindings.rs | 7 ++++++- ports/geckolib/glue.rs | 13 ++++++++++--- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 671e4b9e1392..0a141587517f 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -82,9 +82,8 @@ pub struct MallocSizeOfOps { /// A function that returns the size of a heap allocation. size_of_op: VoidPtrToSizeFn, - /// Like `size_of_op`, but can take an interior pointer. Optional, because - /// many places don't need it. - enclosing_size_of_op: Option, + /// Like `size_of_op`, but can take an interior pointer. + enclosing_size_of_op: VoidPtrToSizeFn, /// Check if a pointer has been seen before, and remember it for next time. /// Useful when measuring `Rc`s and `Arc`s. Optional, because many places @@ -93,8 +92,7 @@ pub struct MallocSizeOfOps { } impl MallocSizeOfOps { - pub fn new(size_of: VoidPtrToSizeFn, - malloc_enclosing_size_of: Option, + pub fn new(size_of: VoidPtrToSizeFn, malloc_enclosing_size_of: VoidPtrToSizeFn, have_seen_ptr: Option>) -> Self { MallocSizeOfOps { size_of_op: size_of, @@ -129,8 +127,7 @@ impl MallocSizeOfOps { /// Call `enclosing_size_of_op` on `ptr`, which must not be empty. pub unsafe fn malloc_enclosing_size_of(&self, ptr: *const T) -> usize { assert!(!MallocSizeOfOps::is_empty(ptr)); - let enclosing_size_of_op = self.enclosing_size_of_op.expect("missing enclosing_size_of_op"); - enclosing_size_of_op(ptr as *const c_void) + (self.enclosing_size_of_op)(ptr as *const c_void) } /// Call `have_seen_ptr_op` on `ptr`. diff --git a/components/style/gecko/generated/bindings.rs b/components/style/gecko/generated/bindings.rs index 7ea765215584..b5f03ce8b493 100644 --- a/components/style/gecko/generated/bindings.rs +++ b/components/style/gecko/generated/bindings.rs @@ -1898,7 +1898,10 @@ extern "C" { pub fn Servo_Element_ClearData(node: RawGeckoElementBorrowed); } extern "C" { - pub fn Servo_Element_SizeOfExcludingThisAndCVs(arg1: MallocSizeOf, + pub fn Servo_Element_SizeOfExcludingThisAndCVs(malloc_size_of: + MallocSizeOf, + malloc_enclosing_size_of: + MallocSizeOf, seen_ptrs: *mut SeenPtrs, node: RawGeckoElementBorrowed) @@ -1964,6 +1967,8 @@ extern "C" { } extern "C" { pub fn Servo_StyleSheet_SizeOfIncludingThis(malloc_size_of: MallocSizeOf, + malloc_enclosing_size_of: + MallocSizeOf, sheet: RawServoStyleSheetContentsBorrowed) -> usize; diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 049f7496df63..9457b7b83e8e 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -783,13 +783,16 @@ pub extern "C" fn Servo_Element_ClearData(element: RawGeckoElementBorrowed) { #[no_mangle] pub extern "C" fn Servo_Element_SizeOfExcludingThisAndCVs(malloc_size_of: GeckoMallocSizeOf, + malloc_enclosing_size_of: + GeckoMallocSizeOf, seen_ptrs: *mut SeenPtrs, element: RawGeckoElementBorrowed) -> usize { let element = GeckoElement(element); let borrow = element.borrow_data(); if let Some(data) = borrow { let have_seen_ptr = move |ptr| { unsafe { Gecko_HaveSeenPtr(seen_ptrs, ptr) } }; - let mut ops = MallocSizeOfOps::new(malloc_size_of.unwrap(), None, + let mut ops = MallocSizeOfOps::new(malloc_size_of.unwrap(), + malloc_enclosing_size_of.unwrap(), Some(Box::new(have_seen_ptr))); (*data).size_of_excluding_cvs(&mut ops) } else { @@ -1089,11 +1092,14 @@ pub extern "C" fn Servo_StyleSheet_Clone( #[no_mangle] pub extern "C" fn Servo_StyleSheet_SizeOfIncludingThis( malloc_size_of: GeckoMallocSizeOf, + malloc_enclosing_size_of: GeckoMallocSizeOf, sheet: RawServoStyleSheetContentsBorrowed ) -> usize { let global_style_data = &*GLOBAL_STYLE_DATA; let guard = global_style_data.shared_lock.read(); - let mut ops = MallocSizeOfOps::new(malloc_size_of.unwrap(), None, None); + let mut ops = MallocSizeOfOps::new(malloc_size_of.unwrap(), + malloc_enclosing_size_of.unwrap(), + None); StylesheetContents::as_arc(&sheet).size_of(&guard, &mut ops) } @@ -3753,7 +3759,8 @@ pub extern "C" fn Servo_StyleSet_AddSizeOfExcludingThis( ) { let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let mut ops = MallocSizeOfOps::new(malloc_size_of.unwrap(), - malloc_enclosing_size_of, None); + malloc_enclosing_size_of.unwrap(), + None); let sizes = unsafe { sizes.as_mut() }.unwrap(); data.add_size_of_children(&mut ops, sizes); }