Skip to content

Commit

Permalink
Auto merge of #18870 - servo:cast, r=KiChjang
Browse files Browse the repository at this point in the history
Use pointer casts instead of tramsutes to raw::TraitObject

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.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18870)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Oct 14, 2017
2 parents f0b8dad + b505c9e commit 86a5135
Show file tree
Hide file tree
Showing 16 changed files with 74 additions and 57 deletions.
4 changes: 4 additions & 0 deletions components/layout/block.rs
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions components/layout/flex.rs
Expand Up @@ -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,
Expand Down
38 changes: 21 additions & 17 deletions components/layout/flow.rs
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -451,11 +459,10 @@ 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)
}
pub fn base<T: ?Sized + HasBaseFlow>(this: &T) -> &BaseFlow {
let ptr: *const T = this;
let ptr = ptr as *const BaseFlow;
unsafe { &*ptr }
}

/// Iterates over the children of this immutable flow.
Expand All @@ -465,11 +472,10 @@ 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)
}
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 }
}

/// Iterates over the children of this flow.
Expand Down Expand Up @@ -1419,11 +1425,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)
}
}
4 changes: 4 additions & 0 deletions components/layout/inline.rs
Expand Up @@ -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,
Expand Down
9 changes: 0 additions & 9 deletions components/layout/lib.rs
Expand Up @@ -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 (),
}
4 changes: 4 additions & 0 deletions components/layout/list_item.rs
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions components/layout/multicol.rs
Expand Up @@ -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,

Expand All @@ -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,
}
Expand Down
4 changes: 4 additions & 0 deletions components/layout/table.rs
Expand Up @@ -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,

Expand Down
4 changes: 4 additions & 0 deletions components/layout/table_caption.rs
Expand Up @@ -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,
}
Expand Down
4 changes: 4 additions & 0 deletions components/layout/table_cell.rs
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions components/layout/table_colgroup.rs
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions components/layout/table_row.rs
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions components/layout/table_rowgroup.rs
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions components/layout/table_wrapper.rs
Expand Up @@ -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,

Expand Down
3 changes: 1 addition & 2 deletions components/script/dom/element.rs
Expand Up @@ -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;
Expand Down
29 changes: 0 additions & 29 deletions tests/unit/layout/lib.rs
Expand Up @@ -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));
}

0 comments on commit 86a5135

Please sign in to comment.