diff options
author | Philip Herron <herron.philip@googlemail.com> | 2023-06-29 10:13:01 +0100 |
---|---|---|
committer | Philip Herron <philip.herron@embecosm.com> | 2023-06-29 15:38:44 +0000 |
commit | 7af86eaaffec8cab5ad80e242480fe9f6c1ffaa6 (patch) | |
tree | d27ca036110d5a62c0f0f08990e3eeba9e6da87a | |
parent | eb88e324cde1e832dc6f3a5932aef9ee8e7abf6a (diff) | |
download | gcc-7af86eaaffec8cab5ad80e242480fe9f6c1ffaa6.zip gcc-7af86eaaffec8cab5ad80e242480fe9f6c1ffaa6.tar.gz gcc-7af86eaaffec8cab5ad80e242480fe9f6c1ffaa6.tar.bz2 |
gccrs: fix how we handle coercions and casts of unsafe pointers
I copied swap_nonoverlapping badly from libcore which uses an unsafe
pointer case of x as *mut u8, I thought this was x as *mut T when i was
copying code form libcore this caused new infererence variables to be
introduced which actually automatically turned this swap_nonoverlapping
function to turn into a swap_nonoverlapping<u8> and lost the general
generic one in the type system context.
Rust allows for you to cast any pointer to another pointer of different
base type. The rules here are really subtle though because we need to
handle the case where we do really need to unify the types here there
are a few cases to consider but the main three are:
*u32 vs *u32 -> valid pointers match = simple coercion
*<?> vs *u8 -> inference variable unified with u8 which is valid
*T vs *u8 -> invalid coercion as the element types don't match
-> But this is a valid cast site
The code for casts try a coercion first then if that was sucsessful goes
on to perform a real coercion. Otherwise it follows the cast rules. The
bug here was that we saw the case of *T vs *u8 the try coercion used
inference variables to match T vs u8 but this will cause a cascase of
bad inference variables which we don't want when we perform the simple
coercion.
Fixes #2330
gcc/rust/ChangeLog:
* typecheck/rust-casts.cc (TypeCastRules::check): apply new argument
* typecheck/rust-coercion.cc (TypeCoercionRules::Coerce): track if this is a cast site
(TypeCoercionRules::TryCoerce): update ctor
(TypeCoercionRules::TypeCoercionRules): cleanup error handling
(TypeCoercionRules::do_coercion): likewise
(TypeCoercionRules::coerce_unsafe_ptr): only infer in try mode
(TypeCoercionRules::coerce_borrowed_pointer): cleanup
* typecheck/rust-coercion.h: update header
gcc/testsuite/ChangeLog:
* rust/compile/issue-1981.rs: Fix badly copied libcore code test
* rust/compile/issue-2330.rs: New test.
Signed-off-by: Philip Herron <herron.philip@googlemail.com>
-rw-r--r-- | gcc/rust/typecheck/rust-casts.cc | 6 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-coercion.cc | 48 | ||||
-rw-r--r-- | gcc/rust/typecheck/rust-coercion.h | 9 | ||||
-rw-r--r-- | gcc/testsuite/rust/compile/issue-1981.rs | 4 | ||||
-rw-r--r-- | gcc/testsuite/rust/compile/issue-2330.rs | 200 |
5 files changed, 245 insertions, 22 deletions
diff --git a/gcc/rust/typecheck/rust-casts.cc b/gcc/rust/typecheck/rust-casts.cc index 95db478..26ed531 100644 --- a/gcc/rust/typecheck/rust-casts.cc +++ b/gcc/rust/typecheck/rust-casts.cc @@ -40,13 +40,15 @@ TypeCastRules::check () // https://github.com/rust-lang/rust/blob/7eac88abb2e57e752f3302f02be5f3ce3d7adfb4/compiler/rustc_typeck/src/check/cast.rs#L565-L582 auto possible_coercion = TypeCoercionRules::TryCoerce (from.get_ty (), to.get_ty (), locus, - true /*allow-autoderef*/); + true /*allow-autoderef*/, + true /*is_cast_site*/); if (!possible_coercion.is_error ()) { // given the attempt was ok we need to ensure we perform it so that any // inference variables are unified correctly return TypeCoercionRules::Coerce (from.get_ty (), to.get_ty (), locus, - true /*allow-autoderef*/); + true /*allow-autoderef*/, + true /*is_cast_site*/); } // try the simple cast rules diff --git a/gcc/rust/typecheck/rust-coercion.cc b/gcc/rust/typecheck/rust-coercion.cc index 4eb3746..591321f 100644 --- a/gcc/rust/typecheck/rust-coercion.cc +++ b/gcc/rust/typecheck/rust-coercion.cc @@ -24,9 +24,11 @@ namespace Resolver { TypeCoercionRules::CoercionResult TypeCoercionRules::Coerce (TyTy::BaseType *receiver, TyTy::BaseType *expected, - Location locus, bool allow_autoderef) + Location locus, bool allow_autoderef, + bool is_cast_site) { - TypeCoercionRules resolver (expected, locus, true, allow_autoderef, false); + TypeCoercionRules resolver (expected, locus, true, allow_autoderef, false, + is_cast_site); bool ok = resolver.do_coercion (receiver); return ok ? resolver.try_result : CoercionResult::get_error (); } @@ -34,20 +36,21 @@ TypeCoercionRules::Coerce (TyTy::BaseType *receiver, TyTy::BaseType *expected, TypeCoercionRules::CoercionResult TypeCoercionRules::TryCoerce (TyTy::BaseType *receiver, TyTy::BaseType *expected, Location locus, - bool allow_autoderef) + bool allow_autoderef, bool is_cast_site) { - TypeCoercionRules resolver (expected, locus, false, allow_autoderef, true); + TypeCoercionRules resolver (expected, locus, false, allow_autoderef, true, + is_cast_site); bool ok = resolver.do_coercion (receiver); return ok ? resolver.try_result : CoercionResult::get_error (); } TypeCoercionRules::TypeCoercionRules (TyTy::BaseType *expected, Location locus, bool emit_errors, bool allow_autoderef, - bool try_flag) + bool try_flag, bool is_cast_site) : AutoderefCycle (!allow_autoderef), mappings (Analysis::Mappings::get ()), context (TypeCheckContext::get ()), expected (expected), locus (locus), try_result (CoercionResult::get_error ()), emit_errors (emit_errors), - try_flag (try_flag) + try_flag (try_flag), is_cast_site (is_cast_site) {} bool @@ -152,10 +155,8 @@ TypeCoercionRules::do_coercion (TyTy::BaseType *receiver) TyTy::TyWithLocation (expected), TyTy::TyWithLocation (receiver), locus /*unify_locus*/, false /*emit_errors*/, - !try_flag /*commit_if_ok*/, true /*infer*/, + !try_flag /*commit_if_ok*/, try_flag /*infer*/, try_flag /*cleanup on error*/); - rust_debug ("result"); - result->debug (); if (result->get_kind () != TyTy::TypeKind::ERROR) { try_result = CoercionResult{{}, result}; @@ -223,11 +224,23 @@ TypeCoercionRules::coerce_unsafe_ptr (TyTy::BaseType *receiver, = new TyTy::PointerType (receiver->get_ref (), TyTy::TyVar (element->get_ref ()), to_mutbl); + rust_debug ("coerce_unsafe_ptr unify-site"); + + // this is a really annoying case rust allows casts of any ptr to another ptr + // types + // + // *? vs *i32 - simple coercion valid + // *? vs *T - simple coercion valid + // *i32 vs *i32 - simple coercion valid + // *i32 vs *u8 - simple coercion not valid but allowed in cast site + // *T vs *u8 - not valid but is allowed in cast site + TyTy::BaseType *result = unify_site_and (receiver->get_ref (), TyTy::TyWithLocation (expected), TyTy::TyWithLocation (coerced_mutability), - locus /*unify_locus*/, false /*emit_errors*/, - !try_flag /*commit_if_ok*/, true /*infer*/, + locus /*unify_locus*/, !try_flag /*emit_errors*/, + !try_flag /*commit_if_ok*/, + try_flag && !is_cast_site /*infer*/, try_flag /*cleanup on error*/); bool unsafe_ptr_coerceion_ok = result->get_kind () != TyTy::TypeKind::ERROR; if (unsafe_ptr_coerceion_ok) @@ -266,10 +279,15 @@ TypeCoercionRules::coerce_borrowed_pointer (TyTy::BaseType *receiver, = unify_site_and (receiver->get_ref (), TyTy::TyWithLocation (receiver), TyTy::TyWithLocation (expected), locus, - false /*emit_errors*/, true /*commit_if_ok*/, - false /* FIXME infer do we want to allow this?? */, - true /*cleanup_on_failure*/); - return CoercionResult{{}, result}; + false /*emit_errors*/, !try_flag /*commit_if_ok*/, + try_flag /* infer */, + try_flag /*cleanup_on_failure*/); + bool default_coerceion_ok + = result->get_kind () != TyTy::TypeKind::ERROR; + if (default_coerceion_ok) + return CoercionResult{{}, result}; + + return TypeCoercionRules::CoercionResult::get_error (); } } diff --git a/gcc/rust/typecheck/rust-coercion.h b/gcc/rust/typecheck/rust-coercion.h index 5a6dc64..3a6c1ae 100644 --- a/gcc/rust/typecheck/rust-coercion.h +++ b/gcc/rust/typecheck/rust-coercion.h @@ -43,11 +43,13 @@ public: static CoercionResult Coerce (TyTy::BaseType *receiver, TyTy::BaseType *expected, Location locus, - bool allow_autoderef); + bool allow_autoderef, + bool is_cast_site = false); static CoercionResult TryCoerce (TyTy::BaseType *receiver, TyTy::BaseType *expected, Location locus, - bool allow_autoderef); + bool allow_autoderef, + bool is_cast_site = false); CoercionResult coerce_unsafe_ptr (TyTy::BaseType *receiver, TyTy::PointerType *expected, @@ -69,7 +71,7 @@ public: protected: TypeCoercionRules (TyTy::BaseType *expected, Location locus, bool emit_errors, - bool allow_autoderef, bool try_flag); + bool allow_autoderef, bool try_flag, bool is_cast_site); bool select (TyTy::BaseType &autoderefed) override; @@ -88,6 +90,7 @@ private: CoercionResult try_result; bool emit_errors; bool try_flag; + bool is_cast_site; }; } // namespace Resolver diff --git a/gcc/testsuite/rust/compile/issue-1981.rs b/gcc/testsuite/rust/compile/issue-1981.rs index e3f1723..9ad2a55 100644 --- a/gcc/testsuite/rust/compile/issue-1981.rs +++ b/gcc/testsuite/rust/compile/issue-1981.rs @@ -23,8 +23,8 @@ mod ptr { } pub unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) { - let x = x as *mut T; - let y = y as *mut T; + let x = x as *mut u8; + let y = y as *mut u8; let len = mem::size_of::<T>() * count; swap_nonoverlapping_bytes(x, y, len) } diff --git a/gcc/testsuite/rust/compile/issue-2330.rs b/gcc/testsuite/rust/compile/issue-2330.rs new file mode 100644 index 0000000..97c1503 --- /dev/null +++ b/gcc/testsuite/rust/compile/issue-2330.rs @@ -0,0 +1,200 @@ +// { dg-options "-w" } +#![feature(intrinsics)] + +pub use option::Option::{self, None, Some}; +pub use result::Result::{self, Err, Ok}; + +mod option { + pub enum Option<T> { + None, + Some(T), + } +} + +mod result { + pub enum Result<T, E> { + Ok(T), + Err(E), + } +} + +#[lang = "sized"] +pub trait Sized {} + +#[lang = "clone"] +pub trait Clone: Sized { + fn clone(&self) -> Self; + + fn clone_from(&mut self, source: &Self) { + *self = source.clone() + } +} + +mod impls { + use super::Clone; + + macro_rules! impl_clone { + ($($t:ty)*) => { + $( + impl Clone for $t { + fn clone(&self) -> Self { + *self + } + } + )* + } + } + + impl_clone! { + usize u8 u16 u32 u64 // u128 + isize i8 i16 i32 i64 // i128 + f32 f64 + bool char + } +} + +#[lang = "copy"] +pub trait Copy: Clone { + // Empty. +} + +mod copy_impls { + use super::Copy; + + macro_rules! impl_copy { + ($($t:ty)*) => { + $( + impl Copy for $t {} + )* + } + } + + impl_copy! { + usize u8 u16 u32 u64 // u128 + isize i8 i16 i32 i64 // i128 + f32 f64 + bool char + } +} + +mod intrinsics { + extern "rust-intrinsic" { + pub fn add_with_overflow<T>(x: T, y: T) -> (T, bool); + pub fn wrapping_add<T>(a: T, b: T) -> T; + pub fn wrapping_sub<T>(a: T, b: T) -> T; + pub fn rotate_left<T>(a: T, b: T) -> T; + pub fn rotate_right<T>(a: T, b: T) -> T; + pub fn offset<T>(ptr: *const T, count: isize) -> *const T; + pub fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize); + pub fn move_val_init<T>(dst: *mut T, src: T); + pub fn uninit<T>() -> T; + } +} + +mod ptr { + #[lang = "const_ptr"] + impl<T> *const T { + pub unsafe fn offset(self, count: isize) -> *const T { + intrinsics::offset(self, count) + } + } + + #[lang = "mut_ptr"] + impl<T> *mut T { + pub unsafe fn offset(self, count: isize) -> *mut T { + intrinsics::offset(self, count) as *mut T + } + } + + pub unsafe fn swap_nonoverlapping<T>(x: *mut T, y: *mut T, count: usize) { + let x = x as *mut u8; + let y = y as *mut u8; + let len = mem::size_of::<T>() * count; + swap_nonoverlapping_bytes(x, y, len) + } + + pub unsafe fn swap_nonoverlapping_one<T>(x: *mut T, y: *mut T) { + // For types smaller than the block optimization below, + // just swap directly to avoid pessimizing codegen. + if mem::size_of::<T>() < 32 { + let z = read(x); + intrinsics::copy_nonoverlapping(y, x, 1); + write(y, z); + } else { + swap_nonoverlapping(x, y, 1); + } + } + + pub unsafe fn write<T>(dst: *mut T, src: T) { + intrinsics::move_val_init(&mut *dst, src) + } + + pub unsafe fn read<T>(src: *const T) -> T { + let mut tmp: T = mem::uninitialized(); + intrinsics::copy_nonoverlapping(src, &mut tmp, 1); + tmp + } + + unsafe fn swap_nonoverlapping_bytes(x: *mut u8, y: *mut u8, len: usize) { + struct Block(u64, u64, u64, u64); + struct UnalignedBlock(u64, u64, u64, u64); + + let block_size = mem::size_of::<Block>(); + + // Loop through x & y, copying them `Block` at a time + // The optimizer should unroll the loop fully for most types + // N.B. We can't use a for loop as the `range` impl calls `mem::swap` recursively + let mut i: usize = 0; + while i + block_size <= len { + // Create some uninitialized memory as scratch space + // Declaring `t` here avoids aligning the stack when this loop is unused + let mut t: Block = mem::uninitialized(); + let t = &mut t as *mut _ as *mut u8; + let x = x.offset(i as isize); + let y = y.offset(i as isize); + + // Swap a block of bytes of x & y, using t as a temporary buffer + // This should be optimized into efficient SIMD operations where available + intrinsics::copy_nonoverlapping(x, t, block_size); + intrinsics::copy_nonoverlapping(y, x, block_size); + intrinsics::copy_nonoverlapping(t, y, block_size); + i += block_size; + } + + if i < len { + // Swap any remaining bytes + let mut t: UnalignedBlock = mem::uninitialized(); + let rem = len - i; + + let t = &mut t as *mut _ as *mut u8; + let x = x.offset(i as isize); + let y = y.offset(i as isize); + + intrinsics::copy_nonoverlapping(x, t, rem); + intrinsics::copy_nonoverlapping(y, x, rem); + intrinsics::copy_nonoverlapping(t, y, rem); + } + } +} + +mod mem { + extern "rust-intrinsic" { + pub fn transmute<T, U>(_: T) -> U; + pub fn size_of<T>() -> usize; + } + + pub fn swap<T>(x: &mut T, y: &mut T) { + unsafe { + ptr::swap_nonoverlapping_one(x, y); + } + } + + pub fn test(dest: &mut i32, mut src: i32) -> i32 { + swap(dest, &mut src); + src + } + + pub unsafe fn uninitialized<T>() -> T { + intrinsics::uninit() + } +} |