From e7c53d4ca22cbc51bdd2657a26575f306df97cb9 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Fri, 13 Oct 2017 21:55:11 +0200
Subject: [PATCH 1/3] Use pointer casts instead of tramsutes to
 raw::TraitObject
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Casting `*const T` to `*const U` with `U: Sized` is allowed even if `T: ?Sized`.
This safely extracts the data pointer out of a trait object,
without relying on the memory representation of trait objects.
---
 components/layout/flow.rs | 24 ++++++++++--------------
 components/layout/lib.rs  |  9 ---------
 tests/unit/layout/lib.rs  | 29 -----------------------------
 3 files changed, 10 insertions(+), 52 deletions(-)

diff --git a/components/layout/flow.rs b/components/layout/flow.rs
index fbcf78669316..8dad21045231 100644
--- a/components/layout/flow.rs
+++ b/components/layout/flow.rs
@@ -44,7 +44,7 @@ use multicol::MulticolFlow;
 use parallel::FlowParallelInfo;
 use serde::ser::{Serialize, SerializeStruct, Serializer};
 use servo_geometry::{au_rect_to_f32_rect, f32_rect_to_au_rect, max_rect};
-use std::{fmt, mem};
+use std::fmt;
 use std::iter::Zip;
 use std::slice::IterMut;
 use std::sync::Arc;
@@ -452,10 +452,9 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static {
 #[inline(always)]
 #[allow(unsafe_code)]
 pub fn base<T: ?Sized + Flow>(this: &T) -> &BaseFlow {
-    unsafe {
-        let obj = mem::transmute::<&&T, &::TraitObject>(&this);
-        mem::transmute::<*mut (), &BaseFlow>(obj.data)
-    }
+    let ptr: *const T = this;
+    let ptr = ptr as *const BaseFlow;
+    unsafe { &*ptr }
 }
 
 /// Iterates over the children of this immutable flow.
@@ -466,10 +465,9 @@ pub fn child_iter<'a>(flow: &'a Flow) -> FlowListIterator {
 #[inline(always)]
 #[allow(unsafe_code)]
 pub fn mut_base<T: ?Sized + Flow>(this: &mut T) -> &mut BaseFlow {
-    unsafe {
-        let obj = mem::transmute::<&&mut T, &::TraitObject>(&this);
-        mem::transmute::<*mut (), &mut BaseFlow>(obj.data)
-    }
+    let ptr: *mut T = this;
+    let ptr = ptr as *mut BaseFlow;
+    unsafe { &mut *ptr }
 }
 
 /// Iterates over the children of this flow.
@@ -1419,11 +1417,9 @@ impl ContainingBlockLink {
 pub struct OpaqueFlow(pub usize);
 
 impl OpaqueFlow {
-    #[allow(unsafe_code)]
     pub fn from_flow(flow: &Flow) -> OpaqueFlow {
-        unsafe {
-            let object = mem::transmute::<&Flow, ::TraitObject>(flow);
-            OpaqueFlow(object.data as usize)
-        }
+        let object_ptr: *const Flow = flow;
+        let data_ptr = object_ptr as *const ();
+        OpaqueFlow(data_ptr as usize)
     }
 }
diff --git a/components/layout/lib.rs b/components/layout/lib.rs
index 1e48a2b5a6d4..f9c3cbeda2bb 100644
--- a/components/layout/lib.rs
+++ b/components/layout/lib.rs
@@ -90,12 +90,3 @@ pub use self::data::LayoutData;
 // We can't use servo_arc for everything in layout, because the Flow stuff uses
 // weak references.
 use servo_arc::Arc as ServoArc;
-
-/// Stable copy of std::raw::TraitObject
-/// test/unit/layout/lib.rs asserts that the memory layout matches.
-#[repr(C)]
-#[derive(Clone, Copy)]
-pub struct TraitObject {
-    pub data: *mut (),
-    pub vtable: *mut (),
-}
diff --git a/tests/unit/layout/lib.rs b/tests/unit/layout/lib.rs
index bc1d9dfd464d..a763135260a5 100644
--- a/tests/unit/layout/lib.rs
+++ b/tests/unit/layout/lib.rs
@@ -2,36 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#![feature(raw)]
-
 extern crate layout;
 #[macro_use] extern crate size_of_test;
 
 #[cfg(all(test, target_pointer_width = "64"))] mod size_of;
-
-use std::mem;
-use std::ptr;
-use std::raw;
-
-#[test]
-fn test_trait_object_layout() {
-    assert_eq!(mem::size_of::<raw::TraitObject>(), mem::size_of::<layout::TraitObject>());
-    let null: *mut () = ptr::null_mut();
-    let a = raw::TraitObject {
-        data: null,
-        vtable: null,
-    };
-    let b = layout::TraitObject {
-        data: null,
-        vtable: null,
-    };
-
-    fn offset<T, U>(struct_: &T, field: &U) -> usize {
-        let addr_struct = struct_ as *const T as usize;
-        let addr_field = field as *const U as usize;
-        addr_field - addr_struct
-    }
-
-    assert_eq!(offset(&a, &a.data), offset(&b, &b.data));
-    assert_eq!(offset(&a, &a.vtable), offset(&b, &b.vtable));
-}

From 4c5d6fd8345f301be4aafe2dba71cdf2de0ab01e Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Fri, 13 Oct 2017 21:57:26 +0200
Subject: [PATCH 2/3] Remove an unused import, fix a warning.

---
 components/script/dom/element.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index ee5682b561a5..f6e5357df10f 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -89,9 +89,8 @@ use script_layout_interface::message::ReflowGoal;
 use script_thread::ScriptThread;
 use selectors::Element as SelectorsElement;
 use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity};
-use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext, MatchingMode};
+use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext, RelevantLinkStatus};
 use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS};
-use selectors::matching::{RelevantLinkStatus, matches_selector_list};
 use selectors::sink::Push;
 use servo_arc::Arc;
 use servo_atoms::Atom;

From b505c9e94817847e19f22854b2ee4258f66f18d2 Mon Sep 17 00:00:00 2001
From: Simon Sapin <simon.sapin@exyr.org>
Date: Sat, 14 Oct 2017 00:03:57 +0200
Subject: [PATCH 3/3] Introduce an unsafe HasBaseFlow trait for
 base()/base_mut() casts.

---
 components/layout/block.rs          |  4 ++++
 components/layout/flex.rs           |  4 ++++
 components/layout/flow.rs           | 14 +++++++++++---
 components/layout/inline.rs         |  4 ++++
 components/layout/list_item.rs      |  4 ++++
 components/layout/multicol.rs       |  8 ++++++++
 components/layout/table.rs          |  4 ++++
 components/layout/table_caption.rs  |  4 ++++
 components/layout/table_cell.rs     |  4 ++++
 components/layout/table_colgroup.rs |  4 ++++
 components/layout/table_row.rs      |  4 ++++
 components/layout/table_rowgroup.rs |  4 ++++
 components/layout/table_wrapper.rs  |  4 ++++
 13 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/components/layout/block.rs b/components/layout/block.rs
index c7f675293d3e..bffcbef0e67e 100644
--- a/components/layout/block.rs
+++ b/components/layout/block.rs
@@ -492,8 +492,12 @@ pub enum FormattingContextType {
     Other,
 }
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for BlockFlow {}
+
 // A block formatting context.
 #[derive(Serialize)]
+#[repr(C)]
 pub struct BlockFlow {
     /// Data common to all flows.
     pub base: BaseFlow,
diff --git a/components/layout/flex.rs b/components/layout/flex.rs
index 063add9e1d56..6d969ef8643f 100644
--- a/components/layout/flex.rs
+++ b/components/layout/flex.rs
@@ -328,8 +328,12 @@ impl FlexLine {
     }
 }
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for FlexFlow {}
+
 /// A block with the CSS `display` property equal to `flex`.
 #[derive(Debug, Serialize)]
+#[repr(C)]
 pub struct FlexFlow {
     /// Data common to all block flows.
     block_flow: BlockFlow,
diff --git a/components/layout/flow.rs b/components/layout/flow.rs
index 8dad21045231..990e29e0238e 100644
--- a/components/layout/flow.rs
+++ b/components/layout/flow.rs
@@ -65,11 +65,19 @@ use table_rowgroup::TableRowGroupFlow;
 use table_wrapper::TableWrapperFlow;
 use webrender_api::ClipAndScrollInfo;
 
+/// This marker trait indicates that a type is a struct with `#[repr(C)]` whose first field
+/// is of type `BaseFlow` or some type that also implements this trait.
+///
+/// In other words, the memory representation of `BaseFlow` must be a prefix
+/// of the memory representation of types implementing `HasBaseFlow`.
+#[allow(unsafe_code)]
+pub unsafe trait HasBaseFlow {}
+
 /// Virtual methods that make up a float context.
 ///
 /// Note that virtual methods have a cost; we should not overuse them in Servo. Consider adding
 /// methods to `ImmutableFlowUtils` or `MutableFlowUtils` before adding more methods here.
-pub trait Flow: fmt::Debug + Sync + Send + 'static {
+pub trait Flow: HasBaseFlow + fmt::Debug + Sync + Send + 'static {
     // RTTI
     //
     // TODO(pcwalton): Use Rust's RTTI, once that works.
@@ -451,7 +459,7 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static {
 
 #[inline(always)]
 #[allow(unsafe_code)]
-pub fn base<T: ?Sized + Flow>(this: &T) -> &BaseFlow {
+pub fn base<T: ?Sized + HasBaseFlow>(this: &T) -> &BaseFlow {
     let ptr: *const T = this;
     let ptr = ptr as *const BaseFlow;
     unsafe { &*ptr }
@@ -464,7 +472,7 @@ pub fn child_iter<'a>(flow: &'a Flow) -> FlowListIterator {
 
 #[inline(always)]
 #[allow(unsafe_code)]
-pub fn mut_base<T: ?Sized + Flow>(this: &mut T) -> &mut BaseFlow {
+pub fn mut_base<T: ?Sized + HasBaseFlow>(this: &mut T) -> &mut BaseFlow {
     let ptr: *mut T = this;
     let ptr = ptr as *mut BaseFlow;
     unsafe { &mut *ptr }
diff --git a/components/layout/inline.rs b/components/layout/inline.rs
index e04ede22470b..e6f23afae8f2 100644
--- a/components/layout/inline.rs
+++ b/components/layout/inline.rs
@@ -861,8 +861,12 @@ impl InlineFragments {
     }
 }
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for InlineFlow {}
+
 /// Flows for inline layout.
 #[derive(Serialize)]
+#[repr(C)]
 pub struct InlineFlow {
     /// Data common to all flows.
     pub base: BaseFlow,
diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs
index 969d82b7fd7e..270d15b3771f 100644
--- a/components/layout/list_item.rs
+++ b/components/layout/list_item.rs
@@ -24,8 +24,12 @@ use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
 use style::servo::restyle_damage::RESOLVE_GENERATED_CONTENT;
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for ListItemFlow {}
+
 /// A block with the CSS `display` property equal to `list-item`.
 #[derive(Debug)]
+#[repr(C)]
 pub struct ListItemFlow {
     /// Data common to all block flows.
     pub block_flow: BlockFlow,
diff --git a/components/layout/multicol.rs b/components/layout/multicol.rs
index 2cb181f1f97f..7b811e387723 100644
--- a/components/layout/multicol.rs
+++ b/components/layout/multicol.rs
@@ -24,6 +24,10 @@ use style::properties::ComputedValues;
 use style::values::Either;
 use style::values::computed::{LengthOrPercentageOrAuto, LengthOrPercentageOrNone};
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for MulticolFlow {}
+
+#[repr(C)]
 pub struct MulticolFlow {
     pub block_flow: BlockFlow,
 
@@ -32,6 +36,10 @@ pub struct MulticolFlow {
     pub column_pitch: Au,
 }
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for MulticolColumnFlow {}
+
+#[repr(C)]
 pub struct MulticolColumnFlow {
     pub block_flow: BlockFlow,
 }
diff --git a/components/layout/table.rs b/components/layout/table.rs
index a0c7c69ce851..7009e4ec60bd 100644
--- a/components/layout/table.rs
+++ b/components/layout/table.rs
@@ -34,10 +34,14 @@ use table_row::{self, CellIntrinsicInlineSize, CollapsedBorder, CollapsedBorderP
 use table_row::TableRowFlow;
 use table_wrapper::TableLayout;
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for TableFlow {}
+
 /// A table flow corresponded to the table's internal table fragment under a table wrapper flow.
 /// The properties `position`, `float`, and `margin-*` are used on the table wrapper fragment,
 /// not table fragment per CSS 2.1 § 10.5.
 #[derive(Serialize)]
+#[repr(C)]
 pub struct TableFlow {
     pub block_flow: BlockFlow,
 
diff --git a/components/layout/table_caption.rs b/components/layout/table_caption.rs
index ecdd52784ee7..c6425e93b4e9 100644
--- a/components/layout/table_caption.rs
+++ b/components/layout/table_caption.rs
@@ -19,7 +19,11 @@ use std::fmt;
 use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for TableCaptionFlow {}
+
 /// A table formatting context.
+#[repr(C)]
 pub struct TableCaptionFlow {
     pub block_flow: BlockFlow,
 }
diff --git a/components/layout/table_cell.rs b/components/layout/table_cell.rs
index 1c04b3279042..c6c438204cfb 100644
--- a/components/layout/table_cell.rs
+++ b/components/layout/table_cell.rs
@@ -28,8 +28,12 @@ use style::values::generics::box_::VerticalAlign;
 use table::InternalTable;
 use table_row::{CollapsedBorder, CollapsedBorderProvenance};
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for TableCellFlow {}
+
 /// A table formatting context.
 #[derive(Serialize)]
+#[repr(C)]
 pub struct TableCellFlow {
     /// Data common to all block flows.
     pub block_flow: BlockFlow,
diff --git a/components/layout/table_colgroup.rs b/components/layout/table_colgroup.rs
index 4d54afbe3869..4b6af989e255 100644
--- a/components/layout/table_colgroup.rs
+++ b/components/layout/table_colgroup.rs
@@ -19,7 +19,11 @@ use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
 use style::values::computed::LengthOrPercentageOrAuto;
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for TableColGroupFlow {}
+
 /// A table formatting context.
+#[repr(C)]
 pub struct TableColGroupFlow {
     /// Data common to all flows.
     pub base: BaseFlow,
diff --git a/components/layout/table_row.rs b/components/layout/table_row.rs
index 5a835d0ded23..761a363881a2 100644
--- a/components/layout/table_row.rs
+++ b/components/layout/table_row.rs
@@ -31,7 +31,11 @@ use style::values::computed::{Color, LengthOrPercentageOrAuto};
 use table::{ColumnComputedInlineSize, ColumnIntrinsicInlineSize, InternalTable, VecExt};
 use table_cell::{CollapsedBordersForCell, TableCellFlow};
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for TableRowFlow {}
+
 /// A single row of a table.
+#[repr(C)]
 pub struct TableRowFlow {
     /// Fields common to all block flows.
     pub block_flow: BlockFlow,
diff --git a/components/layout/table_rowgroup.rs b/components/layout/table_rowgroup.rs
index 6877ba8b781d..0639f02dbf5d 100644
--- a/components/layout/table_rowgroup.rs
+++ b/components/layout/table_rowgroup.rs
@@ -24,7 +24,11 @@ use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
 use table::{ColumnIntrinsicInlineSize, InternalTable, TableLikeFlow};
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for TableRowGroupFlow {}
+
 /// A table formatting context.
+#[repr(C)]
 pub struct TableRowGroupFlow {
     /// Fields common to all block flows.
     pub block_flow: BlockFlow,
diff --git a/components/layout/table_wrapper.rs b/components/layout/table_wrapper.rs
index 972f52990941..866916b9ad55 100644
--- a/components/layout/table_wrapper.rs
+++ b/components/layout/table_wrapper.rs
@@ -43,8 +43,12 @@ pub enum TableLayout {
     Auto
 }
 
+#[allow(unsafe_code)]
+unsafe impl ::flow::HasBaseFlow for TableWrapperFlow {}
+
 /// A table wrapper flow based on a block formatting context.
 #[derive(Serialize)]
+#[repr(C)]
 pub struct TableWrapperFlow {
     pub block_flow: BlockFlow,