Bug 1549969 - Fix an assertion that doesn't account for alignment padding. r=bholley
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 08 May 2019 18:03:37 +0000
changeset 531919 0ba7b9f6fdafcd2a2ba6a13706a8d419d7d64f5b
parent 531918 82ae6ce8f3a8e18ab661b8d85fc96649a28694b5
child 531920 e8dbcc5c516f05fe34f90efaf6fd199118979012
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1549969
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1549969 - Fix an assertion that doesn't account for alignment padding. r=bholley I'm hitting this when trying to add bindings for box shadows, which are 32-bit aligned. Depends on D30352 Differential Revision: https://phabricator.services.mozilla.com/D30353
servo/components/servo_arc/lib.rs
--- a/servo/components/servo_arc/lib.rs
+++ b/servo/components/servo_arc/lib.rs
@@ -597,17 +597,17 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
     /// then `alloc` must return an allocation that can be dellocated
     /// by calling Box::from_raw::<ArcInner<HeaderSlice<H, T>>> on it.
     #[inline]
     fn from_header_and_iter_alloc<F, I>(alloc: F, header: H, mut items: I, is_static: bool) -> Self
     where
         F: FnOnce(Layout) -> *mut u8,
         I: Iterator<Item = T> + ExactSizeIterator,
     {
-        use std::mem::size_of;
+        use std::mem::{align_of, size_of};
         assert_ne!(size_of::<T>(), 0, "Need to think about ZST");
 
         // Compute the required size for the allocation.
         let num_items = items.len();
         let size = {
             // First, determine the alignment of a hypothetical pointer to a
             // HeaderSlice.
             let fake_slice_ptr_align: usize = mem::align_of::<ArcInner<HeaderSlice<H, [T; 0]>>>();
@@ -673,18 +673,19 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
                     ptr::write(
                         current,
                         items
                             .next()
                             .expect("ExactSizeIterator over-reported length"),
                     );
                     current = current.offset(1);
                 }
-                // We should have consumed the buffer exactly.
-                debug_assert_eq!(current as *mut u8, buffer.offset(size as isize));
+                // We should have consumed the buffer exactly, maybe accounting
+                // for some padding from the alignment.
+                debug_assert!((buffer.offset(size as isize) as usize - current as *mut u8 as usize) < align_of::<Self>());
             }
             assert!(
                 items.next().is_none(),
                 "ExactSizeIterator under-reported length"
             );
         }
 
         // Return the fat Arc.
@@ -1330,16 +1331,33 @@ mod tests {
         let y = Arc::into_thin(x.clone());
         assert_eq!(y.header.header, 100);
         assert!(y.slice.is_empty());
         assert_eq!(x.header.header, 100);
         assert!(x.slice.is_empty());
     }
 
     #[test]
+    fn thin_assert_padding() {
+        #[derive(Clone, Default)]
+        #[repr(C)]
+        struct Padded {
+            i: u16,
+        }
+
+        // The header will have more alignment than `Padded`
+        let header = HeaderWithLength::new(0i32, 2);
+        let items = vec![Padded { i: 0xdead }, Padded { i: 0xbeef }];
+        let a = ThinArc::from_header_and_iter(header, items.into_iter());
+        assert_eq!(a.slice.len(), 2);
+        assert_eq!(a.slice[0].i, 0xdead);
+        assert_eq!(a.slice[1].i, 0xbeef);
+    }
+
+    #[test]
     fn slices_and_thin() {
         let mut canary = atomic::AtomicUsize::new(0);
         let c = Canary(&mut canary as *mut atomic::AtomicUsize);
         let v = vec![5, 6];
         let header = HeaderWithLength::new(c, v.len());
         {
             let x = Arc::into_thin(Arc::from_header_and_iter(header, v.into_iter()));
             let y = ThinArc::with_arc(&x, |q| q.clone());