From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1643) id 9DD1C3810AC4; Wed, 8 Jun 2022 12:49:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9DD1C3810AC4 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" From: Thomas Schwinge To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] Fix Slice Type Layout X-Act-Checkin: gcc X-Git-Author: Philip Herron X-Git-Refname: refs/heads/devel/rust/master X-Git-Oldrev: c7008d3e254786b5e752aa61e067f62c38042b81 X-Git-Newrev: cd39861da5e1113207193bb8b3e6fb3dde92895f Message-Id: <20220608124913.9DD1C3810AC4@sourceware.org> Date: Wed, 8 Jun 2022 12:49:13 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Jun 2022 12:49:13 -0000 https://gcc.gnu.org/g:cd39861da5e1113207193bb8b3e6fb3dde92895f commit cd39861da5e1113207193bb8b3e6fb3dde92895f Author: Philip Herron Date: Mon May 23 10:46:59 2022 +0100 Fix Slice Type Layout Slices in Rust are represented by TypePaths such as '[i32]'. Though if you actually try to use this explicit type-path you will hit errors such as this type has an unknown size at compile time. This is because this is actually what Rust calls a dynamically sized type. This means when you use types such as: '&[i32]' it is not actually a reference type to a slice. Its a slice in its entirety this means for lack of a better word when you use '*const [i32]' or '&mut [i32]' we end up actually passing around a struct by value _not_ at pointer/reference to it. This patch changes the type-layout so that we handle this layout change properly. This patch will also need to be applied to str types which I believe have a similar layout for safety. The patch also sets up TYPE_MAIN_VARIANT so that we can avoid unnessecary view_convert_expressions between *const [i32] and &mut [i32] which will have the same layout. Reference: https://doc.rust-lang.org/reference/dynamically-sized-types.html https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=672adac002939a2dab43b8d231adc1dc Fixes #1232 Diff: --- gcc/rust/backend/rust-compile-context.h | 12 ++ gcc/rust/backend/rust-compile-expr.cc | 56 +++++++- gcc/rust/backend/rust-compile-type.cc | 82 +++++++++--- gcc/rust/backend/rust-compile-type.h | 8 +- gcc/rust/backend/rust-compile.cc | 19 +++ gcc/rust/backend/rust-tree.h | 6 + gcc/rust/typecheck/rust-tyty.h | 30 +++++ gcc/testsuite/rust/execute/torture/issue-1232.rs | 159 +++++++++++++++++++++++ 8 files changed, 345 insertions(+), 27 deletions(-) diff --git a/gcc/rust/backend/rust-compile-context.h b/gcc/rust/backend/rust-compile-context.h index 83785549259..096b65f8b39 100644 --- a/gcc/rust/backend/rust-compile-context.h +++ b/gcc/rust/backend/rust-compile-context.h @@ -67,6 +67,17 @@ public: return type; } + tree insert_main_variant (tree type) + { + hashval_t h = type_hasher (type); + auto it = main_variants.find (h); + if (it != main_variants.end ()) + return it->second; + + main_variants.insert ({h, type}); + return type; + } + ::Backend *get_backend () { return backend; } Resolver::Resolver *get_resolver () { return resolver; } Resolver::TypeCheckContext *get_tyctx () { return tyctx; } @@ -314,6 +325,7 @@ private: std::map>> mono_fns; std::map implicit_pattern_bindings; + std::map main_variants; // To GCC middle-end std::vector type_decls; diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc index 824d6d30d49..b176ed2cf36 100644 --- a/gcc/rust/backend/rust-compile-expr.cc +++ b/gcc/rust/backend/rust-compile-expr.cc @@ -123,6 +123,11 @@ void CompileExpr::visit (HIR::BorrowExpr &expr) { tree main_expr = CompileExpr::Compile (expr.get_expr ().get (), ctx); + if (SLICE_TYPE_P (TREE_TYPE (main_expr))) + { + translated = main_expr; + return; + } TyTy::BaseType *tyty = nullptr; if (!ctx->get_tyctx ()->lookup_type (expr.get_mappings ().get_hirid (), @@ -164,6 +169,12 @@ CompileExpr::visit (HIR::DereferenceExpr &expr) } tree expected_type = TyTyResolveCompile::compile (ctx, tyty); + if (SLICE_TYPE_P (TREE_TYPE (main_expr)) && SLICE_TYPE_P (expected_type)) + { + translated = main_expr; + return; + } + bool known_valid = true; translated = ctx->get_backend ()->indirect_expression (expected_type, main_expr, @@ -1092,6 +1103,32 @@ CompileExpr::type_cast_expression (tree type_to_cast_to, tree expr_tree, return fold_build1_loc (location.gcc_location (), VIEW_CONVERT_EXPR, type_to_cast_to, expr_tree); } + else if (TREE_CODE (type_to_cast_to) == POINTER_TYPE + && SLICE_TYPE_P (TREE_TYPE (expr_tree))) + { + // returning a raw cast using NOP_EXPR seems to resut in an ICE: + // + // Analyzing compilation unit + // Performing interprocedural optimizations + // <*free_lang_data> {heap 2644k} {heap 2644k} + // {heap 2644k} {heap 2644k}during + // GIMPLE pass: cddce + // In function ‘*T::as_ptr’: + // rust1: internal compiler error: in propagate_necessity, at + // tree-ssa-dce.cc:984 0x1d5b43e propagate_necessity + // ../../gccrs/gcc/tree-ssa-dce.cc:984 + // 0x1d5e180 perform_tree_ssa_dce + // ../../gccrs/gcc/tree-ssa-dce.cc:1876 + // 0x1d5e2c8 tree_ssa_cd_dce + // ../../gccrs/gcc/tree-ssa-dce.cc:1920 + // 0x1d5e49a execute + // ../../gccrs/gcc/tree-ssa-dce.cc:1992 + + // this is returning the direct raw pointer of the slice an assumes a very + // specific layout + return ctx->get_backend ()->struct_field_expression (expr_tree, 0, + location); + } return fold_convert_loc (location.gcc_location (), type_to_cast_to, expr_tree); @@ -1261,9 +1298,13 @@ HIRCompileBase::resolve_adjustements ( case Resolver::Adjustment::AdjustmentType::IMM_REF: case Resolver::Adjustment::AdjustmentType::MUT_REF: { - tree ptrtype - = TyTyResolveCompile::compile (ctx, adjustment.get_expected ()); - e = address_expression (e, ptrtype, locus); + if (!SLICE_TYPE_P (TREE_TYPE (e))) + { + tree ptrtype + = TyTyResolveCompile::compile (ctx, + adjustment.get_expected ()); + e = address_expression (e, ptrtype, locus); + } } break; @@ -1619,6 +1660,15 @@ CompileExpr::visit (HIR::ArrayIndexExpr &expr) index, expr.get_array_expr (), expr.get_index_expr ()); + tree actual_type = TREE_TYPE (operator_overload_call); + bool can_indirect = TYPE_PTR_P (actual_type) || TYPE_REF_P (actual_type); + if (!can_indirect) + { + // nothing to do + translated = operator_overload_call; + return; + } + // lookup the expected type for this expression TyTy::BaseType *tyty = nullptr; bool ok diff --git a/gcc/rust/backend/rust-compile-type.cc b/gcc/rust/backend/rust-compile-type.cc index 102bc0a3f65..707b2afcbe3 100644 --- a/gcc/rust/backend/rust-compile-type.cc +++ b/gcc/rust/backend/rust-compile-type.cc @@ -25,6 +25,13 @@ namespace Rust { namespace Compile { +static const std::string RUST_ENUM_DISR_FIELD_NAME = "RUST$ENUM$DISR"; + +TyTyResolveCompile::TyTyResolveCompile (Context *ctx, bool trait_object_mode) + : ctx (ctx), trait_object_mode (trait_object_mode), + translated (error_mark_node), recurisve_ops (0) +{} + tree TyTyResolveCompile::compile (Context *ctx, const TyTy::BaseType *ty, bool trait_object_mode) @@ -42,8 +49,6 @@ TyTyResolveCompile::compile (Context *ctx, const TyTy::BaseType *ty, return compiler.translated; } -static const std::string RUST_ENUM_DISR_FIELD_NAME = "RUST$ENUM$DISR"; - // see: gcc/c/c-decl.cc:8230-8241 // https://github.com/Rust-GCC/gccrs/blob/0024bc2f028369b871a65ceb11b2fddfb0f9c3aa/gcc/c/c-decl.c#L8229-L8241 tree @@ -375,24 +380,7 @@ TyTyResolveCompile::visit (const TyTy::ArrayType &type) void TyTyResolveCompile::visit (const TyTy::SliceType &type) { - std::vector fields; - - tree element_type - = TyTyResolveCompile::compile (ctx, type.get_element_type ()); - tree data_field_ty = build_pointer_type (element_type); - Backend::typed_identifier data_field ("data", data_field_ty, Location ()); - fields.push_back (std::move (data_field)); - - // lookup usize - TyTy::BaseType *usize = nullptr; - bool ok = ctx->get_tyctx ()->lookup_builtin ("usize", &usize); - rust_assert (ok); - - tree len_field_ty = TyTyResolveCompile::compile (ctx, usize); - Backend::typed_identifier len_field ("len", len_field_ty, Location ()); - fields.push_back (std::move (len_field)); - - tree type_record = ctx->get_backend ()->struct_type (fields); + tree type_record = create_slice_type_record (type); std::string named_struct_str = std::string ("[") + type.get_element_type ()->get_name () + "]"; @@ -536,6 +524,21 @@ TyTyResolveCompile::visit (const TyTy::CharType &type) void TyTyResolveCompile::visit (const TyTy::ReferenceType &type) { + const TyTy::SliceType *slice = nullptr; + if (type.is_dyn_slice_type (&slice)) + { + tree type_record = create_slice_type_record (*slice); + std::string dyn_slice_type_str + = std::string (type.is_mutable () ? "&mut " : "&") + "[" + + slice->get_element_type ()->get_name () + "]"; + + translated + = ctx->get_backend ()->named_type (dyn_slice_type_str, type_record, + slice->get_locus ()); + + return; + } + tree base_compiled_type = TyTyResolveCompile::compile (ctx, type.get_base (), trait_object_mode); if (type.is_mutable ()) @@ -552,6 +555,21 @@ TyTyResolveCompile::visit (const TyTy::ReferenceType &type) void TyTyResolveCompile::visit (const TyTy::PointerType &type) { + const TyTy::SliceType *slice = nullptr; + if (type.is_dyn_slice_type (&slice)) + { + tree type_record = create_slice_type_record (*slice); + std::string dyn_slice_type_str + = std::string (type.is_mutable () ? "*mut " : "*const ") + "[" + + slice->get_element_type ()->get_name () + "]"; + + translated + = ctx->get_backend ()->named_type (dyn_slice_type_str, type_record, + slice->get_locus ()); + + return; + } + tree base_compiled_type = TyTyResolveCompile::compile (ctx, type.get_base (), trait_object_mode); if (type.is_mutable ()) @@ -615,5 +633,29 @@ TyTyResolveCompile::visit (const TyTy::DynamicObjectType &type) type.get_ident ().locus); } +tree +TyTyResolveCompile::create_slice_type_record (const TyTy::SliceType &type) +{ + // lookup usize + TyTy::BaseType *usize = nullptr; + bool ok = ctx->get_tyctx ()->lookup_builtin ("usize", &usize); + rust_assert (ok); + + tree element_type + = TyTyResolveCompile::compile (ctx, type.get_element_type ()); + tree data_field_ty = build_pointer_type (element_type); + Backend::typed_identifier data_field ("data", data_field_ty, + type.get_locus ()); + + tree len_field_ty = TyTyResolveCompile::compile (ctx, usize); + Backend::typed_identifier len_field ("len", len_field_ty, type.get_locus ()); + + tree record = ctx->get_backend ()->struct_type ({data_field, len_field}); + SLICE_FLAG (record) = 1; + TYPE_MAIN_VARIANT (record) = ctx->insert_main_variant (record); + + return record; +} + } // namespace Compile } // namespace Rust diff --git a/gcc/rust/backend/rust-compile-type.h b/gcc/rust/backend/rust-compile-type.h index 262b8fc51a0..aefacea5e60 100644 --- a/gcc/rust/backend/rust-compile-type.h +++ b/gcc/rust/backend/rust-compile-type.h @@ -60,11 +60,11 @@ public: public: static hashval_t type_hasher (tree type); +protected: + tree create_slice_type_record (const TyTy::SliceType &type); + private: - TyTyResolveCompile (Context *ctx, bool trait_object_mode) - : ctx (ctx), trait_object_mode (trait_object_mode), - translated (error_mark_node), recurisve_ops (0) - {} + TyTyResolveCompile (Context *ctx, bool trait_object_mode); Context *ctx; bool trait_object_mode; diff --git a/gcc/rust/backend/rust-compile.cc b/gcc/rust/backend/rust-compile.cc index d9349d5a07b..18a2df6b97a 100644 --- a/gcc/rust/backend/rust-compile.cc +++ b/gcc/rust/backend/rust-compile.cc @@ -218,6 +218,11 @@ HIRCompileBase::coercion_site (tree rvalue, const TyTy::BaseType *rval, = static_cast (expected); const TyTy::ReferenceType *act = static_cast (actual); + if (act->is_dyn_slice_type ()) + { + // nothing to do + return rvalue; + } tree expected_type = TyTyResolveCompile::compile (ctx, act->get_base ()); tree deref_rvalue @@ -227,6 +232,8 @@ HIRCompileBase::coercion_site (tree rvalue, const TyTy::BaseType *rval, tree coerced = coercion_site (deref_rvalue, act->get_base (), exp->get_base (), lvalue_locus, rvalue_locus); + if (exp->is_dyn_slice_type () && SLICE_TYPE_P (TREE_TYPE (coerced))) + return coerced; return address_expression (coerced, build_reference_type (TREE_TYPE (coerced)), @@ -249,6 +256,12 @@ HIRCompileBase::coercion_site (tree rvalue, const TyTy::BaseType *rval, { const TyTy::ReferenceType *act = static_cast (actual); + if (act->is_dyn_slice_type ()) + { + // nothing to do + return rvalue; + } + actual_base = act->get_base (); expected_type = TyTyResolveCompile::compile (ctx, act->get_base ()); } @@ -256,6 +269,12 @@ HIRCompileBase::coercion_site (tree rvalue, const TyTy::BaseType *rval, { const TyTy::PointerType *act = static_cast (actual); + if (act->is_dyn_slice_type ()) + { + // nothing to do + return rvalue; + } + actual_base = act->get_base (); expected_type = TyTyResolveCompile::compile (ctx, act->get_base ()); } diff --git a/gcc/rust/backend/rust-tree.h b/gcc/rust/backend/rust-tree.h index c50e090f939..2b480ada400 100644 --- a/gcc/rust/backend/rust-tree.h +++ b/gcc/rust/backend/rust-tree.h @@ -74,6 +74,12 @@ (INDIRECT_REF_P (NODE) && TREE_TYPE (TREE_OPERAND (NODE, 0)) \ && TYPE_REF_P (TREE_TYPE (TREE_OPERAND ((NODE), 0)))) +// this is a helper to differentiate RECORD types between actual records and +// slices +#define SLICE_FLAG TREE_LANG_FLAG_0 +#define SLICE_TYPE_P(TYPE) \ + (TREE_CODE (TYPE) == RECORD_TYPE && TREE_LANG_FLAG_0 (TYPE)) + namespace Rust { // forked from gcc/cp/cvt.cc convert_to_void diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h index a2950e97b56..31d26dc3340 100644 --- a/gcc/rust/typecheck/rust-tyty.h +++ b/gcc/rust/typecheck/rust-tyty.h @@ -2180,6 +2180,21 @@ public: bool is_mutable () const { return mut == Mutability::Mut; } + bool is_dyn_slice_type () const + { + return get_base ()->destructure ()->get_kind () == TyTy::TypeKind::SLICE; + } + + bool is_dyn_slice_type (const TyTy::SliceType **slice) const + { + const TyTy::BaseType *element = get_base ()->destructure (); + if (element->get_kind () != TyTy::TypeKind::SLICE) + return false; + + *slice = static_cast (element); + return true; + } + private: TyVar base; Mutability mut; @@ -2241,6 +2256,21 @@ public: bool is_const () const { return mut == Mutability::Imm; } + bool is_dyn_slice_type () const + { + return get_base ()->destructure ()->get_kind () == TyTy::TypeKind::SLICE; + } + + bool is_dyn_slice_type (const TyTy::SliceType **slice) const + { + const TyTy::BaseType *element = get_base ()->destructure (); + if (element->get_kind () != TyTy::TypeKind::SLICE) + return false; + + *slice = static_cast (element); + return true; + } + private: TyVar base; Mutability mut; diff --git a/gcc/testsuite/rust/execute/torture/issue-1232.rs b/gcc/testsuite/rust/execute/torture/issue-1232.rs new file mode 100644 index 00000000000..983ea41d7d8 --- /dev/null +++ b/gcc/testsuite/rust/execute/torture/issue-1232.rs @@ -0,0 +1,159 @@ +// { dg-additional-options "-w" } +// { dg-output "slice_access=3\n" } +extern "rust-intrinsic" { + fn offset(dst: *const T, offset: isize) -> *const T; +} + +extern "C" { + fn printf(s: *const i8, ...); +} + +struct FatPtr { + data: *const T, + len: usize, +} + +pub union Repr { + rust: *const [T], + rust_mut: *mut [T], + raw: FatPtr, +} + +pub enum Option { + None, + Some(T), +} + +#[lang = "Range"] +pub struct Range { + pub start: Idx, + pub end: Idx, +} + +#[lang = "const_slice_ptr"] +impl *const [T] { + pub const fn len(self) -> usize { + let a = unsafe { Repr { rust: self }.raw }; + a.len + } + + pub const fn as_ptr(self) -> *const T { + self as *const T + } +} + +#[lang = "const_ptr"] +impl *const T { + pub const unsafe fn offset(self, count: isize) -> *const T { + unsafe { offset(self, count) } + } + + pub const unsafe fn add(self, count: usize) -> Self { + unsafe { self.offset(count as isize) } + } + + pub const fn as_ptr(self) -> *const T { + self as *const T + } +} + +const fn slice_from_raw_parts(data: *const T, len: usize) -> *const [T] { + unsafe { + Repr { + raw: FatPtr { data, len }, + } + .rust + } +} + +#[lang = "index"] +trait Index { + type Output; + + fn index(&self, index: Idx) -> &Self::Output; +} + +pub unsafe trait SliceIndex { + type Output; + + fn get(self, slice: &T) -> Option<&Self::Output>; + + unsafe fn get_unchecked(self, slice: *const T) -> *const Self::Output; + + fn index(self, slice: &T) -> &Self::Output; +} + +unsafe impl SliceIndex<[T]> for usize { + type Output = T; + + fn get(self, slice: &[T]) -> Option<&T> { + unsafe { Option::Some(&*self.get_unchecked(slice)) } + } + + unsafe fn get_unchecked(self, slice: *const [T]) -> *const T { + // SAFETY: the caller guarantees that `slice` is not dangling, so it + // cannot be longer than `isize::MAX`. They also guarantee that + // `self` is in bounds of `slice` so `self` cannot overflow an `isize`, + // so the call to `add` is safe. + unsafe { slice.as_ptr().add(self) } + } + + fn index(self, slice: &[T]) -> &T { + // N.B., use intrinsic indexing + // &(*slice)[self] + unsafe { &*self.get_unchecked(slice) } + } +} + +unsafe impl SliceIndex<[T]> for Range { + type Output = [T]; + + fn get(self, slice: &[T]) -> Option<&[T]> { + if self.start > self.end + /* || self.end > slice.len() */ + { + Option::None + } else { + unsafe { Option::Some(&*self.get_unchecked(slice)) } + } + } + + unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { + unsafe { + let a: *const T = slice.as_ptr(); + let b: *const T = a.add(self.start); + slice_from_raw_parts(b, self.end - self.start) + } + } + + fn index(self, slice: &[T]) -> &[T] { + unsafe { &*self.get_unchecked(slice) } + } +} + +impl Index for [T] +where + I: SliceIndex<[T]>, +{ + type Output = I::Output; + + fn index(&self, index: I) -> &I::Output { + index.index(self) + } +} + +fn main() -> i32 { + let array = [1, 2, 3, 4, 5]; + let slice = &array[1..3]; + let slice_access = slice[1]; + + unsafe { + let a = "slice_access=%i\n"; + let b = a as *const str; + let c = b as *const i8; + + printf(c, slice_access); + } + + 0 +}