From cff25e4147d46e4adbe5c032a36f197f6eaf38ea Mon Sep 17 00:00:00 2001 From: Stiopa Koltsov Date: Wed, 6 Sep 2023 13:19:49 -0700 Subject: [PATCH] Fix the issue with TypeId alignment on new Rust Apple Silicon Summary: * `TypeId` is `u128` [in Rust 1.72](https://github.com/rust-lang/rust/pull/109953/) * `u128` is 16 bytes aligned on Apple Silicon * we require 8 byte alignment for `StarlarkValue` Maybe we should fix heap allocations: insert padding to allow any alignment. But practically we don't need it and should not do it: generated machine code is identical for aligned and 8-byte aligned `u128`: [compiler explorer](https://rust.godbolt.org/z/96KsnjW77) (note it is identical if we load it from memory not pass by value, but this is what we do). So for now just force 8 byte alignment on `TypeId`. Fixes issue https://github.com/facebook/buck2/issues/408 Reviewed By: ndmitchell, shayne-fletcher Differential Revision: D49019964 fbshipit-source-id: 62c8d7db83e9e4bdbe58fe7a62d5daae41996e61 --- .../starlark/src/values/starlark_type_id.rs | 30 +++++++++++++++++++ .../values/typing/type_compiled/matchers.rs | 7 +++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/starlark-rust/starlark/src/values/starlark_type_id.rs b/starlark-rust/starlark/src/values/starlark_type_id.rs index abc441057426..4ba5c7c170f6 100644 --- a/starlark-rust/starlark/src/values/starlark_type_id.rs +++ b/starlark-rust/starlark/src/values/starlark_type_id.rs @@ -47,3 +47,33 @@ impl StarlarkTypeId { StarlarkTypeId(ConstTypeId::of::()) } } + +/// We require alignment 8 for `StarlarkValue`. +/// `TypeId` is 16 bytes aligned on Rust 1.72 on Apple Silicon. +/// Use this struct to put `ConstTypeId` in a `StarlarkValue`. +// TODO(nga): remove alignment requirement from `Heap`. +#[repr(packed(8))] +#[derive(Allocative, Eq, Clone, Copy, Dupe, Debug)] +#[allocative(skip)] // There are no heap allocations in this struct. +pub(crate) struct StarlarkTypeIdAligned { + starlark_type_id: StarlarkTypeId, +} + +impl PartialEq for StarlarkTypeIdAligned { + #[inline] + fn eq(&self, other: &Self) -> bool { + self.get() == other.get() + } +} + +impl StarlarkTypeIdAligned { + #[inline] + pub(crate) const fn new(starlark_type_id: StarlarkTypeId) -> StarlarkTypeIdAligned { + StarlarkTypeIdAligned { starlark_type_id } + } + + #[inline] + pub(crate) const fn get(&self) -> StarlarkTypeId { + self.starlark_type_id + } +} diff --git a/starlark-rust/starlark/src/values/typing/type_compiled/matchers.rs b/starlark-rust/starlark/src/values/typing/type_compiled/matchers.rs index 7288ef490216..7a8ea5712208 100644 --- a/starlark-rust/starlark/src/values/typing/type_compiled/matchers.rs +++ b/starlark-rust/starlark/src/values/typing/type_compiled/matchers.rs @@ -27,6 +27,7 @@ use crate::values::dict::DictRef; use crate::values::list::value::FrozenList; use crate::values::list::ListRef; use crate::values::starlark_type_id::StarlarkTypeId; +use crate::values::starlark_type_id::StarlarkTypeIdAligned; use crate::values::tuple::value::Tuple; use crate::values::types::int_or_big::StarlarkIntRef; use crate::values::typing::type_compiled::matcher::TypeMatcher; @@ -245,20 +246,20 @@ impl TypeMatcher for IsNone { #[derive(Allocative, Debug, Clone)] pub(crate) struct StarlarkTypeIdMatcher { - starlark_type_id: StarlarkTypeId, + starlark_type_id: StarlarkTypeIdAligned, } impl StarlarkTypeIdMatcher { pub(crate) fn new(ty: TyStarlarkValue) -> StarlarkTypeIdMatcher { StarlarkTypeIdMatcher { - starlark_type_id: ty.starlark_type_id(), + starlark_type_id: StarlarkTypeIdAligned::new(ty.starlark_type_id()), } } } impl TypeMatcher for StarlarkTypeIdMatcher { fn matches(&self, value: Value) -> bool { - value.starlark_type_id() == self.starlark_type_id + value.starlark_type_id() == self.starlark_type_id.get() } }