aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhilip Herron <herron.philip@googlemail.com>2023-06-29 10:13:01 +0100
committerPhilip Herron <philip.herron@embecosm.com>2023-06-29 15:38:44 +0000
commit7af86eaaffec8cab5ad80e242480fe9f6c1ffaa6 (patch)
treed27ca036110d5a62c0f0f08990e3eeba9e6da87a
parenteb88e324cde1e832dc6f3a5932aef9ee8e7abf6a (diff)
downloadgcc-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.cc6
-rw-r--r--gcc/rust/typecheck/rust-coercion.cc48
-rw-r--r--gcc/rust/typecheck/rust-coercion.h9
-rw-r--r--gcc/testsuite/rust/compile/issue-1981.rs4
-rw-r--r--gcc/testsuite/rust/compile/issue-2330.rs200
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()
+ }
+}