From 711bb86ba4bf734bbb184767b494e054ebc196ff Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 11 Jun 2024 15:23:29 -0700 Subject: [PATCH 01/28] Update docs for byreflike with generics --- docs/design/features/byreflike-generics.md | 8 +++++--- docs/design/specs/Ecma-335-Augments.md | 24 +++++++++++++++++++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index d529f18f140dfb..0a19e23d42642c 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -15,7 +15,7 @@ Supporting ByRefLike type as Generic parameters will impact the following IL ins - `newobj` – For multi-dimensional array construction. - `constrained.callvirt` – If this IL sequence resolves to a method implemented on `object` or default interface method, an error will occur during the attempt to box the instance. -If any of the above instructions are attempted to be used with a ByRefLike type, the runtime will throw an `InvalidProgramException`. Sequences involving some of the above instructions are considered optimizations and represent cases that will remain valid regardless of a `T` being ByRefLike. See "Special IL Sequences" section below for details. +If any of the above instructions are attempted to be used with a ByRefLike type, the runtime will throw an `InvalidProgramException` The following instructions are already set up to support this feature since their behavior will fail as currently defined due to the inability to box a ByRefLike type. @@ -24,6 +24,8 @@ The following instructions are already set up to support this feature since thei - `isinst` – Will always place `null` on stack. - `castclass` – Will always throw `InvalidCastException`. +**NOTE** There are sequences involving some of the above instructions that are considered optimizations. These sequences represent cases that will remain valid regardless of a `T` being ByRefLike—see ["Special IL Sequences" section](#special_il_sequences) below for details. + The expansion of ByRefLike types as Generic parameters does not relax restrictions on where ByRefLike types can be used. When `T` is ByRefLike, the use of `T` as a field will require the enclosing type to be ByRefLike. ## API Proposal @@ -112,7 +114,7 @@ Adding `gpAcceptByRefLike` to the metadata of a Generic parameter will be consid Enumerating of constructors/methods on `Span` and `ReadOnlySpan` may throw `TypeLoadException` if `T` is a ByRefLike type. See "Troublesome APIs" above for the list of APIs that cause this condition. -## Special IL Sequences +## Special IL Sequences The following are IL sequences involving the `box` instruction. They are used for common C# language constructs and shall continue to be valid, even with ByRefLike types, in cases where the result can be computed at JIT time and elided safely. These sequences must now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below and each condition will be added to the ECMA-335 addendum. @@ -120,7 +122,7 @@ The following are IL sequences involving the `box` instruction. They are used fo `box` ; `br_true/false` – The box target type is non-`Nullable`. -`box` ; `isinst` ; `unbox.any` – The box, `isint`, and unbox target types are all equal. +`box` ; `isinst` ; `unbox.any` – The box, `isinst`, and unbox target types are all equal. `box` ; `isinst` ; `br_true/false` – The box target type is equal to the unboxed target type or the box target type is `Nullable` and target type equalities can be computed. diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index f6b8f9b964eff9..7b4b0123f2d32f 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -14,7 +14,8 @@ This is a list of additions and edits to be made in ECMA-335 specifications. It - [Covariant Return Types](#covariant-return-types) - [Function Pointer Type Identity](#function-pointer-type-identity) - [Unsigned data conversion with overflow detection](#unsigned-data-conversion-with-overflow-detection) -- [Ref field support](#ref-fields) +- [Ref fields support](#ref-fields) +- [ByRefLike types in generics](#byreflike-generics) - [Rules for IL rewriters](#rules-for-il-rewriters) - [Checked user-defined operators](#checked-user-defined-operators) - [Atomic reads and writes](#atomic-reads-and-writes) @@ -1026,6 +1027,27 @@ Changes to signatures: - Add a bullet point - Managed pointers which point at null, the address just past the end of an object, or the address where an element just past the end of an array would be stored, are permitted but not dereferenceable. +## ByRefLike types in generics + +ByRefLike types, defined in C# with the `ref struct` syntax, represent types that cannot escape to the managed heap and must remain on the stack. It is possible for these types to be used as generic parameters, but in order to improve utility certain affordances are required. + +### II.10.1.7 +An additional IL keyword, `byreflike`, is introduced to indicate use of ByRefLike types is permitted. This expands the set of permissible types used by this parameters, but limits the potential instructions that can be used on instances of this generic parameter type. + +### III.4 +New sub-section should be added after III.4.33 that describes sequences of IL instructions that can be used on ByRefLike types when using within a generic context. + +#### III.4.X +The following are IL sequences involving the `box` instruction. They are used for on ByRefLike types and shall be valid in cases where the result can be computed at run-time and elided safely, through JIT compilation or interpretation. These sequences must now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below. + +`box` ; `unbox.any` – The box target type is equal to the unboxed target type. + +`box` ; `br_true/false` – The box target type is non-`Nullable`. + +`box` ; `isinst` ; `unbox.any` – The box, `isinst`, and unbox target types are all equal. + +`box` ; `isinst` ; `br_true/false` – The box target type is equal to the unboxed target type or the box target type is `Nullable` and target type equalities can be computed. + ## Rules for IL Rewriters There are apis such as `System.Runtime.CompilerServices.RuntimeHelpers.CreateSpan(...)` which require that the PE file have a particular structure. In particular, that api requires that the associated RVA of a FieldDef which is used to create a span must be naturally aligned over the data type that `CreateSpan` is instantiated over. There are 2 major concerns. From 997df5c16775136bf63f0ee7582db5070be44c3e Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 11 Jun 2024 16:13:38 -0700 Subject: [PATCH 02/28] Apply suggestions from code review Co-authored-by: Jan Kotas --- docs/design/features/byreflike-generics.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 0a19e23d42642c..b626e39e93ce69 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -15,7 +15,7 @@ Supporting ByRefLike type as Generic parameters will impact the following IL ins - `newobj` – For multi-dimensional array construction. - `constrained.callvirt` – If this IL sequence resolves to a method implemented on `object` or default interface method, an error will occur during the attempt to box the instance. -If any of the above instructions are attempted to be used with a ByRefLike type, the runtime will throw an `InvalidProgramException` +If any of the above instructions are attempted to be used with a ByRefLike type, the runtime will throw an `InvalidProgramException`. The following instructions are already set up to support this feature since their behavior will fail as currently defined due to the inability to box a ByRefLike type. @@ -24,7 +24,7 @@ The following instructions are already set up to support this feature since thei - `isinst` – Will always place `null` on stack. - `castclass` – Will always throw `InvalidCastException`. -**NOTE** There are sequences involving some of the above instructions that are considered optimizations. These sequences represent cases that will remain valid regardless of a `T` being ByRefLike—see ["Special IL Sequences" section](#special_il_sequences) below for details. +**NOTE** There are sequences involving some of the above instructions that will remain valid regardless of a `T` being ByRefLike—see ["Special IL Sequences" section](#special_il_sequences) below for details. The expansion of ByRefLike types as Generic parameters does not relax restrictions on where ByRefLike types can be used. When `T` is ByRefLike, the use of `T` as a field will require the enclosing type to be ByRefLike. From d51a810e161f66e2099f82440392036057a22a27 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 11 Jun 2024 16:19:13 -0700 Subject: [PATCH 03/28] Apply suggestions from code review --- docs/design/specs/Ecma-335-Augments.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 7b4b0123f2d32f..0bc5ec73c1b0df 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -1038,7 +1038,7 @@ An additional IL keyword, `byreflike`, is introduced to indicate use of ByRefLik New sub-section should be added after III.4.33 that describes sequences of IL instructions that can be used on ByRefLike types when using within a generic context. #### III.4.X -The following are IL sequences involving the `box` instruction. They are used for on ByRefLike types and shall be valid in cases where the result can be computed at run-time and elided safely, through JIT compilation or interpretation. These sequences must now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below. +The following are IL sequences involving the `box` instruction. They can be used on ByRefLike types and shall be valid in cases where the result can be computed at run-time and elided safely—through JIT compilation or interpretation. These sequences must now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below. `box` ; `unbox.any` – The box target type is equal to the unboxed target type. From 77fd68200bc38c6e3765644478a999e9abf772e1 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 11 Jun 2024 18:28:22 -0700 Subject: [PATCH 04/28] Feedback. --- docs/design/specs/Ecma-335-Augments.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 0bc5ec73c1b0df..9006e5da98bc8d 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -1034,6 +1034,15 @@ ByRefLike types, defined in C# with the `ref struct` syntax, represent types tha ### II.10.1.7 An additional IL keyword, `byreflike`, is introduced to indicate use of ByRefLike types is permitted. This expands the set of permissible types used by this parameters, but limits the potential instructions that can be used on instances of this generic parameter type. +### II.23.1.7 +Update the `SpecialConstraintMask` flag value and description, and add a new flag, `AllowByRefLike`. + +| Flag | Value | Description | +| --- | ----- | ----------- | +| `SpecialConstraintMask` | `0x3C` | These 4 bits contain one of the following values: | +| ... | ... | ... | +| `AllowByRefLike` | `0x20` | The generic parameter is allowed to be ByRefLike | + ### III.4 New sub-section should be added after III.4.33 that describes sequences of IL instructions that can be used on ByRefLike types when using within a generic context. From afd39c62950b5ed801f1b4a6835b64972818b76c Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 12 Jun 2024 21:28:08 -0700 Subject: [PATCH 05/28] Offline feedback. --- docs/design/features/byreflike-generics.md | 8 ++++---- docs/design/specs/Ecma-335-Augments.md | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index b626e39e93ce69..9562b0b2e71a97 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -13,7 +13,7 @@ Supporting ByRefLike type as Generic parameters will impact the following IL ins - `stsfld` / `ldsfld` – Type fields of a ByRefLike parameter cannot be marked `static`. - `newarr` / `stelem` / `ldelem` / `ldelema` – Arrays are not able to contain ByRefLike types. - `newobj` – For multi-dimensional array construction. -- `constrained.callvirt` – If this IL sequence resolves to a method implemented on `object` or default interface method, an error will occur during the attempt to box the instance. +- `constrained.callvirt` – When applied to a ByRefLike instance, if this IL sequence resolves to a method implemented on `object` or a default interface method, an error will occur during the attempt to box the ByRefLike instance. If any of the above instructions are attempted to be used with a ByRefLike type, the runtime will throw an `InvalidProgramException`. @@ -116,7 +116,7 @@ Enumerating of constructors/methods on `Span` and `ReadOnlySpan` may throw ## Special IL Sequences -The following are IL sequences involving the `box` instruction. They are used for common C# language constructs and shall continue to be valid, even with ByRefLike types, in cases where the result can be computed at JIT time and elided safely. These sequences must now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below and each condition will be added to the ECMA-335 addendum. +The following are IL sequences involving the `box` instruction. They are used for common C# language constructs and shall continue to be valid, even with ByRefLike types, in cases where the result can be computed at JIT compile time or interpretation and elided safely. These sequences **must** now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below and each condition will be added to the ECMA-335 addendum. `box` ; `unbox.any` – The box target type is equal to the unboxed target type. @@ -124,11 +124,11 @@ The following are IL sequences involving the `box` instruction. They are used fo `box` ; `isinst` ; `unbox.any` – The box, `isinst`, and unbox target types are all equal. -`box` ; `isinst` ; `br_true/false` – The box target type is equal to the unboxed target type or the box target type is `Nullable` and target type equalities can be computed. +`box` ; `isinst` ; `br_true/false` – The box target type is ByRefLike or the box target type is `Nullable` and target type equalities can be computed. ## Examples -Below are valid and invalid examples of ByRefLike as Generic parameters. All examples use the **not official** syntax, `allows ref struct`, for indicating the Generic permits ByRefLike types. +Below are valid and invalid examples of ByRefLike as Generic parameters. **1) Valid** ```csharp diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 9006e5da98bc8d..c18f0c7013d5ec 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -1047,7 +1047,7 @@ Update the `SpecialConstraintMask` flag value and description, and add a new fla New sub-section should be added after III.4.33 that describes sequences of IL instructions that can be used on ByRefLike types when using within a generic context. #### III.4.X -The following are IL sequences involving the `box` instruction. They can be used on ByRefLike types and shall be valid in cases where the result can be computed at run-time and elided safely—through JIT compilation or interpretation. These sequences must now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below. +The following are IL sequences involving the `box` instruction. They are used for ByRefLike types and shall be valid in cases where the result can be computed at run-time and elided safely—through JIT compilation or interpretation. These sequences **must** now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below. `box` ; `unbox.any` – The box target type is equal to the unboxed target type. @@ -1055,7 +1055,7 @@ The following are IL sequences involving the `box` instruction. They can be used `box` ; `isinst` ; `unbox.any` – The box, `isinst`, and unbox target types are all equal. -`box` ; `isinst` ; `br_true/false` – The box target type is equal to the unboxed target type or the box target type is `Nullable` and target type equalities can be computed. +`box` ; `isinst` ; `br_true/false` – The box target type is ByRefLike or the box target type is `Nullable` and target type equalities can be computed. ## Rules for IL Rewriters From 234889a7074d00303eac1b087093e98cb543956f Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 13 Jun 2024 09:15:40 -0700 Subject: [PATCH 06/28] Add additional tests based on feedback. --- .../generics/ByRefLike/InvalidCSharp.il | 195 +++++++++++++++--- .../generics/ByRefLike/Validate.cs | 8 +- 2 files changed, 172 insertions(+), 31 deletions(-) diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index 02a9037ca10ae6..9e2580b9300a9c 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -334,22 +334,46 @@ } .method public hidebysig - instance bool BoxIsinstBranch_UsingTypeConstraints(class InvalidCSharp.EmptyInterface) cil managed + instance bool BoxIsinstBranch_CheckForSimpleInterface(!!U) cil managed + { + ldarg.1 + box !!U + isinst InvalidCSharp.SimpleInterface + brfalse.s NOT_SIMPLEINTERFACE + + ldc.i4.1 + ret + + NOT_SIMPLEINTERFACE: + ldc.i4.0 + ret + } + + .method public hidebysig + instance bool BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!U) cil managed { .locals init ( - [0] !!U + [0] valuetype InvalidCSharp.ByRefLikeTypeWithInterface ) + ldarg.1 - isinst !!U - brfalse.s NOT_U + box !!U + isinst InvalidCSharp.ByRefLikeTypeWithInterface + brfalse.s NOT_BYREFLIKETYPEWITHINTERFACE + ldarg.1 - isinst !!U - unbox.any !!U + box !!U + isinst InvalidCSharp.ByRefLikeTypeWithInterface + unbox.any InvalidCSharp.ByRefLikeTypeWithInterface stloc.0 - ldc.i4.0 + + ldloca.s 0 + constrained. InvalidCSharp.ByRefLikeTypeWithInterface + callvirt instance int32 InvalidCSharp.SimpleInterface::Method() ret - NOT_U: - ldc.i4.1 + + NOT_BYREFLIKETYPEWITHINTERFACE: + ldc.i4.0 ret } @@ -465,17 +489,28 @@ ) } -.class interface public auto ansi abstract InvalidCSharp.EmptyInterface +.class interface public auto ansi abstract InvalidCSharp.SimpleInterface { + .method public hidebysig newslot abstract virtual + instance int32 Method() cil managed + { + } } .class public sequential ansi sealed beforefieldinit InvalidCSharp.ByRefLikeTypeWithInterface extends [System.Runtime]System.ValueType - implements InvalidCSharp.EmptyInterface + implements InvalidCSharp.SimpleInterface { .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( 01 00 00 00 ) + + .method public final hidebysig newslot virtual + instance int32 Method() cil managed + { + ldc.i4.1 + ret + } } .class public sequential ansi sealed beforefieldinit RegularValueType @@ -485,7 +520,7 @@ .class public auto ansi beforefieldinit InvalidCSharp.ClassWithInterface extends [System.Runtime]System.Object - implements InvalidCSharp.EmptyInterface + implements InvalidCSharp.SimpleInterface { .method public hidebysig specialname rtspecialname instance void .ctor () cil managed @@ -494,6 +529,13 @@ call instance void [System.Runtime]System.Object::.ctor() ret } + + .method public final hidebysig newslot virtual + instance int32 Method() cil managed + { + ldc.i4.0 + ret + } } // Generic substitution of allow-byreflike with allow-byreflike @@ -662,7 +704,7 @@ .method public hidebysig static class [System.Runtime]System.Type GenericByRefLike_ConstraintsAreIndependent_Interface_ByRefLike_Invalid() cil managed { - newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() + newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() callvirt instance class [System.Runtime]System.Type [System.Runtime]System.Object::GetType() ret } @@ -709,10 +751,11 @@ } .method public hidebysig static - bool BoxUnboxAny() cil managed + int32 BoxUnboxAny() cil managed { .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 + [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, + [1] int32 ) ldloca.s 0 @@ -721,15 +764,24 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxUnboxAny(!0) + brfalse.s NEXT_1 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + + NEXT_1: + ldloc.1 ret } .method public hidebysig static - bool BoxBranch() cil managed + int32 BoxBranch() cil managed { .locals init ( [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, - [1] bool + [1] int32 ) ldloca.s 0 @@ -739,33 +791,60 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranch(!0) - pop + brfalse.s NEXT_1 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + NEXT_1: ldloca.s 0 ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranchToOther(!0) - pop + brfalse.s NEXT_2 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + NEXT_2: ldloca.s 0 ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranchToOther(!0) - pop + brfalse.s NEXT_3 + ldloc.1 + ldc.i4.1 + add + stloc.1 + + NEXT_3: ldloca.s 0 ldloca.s 0 ldflda !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranch_WithSideEffects(!0&) + brfalse.s NEXT_4 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + NEXT_4: + ldloc.1 ret } .method public hidebysig static - bool BoxIsinstUnboxAny() cil managed + int32 BoxIsinstUnboxAny() cil managed { .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 + [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, + [1] int32 ) ldloca.s 0 @@ -774,6 +853,15 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxIsinstUnboxAny(!0) + brfalse.s NEXT_1 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + + NEXT_1: + ldloc.1 ret } @@ -794,10 +882,12 @@ } .method public hidebysig static - bool BoxIsinstBranch() cil managed + int32 BoxIsinstBranch() cil managed { .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 + [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, + [1] int32, + [2] valuetype InvalidCSharp.ByRefLikeTypeWithInterface ) ldloca.s 0 @@ -807,18 +897,69 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxIsinstBranch(!0) - pop + brfalse.s NEXT_1 + ldloc.1 + ldc.i4.1 + add + stloc.1 + + NEXT_1: ldloca.s 0 ldloca.s 0 ldflda !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxIsinstBranch_WithSideEffects(!0&) - pop + brfalse.s NEXT_2 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + + NEXT_2: + ldloca.s 0 + ldloca.s 2 + initobj InvalidCSharp.ByRefLikeTypeWithInterface + ldloc.2 + call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: + BoxIsinstBranch_CheckForSimpleInterface(!!0) + brfalse.s NEXT_3 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + + NEXT_3: + ldloca.s 0 + ldloca.s 2 + initobj InvalidCSharp.ByRefLikeTypeWithInterface + ldloc.2 + call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: + BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!0) + brfalse.s NEXT_4 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + NEXT_4: ldloca.s 0 newobj instance void InvalidCSharp.ClassWithInterface::.ctor() - call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxIsinstBranch_UsingTypeConstraints(class InvalidCSharp.EmptyInterface) + // This is expected to fail since we are not passing a ByRefLikeTypeWithInterface instance. + call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: + BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!0) + brfalse.s NEXT_5 + + ldloc.1 + ldc.i4.1 + add + stloc.1 + + NEXT_5: + ldloc.1 ret } diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 280a567f2deb84..5b304e79568044 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -61,10 +61,10 @@ public static void Validate_RecognizedOpCodeSequences_Scenarios() { Console.WriteLine($"{nameof(Validate_RecognizedOpCodeSequences_Scenarios)}..."); - Assert.True(Exec.BoxUnboxAny()); - Assert.True(Exec.BoxBranch()); - Assert.True(Exec.BoxIsinstUnboxAny()); - Assert.True(Exec.BoxIsinstBranch()); + Assert.Equal(1, Exec.BoxUnboxAny()); + Assert.Equal(4, Exec.BoxBranch()); + Assert.Equal(1, Exec.BoxIsinstUnboxAny()); + Assert.Equal(4, Exec.BoxIsinstBranch()); } [Fact] From 111f4daeedc35e1a7ee795c696c1b06e5d04412a Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 16 Jun 2024 10:05:25 -0700 Subject: [PATCH 07/28] Add check at run-time to CoreCLR and native AOT. This is needed when generics parameters are involved. --- docs/design/features/byreflike-generics.md | 16 +- docs/design/specs/Ecma-335-Augments.md | 8 +- .../Runtime/CompilerServices/CastHelpers.cs | 9 +- src/coreclr/inc/corinfo.h | 1 + src/coreclr/inc/jithelpers.h | 1 + src/coreclr/jit/importer.cpp | 18 ++ .../src/System/Runtime/TypeCast.cs | 12 +- .../Internal/Runtime/ReadyToRunConstants.cs | 1 + .../Common/JitInterface/CorInfoHelpFunc.cs | 1 + .../ILCompiler.Compiler/Compiler/JitHelper.cs | 3 + .../JitInterface/CorInfoImpl.RyuJit.cs | 3 + src/coreclr/vm/corelib.h | 1 + src/coreclr/vm/ecall.cpp | 4 + src/coreclr/vm/jithelpers.cpp | 15 + src/coreclr/vm/jitinterface.h | 3 + src/coreclr/vm/metasig.h | 1 + src/coreclr/vm/qcallentrypoints.cpp | 1 + .../generics/ByRefLike/InvalidCSharp.il | 283 ++++++++++++------ .../generics/ByRefLike/Validate.cs | 8 +- 19 files changed, 276 insertions(+), 113 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 9562b0b2e71a97..ea95ca1f56f53e 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -19,10 +19,10 @@ If any of the above instructions are attempted to be used with a ByRefLike type, The following instructions are already set up to support this feature since their behavior will fail as currently defined due to the inability to box a ByRefLike type. -- `throw` – Requires an object reference to be on stack, which can never be a ByRefLike type. -- `unbox` / `unbox.any` – Requires an object reference to be on stack, which can never be a ByRefLike type. -- `isinst` – Will always place `null` on stack. -- `castclass` – Will always throw `InvalidCastException`. +- `throw` +- `unbox` / `unbox.any` +- `isinst` +- `castclass` **NOTE** There are sequences involving some of the above instructions that will remain valid regardless of a `T` being ByRefLike—see ["Special IL Sequences" section](#special_il_sequences) below for details. @@ -118,13 +118,13 @@ Enumerating of constructors/methods on `Span` and `ReadOnlySpan` may throw The following are IL sequences involving the `box` instruction. They are used for common C# language constructs and shall continue to be valid, even with ByRefLike types, in cases where the result can be computed at JIT compile time or interpretation and elided safely. These sequences **must** now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below and each condition will be added to the ECMA-335 addendum. -`box` ; `unbox.any` – The box target type is equal to the unboxed target type. +`box` ; `unbox.any` – Becomes a NOP, if the box target type is equal to the unboxed target type. -`box` ; `br_true/false` – The box target type is non-`Nullable`. +`box` ; `br_true/false` – Becomes the constant `true`, if the box target type is non-`Nullable`. -`box` ; `isinst` ; `unbox.any` – The box, `isinst`, and unbox target types are all equal. +`box` ; `isinst` ; `unbox.any` – Becomes a NOP, if the box, `isinst`, and unbox target types are all equal. -`box` ; `isinst` ; `br_true/false` – The box target type is ByRefLike or the box target type is `Nullable` and target type equalities can be computed. +`box` ; `isinst` ; `br_true/false` – Becomes a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. The sequence will also be elided if the box target type is a ByRefLike type, but needs to be checked at run-time, not JIT compile time. This latter case is common when the box target type uses Generic parameters. ## Examples diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index c18f0c7013d5ec..7bcf723d05f41f 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -1049,13 +1049,13 @@ New sub-section should be added after III.4.33 that describes sequences of IL in #### III.4.X The following are IL sequences involving the `box` instruction. They are used for ByRefLike types and shall be valid in cases where the result can be computed at run-time and elided safely—through JIT compilation or interpretation. These sequences **must** now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below. -`box` ; `unbox.any` – The box target type is equal to the unboxed target type. +`box` ; `unbox.any` – Becomes a NOP, if the box target type is equal to the unboxed target type. -`box` ; `br_true/false` – The box target type is non-`Nullable`. +`box` ; `br_true/false` – Becomes the constant `true`, if the box target type is non-`Nullable`. -`box` ; `isinst` ; `unbox.any` – The box, `isinst`, and unbox target types are all equal. +`box` ; `isinst` ; `unbox.any` – Becomes a NOP, if the box, `isinst`, and unbox target types are all equal. -`box` ; `isinst` ; `br_true/false` – The box target type is ByRefLike or the box target type is `Nullable` and target type equalities can be computed. +`box` ; `isinst` ; `br_true/false` – Becomes a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. The sequence will also be elided if the box target type is a ByRefLike type, but needs to be checked at run-time, not JIT compile time. This latter case is common when the box target type uses Generic parameters. ## Rules for IL Rewriters diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 0c640dd1c2f0a4..5d4e5c542f55c2 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -8,7 +8,7 @@ namespace System.Runtime.CompilerServices { [StackTraceHidden] [DebuggerStepThrough] - internal static unsafe class CastHelpers + internal static unsafe partial class CastHelpers { // In coreclr the table is allocated and written to on the native side. internal static int[]? s_table; @@ -25,6 +25,9 @@ internal static unsafe class CastHelpers [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CastHelpers_CanCastTypeToType")] + private static partial int CanCastTypeToType(void* fromTypeHnd, void* toTypeHnd); + // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, // this test must deal with all kinds of type tests @@ -316,6 +319,10 @@ internal static unsafe class CastHelpers return ChkCastClassSpecial(toTypeHnd, obj); } + [DebuggerHidden] + private static int ChkCastTypeToType(void* fromTypeHnd, void* toTypeHnd) + => CanCastTypeToType(fromTypeHnd, toTypeHnd); + // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check [DebuggerHidden] diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index db5499b81fd33e..517d5751df9783 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -437,6 +437,7 @@ enum CorInfoHelpFunc CORINFO_HELP_CHKCASTARRAY, CORINFO_HELP_CHKCASTCLASS, CORINFO_HELP_CHKCASTANY, + CORINFO_HELP_CHKCASTTYPETOTYPE, // Used to check if TypeHandle can be cast to TypeHandle CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index ec89b618b909bc..257d6c3899c578 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -99,6 +99,7 @@ DYNAMICJITHELPER(CORINFO_HELP_CHKCASTARRAY, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTANY, NULL, CORINFO_HELP_SIG_REG_ONLY) + DYNAMICJITHELPER(CORINFO_HELP_CHKCASTTYPETOTYPE, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS_SPECIAL, NULL, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_ISINSTANCEOF_EXCEPTION, JIT_IsInstanceOfException, CORINFO_HELP_SIG_REG_ONLY) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index a98a564d403b92..4ae7a1e6a97d83 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -3021,6 +3021,24 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, typeInfo(TYP_INT)); return returnToken; } + + // Casts that result to TypeCompareState::May and involve a ByRefLike type, + // must be checked at run-time. + if (opts == BoxPatterns::IsByRefLike) + { + assert(castResult == TypeCompareState::May); + JITDUMP("\n Importing BOX; ISINST; BR_TRUE/FALSE for ByRefLike, must be checked at run-time\n"); + + impSpillSideEffects(false, CHECK_SPILL_ALL DEBUGARG("spilling side-effects")); + impPopStack(); + + GenTree* toType = impTokenToHandle(pResolvedToken); + GenTree* fromType = impTokenToHandle(&isInstResolvedToken); + impPushOnStack( + gtNewHelperCallNode(CORINFO_HELP_CHKCASTTYPETOTYPE, TYP_INT, toType, fromType), + typeInfo(TYP_INT)); + return returnToken; + } } else if ((foldAsHelper == CORINFO_HELP_BOX_NULLABLE) && ((impStackTop().val->gtFlags & GTF_SIDE_EFFECT) == 0)) diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 0161e8c47c1508..1c7c32131fa14f 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -307,6 +307,10 @@ public static unsafe object CheckCastAny(MethodTable* pTargetType, object obj) return objRet; } + [RuntimeExport("RhTypeCast_CheckCastTypeToType")] + public static unsafe int CheckCastTypeToType(MethodTable* fromTypeMT, MethodTable* toTypeMT) + => AreTypesAssignable(fromTypeMT, toTypeMT) ? 1 : 0; + [RuntimeExport("RhTypeCast_CheckCastInterface")] public static unsafe object CheckCastInterface(MethodTable* pTargetType, object obj) { @@ -414,10 +418,10 @@ public static unsafe object CheckCastClass(MethodTable* pTargetType, object obj) [RuntimeExport("RhTypeCast_CheckCastClassSpecial")] private static unsafe object CheckCastClassSpecial(MethodTable* pTargetType, object obj) { - Debug.Assert(!pTargetType->IsParameterizedType, "CheckCastClass called with parameterized MethodTable"); - Debug.Assert(!pTargetType->IsFunctionPointer, "CheckCastClass called with function pointer MethodTable"); - Debug.Assert(!pTargetType->IsInterface, "CheckCastClass called with interface MethodTable"); - Debug.Assert(!pTargetType->HasGenericVariance, "CheckCastClass with variant MethodTable"); + Debug.Assert(!pTargetType->IsParameterizedType, "CheckCastClassSpecial called with parameterized MethodTable"); + Debug.Assert(!pTargetType->IsFunctionPointer, "CheckCastClassSpecial called with function pointer MethodTable"); + Debug.Assert(!pTargetType->IsInterface, "CheckCastClassSpecial called with interface MethodTable"); + Debug.Assert(!pTargetType->HasGenericVariance, "CheckCastClassSpecial with variant MethodTable"); MethodTable* mt = obj.GetMethodTable(); Debug.Assert(mt != pTargetType, "The check for the trivial cases should be inlined by the JIT"); diff --git a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs index af86b107e5b31e..4ad3e6eb6b0a3a 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs @@ -365,6 +365,7 @@ public enum ReadyToRunHelper GetRuntimeType, + CheckCastTypeToType, CheckCastInterface, CheckCastClass, CheckCastClassSpecial, diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs index 14bf90a9aaaed4..07b1b1b85232b6 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs @@ -79,6 +79,7 @@ which is the right helper to use to allocate an object of a given type. */ CORINFO_HELP_CHKCASTARRAY, CORINFO_HELP_CHKCASTCLASS, CORINFO_HELP_CHKCASTANY, + CORINFO_HELP_CHKCASTTYPETOTYPE, CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs index c04658df379f7a..a0f4b6b231d833 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs @@ -278,6 +278,9 @@ public static void GetEntryPoint(TypeSystemContext context, ReadyToRunHelper id, case ReadyToRunHelper.CheckCastAny: mangledName = "RhTypeCast_CheckCastAny"; break; + case ReadyToRunHelper.CheckCastTypeToType: + mangledName = "RhTypeCast_CheckCastTypeToType"; + break; case ReadyToRunHelper.CheckCastInterface: mangledName = "RhTypeCast_CheckCastInterface"; break; diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 46e3b1af0e4628..0ae284cf09f11b 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -728,6 +728,9 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum) case CorInfoHelpFunc.CORINFO_HELP_CHKCASTARRAY: id = ReadyToRunHelper.CheckCastAny; break; + case CorInfoHelpFunc.CORINFO_HELP_CHKCASTTYPETOTYPE: + id = ReadyToRunHelper.CheckCastTypeToType; + break; case CorInfoHelpFunc.CORINFO_HELP_CHKCASTINTERFACE: id = ReadyToRunHelper.CheckCastInterface; break; diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index df42e52eab9527..7a4cd8c417f1d8 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1165,6 +1165,7 @@ DEFINE_METHOD(CASTHELPERS, ISINSTANCEOFINTERFACE, IsInstanceOfInterface, SM_Ptr DEFINE_METHOD(CASTHELPERS, CHKCASTANY, ChkCastAny, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTINTERFACE, ChkCastInterface, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASS, ChkCastClass, SM_PtrVoid_Obj_RetObj) +DEFINE_METHOD(CASTHELPERS, CHKCASTTYPETOTYPE, ChkCastTypeToType, SM_PtrVoid_PtrVoid_RetInt) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte) DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) diff --git a/src/coreclr/vm/ecall.cpp b/src/coreclr/vm/ecall.cpp index d3e8415d6adeea..0b07c8dfe821dd 100644 --- a/src/coreclr/vm/ecall.cpp +++ b/src/coreclr/vm/ecall.cpp @@ -121,6 +121,10 @@ void ECall::PopulateManagedHelpers() // array cast uses the "ANY" helper SetJitHelperFunction(CORINFO_HELP_CHKCASTARRAY, pDest); + pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__CHKCASTTYPETOTYPE)); + pDest = pMD->GetMultiCallableAddrOfCode(); + SetJitHelperFunction(CORINFO_HELP_CHKCASTTYPETOTYPE, pDest); + pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__CHKCASTINTERFACE)); pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_CHKCASTINTERFACE, pDest); diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 13fbeaab2332d5..e11448950b07dc 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2612,6 +2612,21 @@ HCIMPL2(LPVOID, ArrayStoreCheck, Object** pElement, PtrArray** pArray) } HCIMPLEND +extern "C" BOOL QCALLTYPE CastHelpers_CanCastTypeToType(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd) +{ + QCALL_CONTRACT; + + BOOL ret = FALSE; + + BEGIN_QCALL; + TypeHandle fromType(fromTypeHnd); + TypeHandle toType(toTypeHnd); + ret = fromType.CanCastTo(toType); + END_QCALL; + + return ret; +} + //======================================================================== // // VALUETYPE/BYREF HELPERS diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 51e2b959d6f694..3f425747172169 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -18,6 +18,7 @@ #define MAX_UNCHECKED_OFFSET_FOR_NULL_OBJECT ((GetOsPageSize() / 2) - 1) #endif // !TARGET_UNIX #include "pgo.h" +#include "qcall.h" enum StompWriteBarrierCompletionAction { @@ -243,6 +244,8 @@ extern "C" FCDECL2(Object*, IsInstanceOfAny_NoCacheLookup, CORINFO_CLASS_HANDLE extern "C" FCDECL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj); extern "C" FCDECL3(void, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Object* obj); +extern "C" BOOL QCALLTYPE CastHelpers_CanCastTypeToType(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd); + // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location extern "C" FCDECL2(VOID, JIT_WriteBarrier_Callable, Object **dst, Object *ref); diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 35e6472b4b8daf..bc741689dd5f18 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -354,6 +354,7 @@ DEFINE_METASIG(SM(PtrChar_RetVoid, P(u), v)) DEFINE_METASIG(SM(IntPtr_IntPtr_RetIntPtr, I I, I)) DEFINE_METASIG(SM(IntPtr_IntPtr_Int_RetIntPtr, I I i, I)) DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetVoid, P(v) P(v), v)) +DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetInt, P(v) P(v), i)) DEFINE_METASIG(SM(PtrVoid_PtrVoid_UInt_RetVoid, P(v) P(v) K, v)) DEFINE_METASIG(IM(Obj_RetBool, j, F)) DEFINE_METASIG(SM(Obj_RetVoid, j, v)) diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 2c719586aed988..2fc22bc0ae797d 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -80,6 +80,7 @@ static const Entry s_QCall[] = DllImportEntry(ArgIterator_GetNextArgType) DllImportEntry(ArgIterator_GetNextArg) DllImportEntry(ArgIterator_GetNextArg2) + DllImportEntry(CastHelpers_CanCastTypeToType) DllImportEntry(CustomAttribute_ParseAttributeUsageAttribute) DllImportEntry(CustomAttribute_CreateCustomAttributeInstance) DllImportEntry(CustomAttribute_CreatePropertyOrFieldData) diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index 9e2580b9300a9c..f946d0261f99b3 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -336,16 +336,25 @@ .method public hidebysig instance bool BoxIsinstBranch_CheckForSimpleInterface(!!U) cil managed { + .locals init ( + [0] !!U + ) + ldarg.1 box !!U isinst InvalidCSharp.SimpleInterface - brfalse.s NOT_SIMPLEINTERFACE + brtrue.s IS_SIMPLEINTERFACE - ldc.i4.1 - ret + ldstr "All types should implement SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw - NOT_SIMPLEINTERFACE: - ldc.i4.0 + IS_SIMPLEINTERFACE: + ldarg.1 + stloc.0 + ldloca.s 0 + constrained. !!U + callvirt instance int32 InvalidCSharp.SimpleInterface::Method() ret } @@ -353,22 +362,22 @@ instance bool BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!U) cil managed { .locals init ( - [0] valuetype InvalidCSharp.ByRefLikeTypeWithInterface + [0] valuetype ByRefLikeTypeWithInterface ) ldarg.1 box !!U - isinst InvalidCSharp.ByRefLikeTypeWithInterface + isinst ByRefLikeTypeWithInterface brfalse.s NOT_BYREFLIKETYPEWITHINTERFACE ldarg.1 box !!U - isinst InvalidCSharp.ByRefLikeTypeWithInterface - unbox.any InvalidCSharp.ByRefLikeTypeWithInterface + isinst ByRefLikeTypeWithInterface + unbox.any ByRefLikeTypeWithInterface stloc.0 ldloca.s 0 - constrained. InvalidCSharp.ByRefLikeTypeWithInterface + constrained. ByRefLikeTypeWithInterface callvirt instance int32 InvalidCSharp.SimpleInterface::Method() ret @@ -489,6 +498,14 @@ ) } +.class public sequential ansi sealed beforefieldinit ByRefLikeType`1 + extends [System.Runtime]System.ValueType +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( + 01 00 00 00 + ) +} + .class interface public auto ansi abstract InvalidCSharp.SimpleInterface { .method public hidebysig newslot abstract virtual @@ -497,7 +514,7 @@ } } -.class public sequential ansi sealed beforefieldinit InvalidCSharp.ByRefLikeTypeWithInterface +.class public sequential ansi sealed beforefieldinit ByRefLikeTypeWithInterface extends [System.Runtime]System.ValueType implements InvalidCSharp.SimpleInterface { @@ -704,14 +721,14 @@ .method public hidebysig static class [System.Runtime]System.Type GenericByRefLike_ConstraintsAreIndependent_Interface_ByRefLike_Invalid() cil managed { - newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() + newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() callvirt instance class [System.Runtime]System.Type [System.Runtime]System.Object::GetType() ret } .method public hidebysig static class [System.Runtime]System.Type GenericByRefLike_ConstraintsAreIndependent_ByRefLike_ByRefLike_Invalid() cil managed { - newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() + newobj instance void class InvalidCSharp.GenericClass_IndependentConstraints`2::.ctor() callvirt instance class [System.Runtime]System.Type [System.Runtime]System.Object::GetType() ret } @@ -751,11 +768,10 @@ } .method public hidebysig static - int32 BoxUnboxAny() cil managed + void BoxUnboxAny() cil managed { .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, - [1] int32 + [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 ) ldloca.s 0 @@ -764,24 +780,21 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxUnboxAny(!0) - brfalse.s NEXT_1 + brtrue.s NEXT_1 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxUnboxAny for ByRefLikeType" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_1: - ldloc.1 ret } .method public hidebysig static - int32 BoxBranch() cil managed + void BoxBranch() cil managed { .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, - [1] int32 + [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 ) ldloca.s 0 @@ -791,60 +804,54 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranch(!0) - brfalse.s NEXT_1 + brtrue.s NEXT_1 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxBranch for ByRefLikeType" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_1: ldloca.s 0 ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field - call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranchToOther(!0) - brfalse.s NEXT_2 + call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranchToOther(!0) + brtrue.s NEXT_2 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxBranchToOther for ByRefLikeTypeWithInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_2: ldloca.s 0 ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranchToOther(!0) - brfalse.s NEXT_3 + brtrue.s NEXT_3 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxBranchToOther for RegularValueType" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_3: ldloca.s 0 ldloca.s 0 ldflda !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxBranch_WithSideEffects(!0&) - brfalse.s NEXT_4 + brtrue.s NEXT_4 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxBranch_WithSideEffects for ByRefLikeType" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_4: - ldloc.1 ret } .method public hidebysig static - int32 BoxIsinstUnboxAny() cil managed + void BoxIsinstUnboxAny() cil managed { .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, - [1] int32 + [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 ) ldloca.s 0 @@ -853,15 +860,13 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxIsinstUnboxAny(!0) - brfalse.s NEXT_1 + brtrue.s NEXT_1 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxIsinstUnboxAny for ByRefLikeType" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_1: - ldloc.1 ret } @@ -881,13 +886,33 @@ ret } + .method private hidebysig static + bool CheckIsInstance(!!T) cil managed noinlining + { + ldarg.0 + // Begin sequence + box !!T + isinst !!U + brfalse.s IS_FALSE + // End sequence + + ldc.i4.1 + ret + + IS_FALSE: + ldc.i4.0 + ret + } + .method public hidebysig static - int32 BoxIsinstBranch() cil managed + void BoxIsinstBranch() cil managed { .locals init ( [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, - [1] int32, - [2] valuetype InvalidCSharp.ByRefLikeTypeWithInterface + [1] valuetype ByRefLikeType, + [2] valuetype ByRefLikeTypeWithInterface, + [3] valuetype ByRefLikeType`1, + [4] valuetype ByRefLikeType`1 ) ldloca.s 0 @@ -897,69 +922,143 @@ ldloc 0 ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxIsinstBranch(!0) - brfalse.s NEXT_1 + brtrue.s NEXT_1 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxIsinstBranch for ByRefLikeType" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_1: ldloca.s 0 ldloca.s 0 ldflda !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxIsinstBranch_WithSideEffects(!0&) - brfalse.s NEXT_2 + brtrue.s NEXT_2 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxIsinstBranch_WithSideEffects for ByRefLikeType" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_2: ldloca.s 0 ldloca.s 2 - initobj InvalidCSharp.ByRefLikeTypeWithInterface + initobj valuetype ByRefLikeTypeWithInterface ldloc.2 call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: - BoxIsinstBranch_CheckForSimpleInterface(!!0) - brfalse.s NEXT_3 + BoxIsinstBranch_CheckForSimpleInterface(!!0) + brtrue.s NEXT_3 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "Failed: BoxIsinstBranch_CheckForSimpleInterface for ByRefLikeTypeWithInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_3: ldloca.s 0 - ldloca.s 2 - initobj InvalidCSharp.ByRefLikeTypeWithInterface - ldloc.2 + newobj instance void InvalidCSharp.ClassWithInterface::.ctor() call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: - BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!0) + BoxIsinstBranch_CheckForSimpleInterface(!!0) brfalse.s NEXT_4 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "The above is expected to be false since ClassWithInterface's interface implementation return 0." + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw NEXT_4: ldloca.s 0 - newobj instance void InvalidCSharp.ClassWithInterface::.ctor() + ldloca.s 2 + initobj valuetype ByRefLikeTypeWithInterface + ldloc.2 + call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: + BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!0) + brtrue.s NEXT_5 + + ldstr "Failed: BoxIsinstBranch_CheckForByRefLikeTypeWithInterface for ByRefLikeTypeWithInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw - // This is expected to fail since we are not passing a ByRefLikeTypeWithInterface instance. + NEXT_5: + ldloca.s 0 + newobj instance void InvalidCSharp.ClassWithInterface::.ctor() call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!0) - brfalse.s NEXT_5 + brfalse.s NEXT_6 - ldloc.1 - ldc.i4.1 - add - stloc.1 + ldstr "The above is expected to be false since we are not passing a ByRefLikeTypeWithInterface instance." + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw - NEXT_5: + NEXT_6: + ldloca.s 1 + initobj valuetype ByRefLikeType ldloc.1 + call bool Exec::CheckIsInstance(!!0) + brfalse.s NEXT_7 + + ldstr "Failed: CheckIsInstance for ByRefLikeType and SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + + NEXT_7: + ldloca.s 2 + initobj valuetype ByRefLikeTypeWithInterface + ldloc.2 + call bool Exec::CheckIsInstance(!!0) + brtrue.s NEXT_8 + + ldstr "Failed: CheckIsInstance for ByRefLikeTypeWithInterface and SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + + NEXT_8: + newobj instance void InvalidCSharp.ClassWithInterface::.ctor() + call bool Exec::CheckIsInstance(!!0) + brtrue.s NEXT_9 + + ldstr "Failed: CheckIsInstance for ClassWithInterface and SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + + NEXT_9: + ldstr "EMPTY" + call bool Exec::CheckIsInstance(!!0) + brfalse.s NEXT_10 + + ldstr "Failed: CheckIsInstance for string and SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + + NEXT_10: + ldc.i4.1 + call bool Exec::CheckIsInstance(!!0) + brfalse.s NEXT_11 + + ldstr "Failed: CheckIsInstance for int32 and SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + + NEXT_11: + ldloca.s 3 + initobj valuetype ByRefLikeType`1 + ldloc.3 + call bool Exec::CheckIsInstance, class InvalidCSharp.SimpleInterface>(!!0) + brfalse.s NEXT_12 + + ldstr "Failed: CheckIsInstance for valuetype ByRefLikeType`1 and SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + + NEXT_12: + ldloca.s 4 + initobj valuetype ByRefLikeType`1 + ldloc 4 + call bool Exec::CheckIsInstance, class InvalidCSharp.SimpleInterface>(!!0) + brfalse.s NEXT_13 + + ldstr "Failed: CheckIsInstance for valuetype ByRefLikeType`1 and SimpleInterface" + newobj instance void [System.Runtime]System.Exception::.ctor(string) + throw + + NEXT_13: ret } diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 5b304e79568044..91042e97404370 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -61,10 +61,10 @@ public static void Validate_RecognizedOpCodeSequences_Scenarios() { Console.WriteLine($"{nameof(Validate_RecognizedOpCodeSequences_Scenarios)}..."); - Assert.Equal(1, Exec.BoxUnboxAny()); - Assert.Equal(4, Exec.BoxBranch()); - Assert.Equal(1, Exec.BoxIsinstUnboxAny()); - Assert.Equal(4, Exec.BoxIsinstBranch()); + Exec.BoxUnboxAny(); + Exec.BoxBranch(); + Exec.BoxIsinstUnboxAny(); + Exec.BoxIsinstBranch(); } [Fact] From 04a31c32fa3ae169063bf7fb95c1257b13a6ab4d Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 16 Jun 2024 10:08:59 -0700 Subject: [PATCH 08/28] New JIT helper mean rev'ing JIT interface ID. --- src/coreclr/inc/jiteeversionguid.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 35a7e623b9a9b3..969d967404412f 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 6e0b439f-0d18-4836-a486-4962af0cc948 */ - 0x6e0b439f, - 0x0d18, - 0x4836, - {0xa4, 0x86, 0x49, 0x62, 0xaf, 0x0c, 0xc9, 0x48} +constexpr GUID JITEEVersionIdentifier = { /* 5939fe51-f24a-4480-8cea-1cd50a21e5e8 */ + 0x5939fe51, + 0xf24a, + 0x4480, + {0x8c, 0xea, 0x1c, 0xd5, 0x0a, 0x21, 0xe5, 0xe8} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// From 7f01e29ef7403bfc2b50f1fedaf04859604d9e09 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 16 Jun 2024 10:15:42 -0700 Subject: [PATCH 09/28] Update JIT interface GUID for new JIT helper. --- src/coreclr/inc/jiteeversionguid.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 6bcb13b5a92656..969d967404412f 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* de0cd36d-3094-4110-af7d-31eb36234e46 */ - 0xde0cd36d, - 0x3094, - 0x4110, - {0xaf, 0x7d, 0x31, 0xeb, 0x36, 0x23, 0x4e, 0x46} +constexpr GUID JITEEVersionIdentifier = { /* 5939fe51-f24a-4480-8cea-1cd50a21e5e8 */ + 0x5939fe51, + 0xf24a, + 0x4480, + {0x8c, 0xea, 0x1c, 0xd5, 0x0a, 0x21, 0xe5, 0xe8} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// From a1e62e93700ab1447443647d28a592f83fff0a75 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 16 Jun 2024 10:33:44 -0700 Subject: [PATCH 10/28] Apply JIT format --- src/coreclr/jit/importer.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 67ac284dfe09ba..2319ecee3a658f 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -3027,16 +3027,17 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, if (opts == BoxPatterns::IsByRefLike) { assert(castResult == TypeCompareState::May); - JITDUMP("\n Importing BOX; ISINST; BR_TRUE/FALSE for ByRefLike, must be checked at run-time\n"); + JITDUMP( + "\n Importing BOX; ISINST; BR_TRUE/FALSE for ByRefLike, must be checked at run-time\n"); impSpillSideEffects(false, CHECK_SPILL_ALL DEBUGARG("spilling side-effects")); impPopStack(); - GenTree* toType = impTokenToHandle(pResolvedToken); + GenTree* toType = impTokenToHandle(pResolvedToken); GenTree* fromType = impTokenToHandle(&isInstResolvedToken); - impPushOnStack( - gtNewHelperCallNode(CORINFO_HELP_CHKCASTTYPETOTYPE, TYP_INT, toType, fromType), - typeInfo(TYP_INT)); + impPushOnStack(gtNewHelperCallNode(CORINFO_HELP_CHKCASTTYPETOTYPE, TYP_INT, toType, + fromType), + typeInfo(TYP_INT)); return returnToken; } } From ebf31d396f323495a68ff07f95a688767bd3ca8b Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 16 Jun 2024 14:34:38 -0700 Subject: [PATCH 11/28] Feedback on rename --- .../src/System/Runtime/CompilerServices/CastHelpers.cs | 8 ++++---- src/coreclr/inc/corinfo.h | 4 ++-- src/coreclr/inc/jithelpers.h | 2 +- src/coreclr/jit/importer.cpp | 2 +- .../nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs | 8 ++++---- .../tools/Common/Internal/Runtime/ReadyToRunConstants.cs | 2 +- src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs | 2 +- .../tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs | 6 +++--- .../ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs | 6 +++--- src/coreclr/vm/corelib.h | 2 +- src/coreclr/vm/ecall.cpp | 8 ++++---- src/coreclr/vm/jithelpers.cpp | 2 +- src/coreclr/vm/jitinterface.h | 2 +- src/coreclr/vm/metasig.h | 2 +- src/coreclr/vm/qcallentrypoints.cpp | 2 +- 15 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index e0017a77c861a0..33a959d7bfb15e 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -25,8 +25,8 @@ internal static unsafe partial class CastHelpers [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CastHelpers_CanCastTypeToType")] - private static partial int CanCastTypeToType(void* fromTypeHnd, void* toTypeHnd); + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CastHelpers_AreTypesAssignable")] + private static partial Interop.BOOL AreTypesAssignable(void* fromTypeHnd, void* toTypeHnd); // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, @@ -320,8 +320,8 @@ internal static unsafe partial class CastHelpers } [DebuggerHidden] - private static int ChkCastTypeToType(void* fromTypeHnd, void* toTypeHnd) - => CanCastTypeToType(fromTypeHnd, toTypeHnd); + private static bool IsAssignable(void* fromTypeHnd, void* toTypeHnd) + => AreTypesAssignable(fromTypeHnd, toTypeHnd) != Interop.BOOL.FALSE; // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 00cdab659e511f..68605f1df043c1 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -437,9 +437,9 @@ enum CorInfoHelpFunc CORINFO_HELP_CHKCASTARRAY, CORINFO_HELP_CHKCASTCLASS, CORINFO_HELP_CHKCASTANY, - CORINFO_HELP_CHKCASTTYPETOTYPE, // Used to check if TypeHandle can be cast to TypeHandle CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check + CORINFO_HELP_ISASSIGNABLE, // Used to check if TypeHandle can be assigned to TypeHandle CORINFO_HELP_ISINSTANCEOF_EXCEPTION, @@ -2420,7 +2420,7 @@ class ICorStaticInfo virtual size_t getClassThreadStaticDynamicInfo ( CORINFO_CLASS_HANDLE cls ) = 0; - + virtual bool getStaticBaseAddress( CORINFO_CLASS_HANDLE cls, bool isGc, diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 304e77b4652020..4d5b43a549789b 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -99,8 +99,8 @@ DYNAMICJITHELPER(CORINFO_HELP_CHKCASTARRAY, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTANY, NULL, CORINFO_HELP_SIG_REG_ONLY) - DYNAMICJITHELPER(CORINFO_HELP_CHKCASTTYPETOTYPE, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS_SPECIAL, NULL, CORINFO_HELP_SIG_REG_ONLY) + DYNAMICJITHELPER(CORINFO_HELP_ISASSIGNABLE, NULL, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_ISINSTANCEOF_EXCEPTION, JIT_IsInstanceOfException, CORINFO_HELP_SIG_REG_ONLY) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 2319ecee3a658f..8e17de0c34d980 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -3035,7 +3035,7 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, GenTree* toType = impTokenToHandle(pResolvedToken); GenTree* fromType = impTokenToHandle(&isInstResolvedToken); - impPushOnStack(gtNewHelperCallNode(CORINFO_HELP_CHKCASTTYPETOTYPE, TYP_INT, toType, + impPushOnStack(gtNewHelperCallNode(CORINFO_HELP_ISASSIGNABLE, TYP_INT, toType, fromType), typeInfo(TYP_INT)); return returnToken; diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 1c7c32131fa14f..4a8c0d921e2f48 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -307,10 +307,6 @@ public static unsafe object CheckCastAny(MethodTable* pTargetType, object obj) return objRet; } - [RuntimeExport("RhTypeCast_CheckCastTypeToType")] - public static unsafe int CheckCastTypeToType(MethodTable* fromTypeMT, MethodTable* toTypeMT) - => AreTypesAssignable(fromTypeMT, toTypeMT) ? 1 : 0; - [RuntimeExport("RhTypeCast_CheckCastInterface")] public static unsafe object CheckCastInterface(MethodTable* pTargetType, object obj) { @@ -479,6 +475,10 @@ private static unsafe object CheckCastClassSpecial(MethodTable* pTargetType, obj return ThrowInvalidCastException(pTargetType); } + [RuntimeExport("RhTypeCast_IsAssignable")] + public static unsafe bool IsAssignable(MethodTable* fromTypeMT, MethodTable* toTypeMT) + => AreTypesAssignable(fromTypeMT, toTypeMT); + private static unsafe bool IsInstanceOfInterfaceViaIDynamicInterfaceCastable(MethodTable* pTargetType, object obj, bool throwing) { var pfnIsInterfaceImplemented = (delegate*) diff --git a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs index 4ad3e6eb6b0a3a..d74f5693c71ea9 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs @@ -365,7 +365,7 @@ public enum ReadyToRunHelper GetRuntimeType, - CheckCastTypeToType, + IsAssignable, CheckCastInterface, CheckCastClass, CheckCastClassSpecial, diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs index 628d6ccdd42220..cb33f38651a9a2 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs @@ -79,9 +79,9 @@ which is the right helper to use to allocate an object of a given type. */ CORINFO_HELP_CHKCASTARRAY, CORINFO_HELP_CHKCASTCLASS, CORINFO_HELP_CHKCASTANY, - CORINFO_HELP_CHKCASTTYPETOTYPE, CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check + CORINFO_HELP_ISASSIGNABLE, CORINFO_HELP_ISINSTANCEOF_EXCEPTION, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs index a0f4b6b231d833..eb3afba20d58f1 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs @@ -278,9 +278,6 @@ public static void GetEntryPoint(TypeSystemContext context, ReadyToRunHelper id, case ReadyToRunHelper.CheckCastAny: mangledName = "RhTypeCast_CheckCastAny"; break; - case ReadyToRunHelper.CheckCastTypeToType: - mangledName = "RhTypeCast_CheckCastTypeToType"; - break; case ReadyToRunHelper.CheckCastInterface: mangledName = "RhTypeCast_CheckCastInterface"; break; @@ -290,6 +287,9 @@ public static void GetEntryPoint(TypeSystemContext context, ReadyToRunHelper id, case ReadyToRunHelper.CheckCastClassSpecial: mangledName = "RhTypeCast_CheckCastClassSpecial"; break; + case ReadyToRunHelper.IsAssignable: + mangledName = "RhTypeCast_IsAssignable"; + break; case ReadyToRunHelper.CheckInstanceAny: mangledName = "RhTypeCast_IsInstanceOfAny"; diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 0ae284cf09f11b..a7ad35ae7d7c48 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -728,9 +728,6 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum) case CorInfoHelpFunc.CORINFO_HELP_CHKCASTARRAY: id = ReadyToRunHelper.CheckCastAny; break; - case CorInfoHelpFunc.CORINFO_HELP_CHKCASTTYPETOTYPE: - id = ReadyToRunHelper.CheckCastTypeToType; - break; case CorInfoHelpFunc.CORINFO_HELP_CHKCASTINTERFACE: id = ReadyToRunHelper.CheckCastInterface; break; @@ -740,6 +737,9 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum) case CorInfoHelpFunc.CORINFO_HELP_CHKCASTCLASS_SPECIAL: id = ReadyToRunHelper.CheckCastClassSpecial; break; + case CorInfoHelpFunc.CORINFO_HELP_ISASSIGNABLE: + id = ReadyToRunHelper.IsAssignable; + break; case CorInfoHelpFunc.CORINFO_HELP_ISINSTANCEOFANY: case CorInfoHelpFunc.CORINFO_HELP_ISINSTANCEOFARRAY: diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 3a384b157e3461..bd7efc69f836a9 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1160,8 +1160,8 @@ DEFINE_METHOD(CASTHELPERS, ISINSTANCEOFINTERFACE, IsInstanceOfInterface, SM_Ptr DEFINE_METHOD(CASTHELPERS, CHKCASTANY, ChkCastAny, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTINTERFACE, ChkCastInterface, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASS, ChkCastClass, SM_PtrVoid_Obj_RetObj) -DEFINE_METHOD(CASTHELPERS, CHKCASTTYPETOTYPE, ChkCastTypeToType, SM_PtrVoid_PtrVoid_RetInt) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_PtrVoid_Obj_RetObj) +DEFINE_METHOD(CASTHELPERS, ISASSIGNABLE, IsAssignable, SM_PtrVoid_PtrVoid_RetBool) DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte) DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj) diff --git a/src/coreclr/vm/ecall.cpp b/src/coreclr/vm/ecall.cpp index 0b07c8dfe821dd..0f9e8d6d3dd86f 100644 --- a/src/coreclr/vm/ecall.cpp +++ b/src/coreclr/vm/ecall.cpp @@ -121,10 +121,6 @@ void ECall::PopulateManagedHelpers() // array cast uses the "ANY" helper SetJitHelperFunction(CORINFO_HELP_CHKCASTARRAY, pDest); - pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__CHKCASTTYPETOTYPE)); - pDest = pMD->GetMultiCallableAddrOfCode(); - SetJitHelperFunction(CORINFO_HELP_CHKCASTTYPETOTYPE, pDest); - pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__CHKCASTINTERFACE)); pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_CHKCASTINTERFACE, pDest); @@ -137,6 +133,10 @@ void ECall::PopulateManagedHelpers() pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_CHKCASTCLASS_SPECIAL, pDest); + pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__ISASSIGNABLE)); + pDest = pMD->GetMultiCallableAddrOfCode(); + SetJitHelperFunction(CORINFO_HELP_ISASSIGNABLE, pDest); + pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__UNBOX)); pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_UNBOX, pDest); diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index c87a876d699308..c045c9b8f68986 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2166,7 +2166,7 @@ HCIMPL2(LPVOID, ArrayStoreCheck, Object** pElement, PtrArray** pArray) } HCIMPLEND -extern "C" BOOL QCALLTYPE CastHelpers_CanCastTypeToType(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd) +extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignable(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd) { QCALL_CONTRACT; diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 6f99f001423a57..01e99112e323ac 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -247,7 +247,7 @@ extern "C" FCDECL2(Object*, IsInstanceOfAny_NoCacheLookup, CORINFO_CLASS_HANDLE extern "C" FCDECL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj); extern "C" FCDECL3(void, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Object* obj); -extern "C" BOOL QCALLTYPE CastHelpers_CanCastTypeToType(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd); +extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignable(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd); // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index cd2f5a50007a6b..7d46b2b2418b80 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -355,7 +355,7 @@ DEFINE_METASIG(SM(PtrChar_RetVoid, P(u), v)) DEFINE_METASIG(SM(IntPtr_IntPtr_RetIntPtr, I I, I)) DEFINE_METASIG(SM(IntPtr_IntPtr_Int_RetIntPtr, I I i, I)) DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetVoid, P(v) P(v), v)) -DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetInt, P(v) P(v), i)) +DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetBool, P(v) P(v), F)) DEFINE_METASIG(SM(PtrVoid_PtrVoid_UInt_RetVoid, P(v) P(v) K, v)) DEFINE_METASIG(IM(Obj_RetBool, j, F)) DEFINE_METASIG(SM(Obj_RetVoid, j, v)) diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 698b6dd92ea092..59e4841018ce3c 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -80,7 +80,7 @@ static const Entry s_QCall[] = DllImportEntry(ArgIterator_GetNextArgType) DllImportEntry(ArgIterator_GetNextArg) DllImportEntry(ArgIterator_GetNextArg2) - DllImportEntry(CastHelpers_CanCastTypeToType) + DllImportEntry(CastHelpers_AreTypesAssignable) DllImportEntry(CustomAttribute_ParseAttributeUsageAttribute) DllImportEntry(CustomAttribute_CreateCustomAttributeInstance) DllImportEntry(CustomAttribute_CreatePropertyOrFieldData) From 2a4b2c6d31851b94b6c4df1c8982a10891141349 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 16 Jun 2024 14:35:35 -0700 Subject: [PATCH 12/28] Update JIT interface ID --- src/coreclr/inc/jiteeversionguid.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 76a1bbc466d790..969d967404412f 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* e428e66d-5e0e-4320-ad8a-fa5a50f6da07 */ - 0xe428e66d, - 0x5e0e, - 0x4320, - {0xad, 0x8a, 0xfa, 0x5a, 0x50, 0xf6, 0xda, 0x07} +constexpr GUID JITEEVersionIdentifier = { /* 5939fe51-f24a-4480-8cea-1cd50a21e5e8 */ + 0x5939fe51, + 0xf24a, + 0x4480, + {0x8c, 0xea, 0x1c, 0xd5, 0x0a, 0x21, 0xe5, 0xe8} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// From fcf32a7c28f6fe618826b0a0dcc7ad499e7b903a Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 17 Jun 2024 11:58:02 -0700 Subject: [PATCH 13/28] Review feedback. --- .../Runtime/CompilerServices/CastHelpers.cs | 28 ++- src/coreclr/inc/corinfo.h | 2 +- src/coreclr/inc/jithelpers.h | 2 +- src/coreclr/jit/importer.cpp | 2 +- .../src/System/Runtime/TypeCast.cs | 4 - .../Internal/Runtime/ReadyToRunConstants.cs | 2 +- .../Common/JitInterface/CorInfoHelpFunc.cs | 2 +- .../ILCompiler.Compiler/Compiler/JitHelper.cs | 4 +- .../IL/ILImporter.Scanner.cs | 8 +- .../JitInterface/CorInfoImpl.RyuJit.cs | 4 +- src/coreclr/vm/corelib.h | 2 +- src/coreclr/vm/ecall.cpp | 4 +- src/coreclr/vm/jithelpers.cpp | 2 +- src/coreclr/vm/jitinterface.h | 2 +- src/coreclr/vm/qcallentrypoints.cpp | 2 +- .../generics/ByRefLike/InvalidCSharp.il | 162 ++---------------- .../generics/ByRefLike/Validate.cs | 59 ++++++- 17 files changed, 120 insertions(+), 171 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index a4fa81ff0536f2..d42a1708069858 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -25,8 +25,8 @@ internal static unsafe partial class CastHelpers [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CastHelpers_AreTypesAssignable")] - private static partial Interop.BOOL AreTypesAssignable(void* fromTypeHnd, void* toTypeHnd); + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CastHelpers_AreTypesAssignableHelper")] + private static partial Interop.BOOL AreTypesAssignableHelper(void* fromTypeHnd, void* toTypeHnd); // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, @@ -320,8 +320,28 @@ internal static unsafe partial class CastHelpers } [DebuggerHidden] - private static bool IsAssignable(void* fromTypeHnd, void* toTypeHnd) - => AreTypesAssignable(fromTypeHnd, toTypeHnd) != Interop.BOOL.FALSE; + private static bool AreTypesAssignable(void* fromTypeHnd, void* toTypeHnd) + { + if (fromTypeHnd == toTypeHnd) + { + return true; + } + + CastResult result = CastCache.TryGet(s_table!, (nuint)fromTypeHnd, (nuint)toTypeHnd); + if (result == CastResult.CanCast) + { + return true; + } + else if (result == CastResult.CannotCast) + { + return false; + } + else + { + // Result will be cached in the helper. + return AreTypesAssignableHelper(fromTypeHnd, toTypeHnd) != Interop.BOOL.FALSE; + } + } // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 64126259987337..f7c2b7ee57ad3b 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -439,7 +439,7 @@ enum CorInfoHelpFunc CORINFO_HELP_CHKCASTANY, CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check - CORINFO_HELP_ISASSIGNABLE, // Used to check if TypeHandle can be assigned to TypeHandle + CORINFO_HELP_ARETYPESASSIGNABLE, // Used to check if TypeHandle can be assigned to TypeHandle CORINFO_HELP_ISINSTANCEOF_EXCEPTION, diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 4d5b43a549789b..8c41794c21b662 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -100,7 +100,7 @@ DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTANY, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS_SPECIAL, NULL, CORINFO_HELP_SIG_REG_ONLY) - DYNAMICJITHELPER(CORINFO_HELP_ISASSIGNABLE, NULL, CORINFO_HELP_SIG_REG_ONLY) + DYNAMICJITHELPER(CORINFO_HELP_ARETYPESASSIGNABLE, NULL, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_ISINSTANCEOF_EXCEPTION, JIT_IsInstanceOfException, CORINFO_HELP_SIG_REG_ONLY) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 8e17de0c34d980..2b8a4254a36de4 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -3035,7 +3035,7 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, GenTree* toType = impTokenToHandle(pResolvedToken); GenTree* fromType = impTokenToHandle(&isInstResolvedToken); - impPushOnStack(gtNewHelperCallNode(CORINFO_HELP_ISASSIGNABLE, TYP_INT, toType, + impPushOnStack(gtNewHelperCallNode(CORINFO_HELP_ARETYPESASSIGNABLE, TYP_INT, toType, fromType), typeInfo(TYP_INT)); return returnToken; diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 4a8c0d921e2f48..f0725a1b40ac46 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -475,10 +475,6 @@ private static unsafe object CheckCastClassSpecial(MethodTable* pTargetType, obj return ThrowInvalidCastException(pTargetType); } - [RuntimeExport("RhTypeCast_IsAssignable")] - public static unsafe bool IsAssignable(MethodTable* fromTypeMT, MethodTable* toTypeMT) - => AreTypesAssignable(fromTypeMT, toTypeMT); - private static unsafe bool IsInstanceOfInterfaceViaIDynamicInterfaceCastable(MethodTable* pTargetType, object obj, bool throwing) { var pfnIsInterfaceImplemented = (delegate*) diff --git a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs index d74f5693c71ea9..b7c4796e4df167 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs @@ -365,7 +365,7 @@ public enum ReadyToRunHelper GetRuntimeType, - IsAssignable, + AreTypesAssignable, CheckCastInterface, CheckCastClass, CheckCastClassSpecial, diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs index cb33f38651a9a2..8973a747ab9ba7 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs @@ -81,7 +81,7 @@ which is the right helper to use to allocate an object of a given type. */ CORINFO_HELP_CHKCASTANY, CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check - CORINFO_HELP_ISASSIGNABLE, + CORINFO_HELP_ARETYPESASSIGNABLE, CORINFO_HELP_ISINSTANCEOF_EXCEPTION, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs index eb3afba20d58f1..00737af6a94a30 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs @@ -287,8 +287,8 @@ public static void GetEntryPoint(TypeSystemContext context, ReadyToRunHelper id, case ReadyToRunHelper.CheckCastClassSpecial: mangledName = "RhTypeCast_CheckCastClassSpecial"; break; - case ReadyToRunHelper.IsAssignable: - mangledName = "RhTypeCast_IsAssignable"; + case ReadyToRunHelper.AreTypesAssignable: + mangledName = "RhTypeCast_AreTypesAssignable"; break; case ReadyToRunHelper.CheckInstanceAny: diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs index b124fe824333c2..b826b3aa2277fd 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs @@ -1095,7 +1095,6 @@ private void ImportBox(int token) // There are some sequences of box with ByRefLike types that are allowed // per the extension to the ECMA-335 specification. - // Everything else is invalid. if (!type.IsRuntimeDeterminedType && type.IsByRefLike) { ILReader reader = new ILReader(_ilBytes, _currentOffset); @@ -1129,8 +1128,6 @@ private void ImportBox(int token) } } } - - ThrowHelper.ThrowInvalidProgramException(); } AddBoxingDependencies(type, "Box"); @@ -1162,6 +1159,11 @@ private void AddBoxingDependencies(TypeDesc type, string reason) { _dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.Box), reason); } + + if (type.IsByRefLike) + { + _dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.AreTypesAssignable), reason); + } } private void ImportLeave(BasicBlock target) diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index a7ad35ae7d7c48..538928988d3b90 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -737,8 +737,8 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum) case CorInfoHelpFunc.CORINFO_HELP_CHKCASTCLASS_SPECIAL: id = ReadyToRunHelper.CheckCastClassSpecial; break; - case CorInfoHelpFunc.CORINFO_HELP_ISASSIGNABLE: - id = ReadyToRunHelper.IsAssignable; + case CorInfoHelpFunc.CORINFO_HELP_ARETYPESASSIGNABLE: + id = ReadyToRunHelper.AreTypesAssignable; break; case CorInfoHelpFunc.CORINFO_HELP_ISINSTANCEOFANY: diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 4433b4848bf0a0..7746c566e15510 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1161,7 +1161,7 @@ DEFINE_METHOD(CASTHELPERS, CHKCASTANY, ChkCastAny, SM_Ptr DEFINE_METHOD(CASTHELPERS, CHKCASTINTERFACE, ChkCastInterface, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASS, ChkCastClass, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_PtrVoid_Obj_RetObj) -DEFINE_METHOD(CASTHELPERS, ISASSIGNABLE, IsAssignable, SM_PtrVoid_PtrVoid_RetBool) +DEFINE_METHOD(CASTHELPERS, ARETYPESASSIGNABLE, AreTypesAssignable, SM_PtrVoid_PtrVoid_RetBool) DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte) DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj) diff --git a/src/coreclr/vm/ecall.cpp b/src/coreclr/vm/ecall.cpp index 0f9e8d6d3dd86f..a384f8e7dc2e50 100644 --- a/src/coreclr/vm/ecall.cpp +++ b/src/coreclr/vm/ecall.cpp @@ -133,9 +133,9 @@ void ECall::PopulateManagedHelpers() pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_CHKCASTCLASS_SPECIAL, pDest); - pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__ISASSIGNABLE)); + pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__ARETYPESASSIGNABLE)); pDest = pMD->GetMultiCallableAddrOfCode(); - SetJitHelperFunction(CORINFO_HELP_ISASSIGNABLE, pDest); + SetJitHelperFunction(CORINFO_HELP_ARETYPESASSIGNABLE, pDest); pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__UNBOX)); pDest = pMD->GetMultiCallableAddrOfCode(); diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index eae5c28df29244..2e96843f2f3caa 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2141,7 +2141,7 @@ HCIMPL3(Object*, JIT_NewMDArr, CORINFO_CLASS_HANDLE classHnd, unsigned dwNumArgs } HCIMPLEND -extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignable(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd) +extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignableHelper(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd) { QCALL_CONTRACT; diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 4d648670f9a2ee..26d856a500dc6d 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -247,7 +247,7 @@ extern "C" FCDECL2(Object*, IsInstanceOfAny_NoCacheLookup, CORINFO_CLASS_HANDLE extern "C" FCDECL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj); extern "C" FCDECL3(void, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Object* obj); -extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignable(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd); +extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignableHelper(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd); // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 59e4841018ce3c..d57667a2db3740 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -80,7 +80,7 @@ static const Entry s_QCall[] = DllImportEntry(ArgIterator_GetNextArgType) DllImportEntry(ArgIterator_GetNextArg) DllImportEntry(ArgIterator_GetNextArg2) - DllImportEntry(CastHelpers_AreTypesAssignable) + DllImportEntry(CastHelpers_AreTypesAssignableHelper) DllImportEntry(CustomAttribute_ParseAttributeUsageAttribute) DllImportEntry(CustomAttribute_CreateCustomAttributeInstance) DllImportEntry(CustomAttribute_CreatePropertyOrFieldData) diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index f946d0261f99b3..01950377c2724e 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -358,34 +358,6 @@ ret } - .method public hidebysig - instance bool BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!U) cil managed - { - .locals init ( - [0] valuetype ByRefLikeTypeWithInterface - ) - - ldarg.1 - box !!U - isinst ByRefLikeTypeWithInterface - brfalse.s NOT_BYREFLIKETYPEWITHINTERFACE - - ldarg.1 - box !!U - isinst ByRefLikeTypeWithInterface - unbox.any ByRefLikeTypeWithInterface - stloc.0 - - ldloca.s 0 - constrained. ByRefLikeTypeWithInterface - callvirt instance int32 InvalidCSharp.SimpleInterface::Method() - ret - - NOT_BYREFLIKETYPEWITHINTERFACE: - ldc.i4.0 - ret - } - .method public hidebysig instance bool AllocArrayOfT() cil managed aggressiveinlining { @@ -886,33 +858,12 @@ ret } - .method private hidebysig static - bool CheckIsInstance(!!T) cil managed noinlining - { - ldarg.0 - // Begin sequence - box !!T - isinst !!U - brfalse.s IS_FALSE - // End sequence - - ldc.i4.1 - ret - - IS_FALSE: - ldc.i4.0 - ret - } - .method public hidebysig static - void BoxIsinstBranch() cil managed + void BoxIsinstBranchVarious() cil managed { .locals init ( [0] valuetype InvalidCSharp.GenericByRefLike_Over`1, - [1] valuetype ByRefLikeType, - [2] valuetype ByRefLikeTypeWithInterface, - [3] valuetype ByRefLikeType`1, - [4] valuetype ByRefLikeType`1 + [1] valuetype ByRefLikeTypeWithInterface ) ldloca.s 0 @@ -941,9 +892,9 @@ NEXT_2: ldloca.s 0 - ldloca.s 2 + ldloca.s 1 initobj valuetype ByRefLikeTypeWithInterface - ldloc.2 + ldloc.1 call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: BoxIsinstBranch_CheckForSimpleInterface(!!0) brtrue.s NEXT_3 @@ -964,101 +915,24 @@ throw NEXT_4: - ldloca.s 0 - ldloca.s 2 - initobj valuetype ByRefLikeTypeWithInterface - ldloc.2 - call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: - BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!0) - brtrue.s NEXT_5 - - ldstr "Failed: BoxIsinstBranch_CheckForByRefLikeTypeWithInterface for ByRefLikeTypeWithInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw - - NEXT_5: - ldloca.s 0 - newobj instance void InvalidCSharp.ClassWithInterface::.ctor() - call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: - BoxIsinstBranch_CheckForByRefLikeTypeWithInterface(!!0) - brfalse.s NEXT_6 - - ldstr "The above is expected to be false since we are not passing a ByRefLikeTypeWithInterface instance." - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw - - NEXT_6: - ldloca.s 1 - initobj valuetype ByRefLikeType - ldloc.1 - call bool Exec::CheckIsInstance(!!0) - brfalse.s NEXT_7 - - ldstr "Failed: CheckIsInstance for ByRefLikeType and SimpleInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw - - NEXT_7: - ldloca.s 2 - initobj valuetype ByRefLikeTypeWithInterface - ldloc.2 - call bool Exec::CheckIsInstance(!!0) - brtrue.s NEXT_8 - - ldstr "Failed: CheckIsInstance for ByRefLikeTypeWithInterface and SimpleInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw - - NEXT_8: - newobj instance void InvalidCSharp.ClassWithInterface::.ctor() - call bool Exec::CheckIsInstance(!!0) - brtrue.s NEXT_9 - - ldstr "Failed: CheckIsInstance for ClassWithInterface and SimpleInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw - - NEXT_9: - ldstr "EMPTY" - call bool Exec::CheckIsInstance(!!0) - brfalse.s NEXT_10 + ret + } - ldstr "Failed: CheckIsInstance for string and SimpleInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw + .method public hidebysig static + bool BoxIsinstBranch(!!T) cil managed noinlining + { + ldarg.0 + // Begin sequence + box !!T + isinst !!U + brfalse.s IS_FALSE + // End sequence - NEXT_10: ldc.i4.1 - call bool Exec::CheckIsInstance(!!0) - brfalse.s NEXT_11 - - ldstr "Failed: CheckIsInstance for int32 and SimpleInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw - - NEXT_11: - ldloca.s 3 - initobj valuetype ByRefLikeType`1 - ldloc.3 - call bool Exec::CheckIsInstance, class InvalidCSharp.SimpleInterface>(!!0) - brfalse.s NEXT_12 - - ldstr "Failed: CheckIsInstance for valuetype ByRefLikeType`1 and SimpleInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw - - NEXT_12: - ldloca.s 4 - initobj valuetype ByRefLikeType`1 - ldloc 4 - call bool Exec::CheckIsInstance, class InvalidCSharp.SimpleInterface>(!!0) - brfalse.s NEXT_13 - - ldstr "Failed: CheckIsInstance for valuetype ByRefLikeType`1 and SimpleInterface" - newobj instance void [System.Runtime]System.Exception::.ctor(string) - throw + ret - NEXT_13: + IS_FALSE: + ldc.i4.0 ret } diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 91042e97404370..13b806a56d07f7 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -56,6 +56,18 @@ public static void Validate_Casting_Scenarios() Assert.Throws(() => { Exec.UnboxToT(new object()); }); } + interface I1 { } + + struct S {} + struct S {} + struct S_I1 : I1 {} + struct S_I1 : I1 {} + + ref struct RS { } + ref struct RS { } + ref struct RS_I1 : I1 { } + ref struct RS_I1 : I1 { } + [Fact] public static void Validate_RecognizedOpCodeSequences_Scenarios() { @@ -64,7 +76,52 @@ public static void Validate_RecognizedOpCodeSequences_Scenarios() Exec.BoxUnboxAny(); Exec.BoxBranch(); Exec.BoxIsinstUnboxAny(); - Exec.BoxIsinstBranch(); + + Exec.BoxIsinstBranchVarious(); + + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.False(Exec.BoxIsinstBranch(default)); + Assert.False(Exec.BoxIsinstBranch(default)); + + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, S>(default)); + Assert.True(Exec.BoxIsinstBranch, S>(default)); + Assert.False(Exec.BoxIsinstBranch(default)); + Assert.False(Exec.BoxIsinstBranch, I1>(default)); + Assert.False(Exec.BoxIsinstBranch, I1>(default)); + + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, S_I1>(default)); + Assert.True(Exec.BoxIsinstBranch, S_I1>(default)); + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, I1>(default)); + Assert.True(Exec.BoxIsinstBranch, I1>(default)); + + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, RS>(default)); + Assert.True(Exec.BoxIsinstBranch, RS>(default)); + Assert.False(Exec.BoxIsinstBranch(default)); + Assert.False(Exec.BoxIsinstBranch, I1>(default)); + Assert.False(Exec.BoxIsinstBranch, I1>(default)); + + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch, object>(default)); + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, RS_I1>(default)); + Assert.True(Exec.BoxIsinstBranch, RS_I1>(default)); + Assert.True(Exec.BoxIsinstBranch(default)); + Assert.True(Exec.BoxIsinstBranch, I1>(default)); + Assert.True(Exec.BoxIsinstBranch, I1>(default)); } [Fact] From 634c207ed22b104c60eea2747d7502b8efe3858a Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 17 Jun 2024 12:03:52 -0700 Subject: [PATCH 14/28] Update spec. --- docs/design/features/byreflike-generics.md | 11 ++++++++++- docs/design/specs/Ecma-335-Augments.md | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index ea95ca1f56f53e..0c4e3c92049203 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -124,7 +124,7 @@ The following are IL sequences involving the `box` instruction. They are used fo `box` ; `isinst` ; `unbox.any` – Becomes a NOP, if the box, `isinst`, and unbox target types are all equal. -`box` ; `isinst` ; `br_true/false` – Becomes a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. The sequence will also be elided if the box target type is a ByRefLike type, but needs to be checked at run-time, not JIT compile time. This latter case is common when the box target type uses Generic parameters. +`box` ; `isinst` ; `br_true/false` – Can become a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. This sequence can be interpreted as behaving as if the ByRefLike nature of the input type doesn't exist. ## Examples @@ -250,4 +250,13 @@ class B : A // not aware they could be calling B. override void M(); } +``` + +**8) Valid** +```csharp +class A +{ + public bool M(T t) where T: allows ref struct + => t is U; +} ``` \ No newline at end of file diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 7bcf723d05f41f..1331d26b005663 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -1055,7 +1055,7 @@ The following are IL sequences involving the `box` instruction. They are used fo `box` ; `isinst` ; `unbox.any` – Becomes a NOP, if the box, `isinst`, and unbox target types are all equal. -`box` ; `isinst` ; `br_true/false` – Becomes a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. The sequence will also be elided if the box target type is a ByRefLike type, but needs to be checked at run-time, not JIT compile time. This latter case is common when the box target type uses Generic parameters. +`box` ; `isinst` ; `br_true/false` – Can become a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. This sequence can be interpreted as behaving as if the ByRefLike nature of the input type doesn't exist. ## Rules for IL Rewriters From 65e6c878744f84dde2b902bf2e157df6ebc64666 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 21 Jun 2024 15:20:59 -0700 Subject: [PATCH 15/28] Updates --- docs/design/features/byreflike-generics.md | 107 +++++++++++++++--- docs/design/specs/Ecma-335-Augments.md | 14 --- .../generics/ByRefLike/InvalidCSharp.il | 72 +++++++++++- .../generics/ByRefLike/Validate.cs | 33 +++++- 4 files changed, 192 insertions(+), 34 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 0c4e3c92049203..bb4f6953ab1f42 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -24,7 +24,7 @@ The following instructions are already set up to support this feature since thei - `isinst` - `castclass` -**NOTE** There are sequences involving some of the above instructions that will remain valid regardless of a `T` being ByRefLike—see ["Special IL Sequences" section](#special_il_sequences) below for details. +**NOTE** There are sequences involving some of the above instructions that may remain valid regardless of a `T` being ByRefLike—see ["Options for invalid IL" section](#invalid_il_options) below for details. The expansion of ByRefLike types as Generic parameters does not relax restrictions on where ByRefLike types can be used. When `T` is ByRefLike, the use of `T` as a field will require the enclosing type to be ByRefLike. @@ -114,21 +114,103 @@ Adding `gpAcceptByRefLike` to the metadata of a Generic parameter will be consid Enumerating of constructors/methods on `Span` and `ReadOnlySpan` may throw `TypeLoadException` if `T` is a ByRefLike type. See "Troublesome APIs" above for the list of APIs that cause this condition. -## Special IL Sequences +## Options for invalid IL -The following are IL sequences involving the `box` instruction. They are used for common C# language constructs and shall continue to be valid, even with ByRefLike types, in cases where the result can be computed at JIT compile time or interpretation and elided safely. These sequences **must** now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below and each condition will be added to the ECMA-335 addendum. +There are two potential options below for how to address this issue. -`box` ; `unbox.any` – Becomes a NOP, if the box target type is equal to the unboxed target type. +The first indented IL sequences below represents the `is-type` sequence. Combining the first with the second indented section represents the "type pattern matching" scenario in C#. The below sequence performs a type check and then, if successful, consumes the unboxed instance. -`box` ; `br_true/false` – Becomes the constant `true`, if the box target type is non-`Nullable`. +```IL +// Type check +ldarg.0 + box + isinst + brfalse.s NOT_INST -`box` ; `isinst` ; `unbox.any` – Becomes a NOP, if the box, `isinst`, and unbox target types are all equal. +// Unbox and store unboxed instance +ldarg.0 + box + isinst + unbox.any +stloc.X -`box` ; `isinst` ; `br_true/false` – Can become a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. This sequence can be interpreted as behaving as if the ByRefLike nature of the input type doesn't exist. +NOT_INST: +ret +``` + +With the above IL composition implemented, the following C# describes the following "type pattern matching" scenarios and what one might expect given current C# semantics. + +```csharp +struct S {} +ref struct RS {} +interface I {} +class C {} +class C {} + +// Not currently valid C# +void M(T t) where T: allows ref struct +{ + if (t is int i) // Valid + if (t is S vc) // Valid + if (t is RS rs) // Valid + if (t is Span s) // Valid + if (t is string str) // Valid + if (t is I itf) // Valid + if (t is C c) // Valid + if (t is C ci) // Valid + if (t is C cu) // Invalid + if (t is U u) // Invalid + if (t is Span su) // Invalid +} +``` + +### Option 1) Compiler helpers + +The following two helper functions could be introduced and would replace currently invalid IL sequences. Their behavior would broadly be defined to operate as if the ByRefLike aspect of either the `TFrom` and `TTo` is not present. + +```csharp +namespace System.Runtime.CompilerServices +{ + public static class RuntimeHelpers + { + // Replacement for the [box; isinst; brfalse/true] sequence. + public static bool IsInstanceOf(TFrom source) + where TFrom: allows ref struct + where TTo: allows ref struct; + + // Replacement for the [box; isinst; unbox.any] sequence. + public static TTo CastTo(TFrom source) + where TFrom: allows ref struct + where TTo: allows ref struct; + } +} +``` + +Example usage of the above methods. + +```csharp +TTo result; +if (RuntimeHelpers.IsInstanceOf(source)) +{ + result = RuntimeHelpers.CastTo(source); +} +``` + +### Option 2) Special IL sequences + +The following are IL sequences involving the `box` instruction. They are used for common C# language constructs and would continue to be valid, even with ByRefLike types. These sequences would be **required** to be valid when the target type is ByRefLike. Each sequence would be added to the ECMA-335 addendum. + +`box` ; `isinst` ; `br_true/false` – Passing a ByRefLike type as the argument to the `box` instruction is permitted to accomplish a type check, in C# `x is Y`. **Note** ByRefLike types would evaluate to `true` when compared against `System.Object`. + +`box` ; `isinst` ; `unbox.any` – In order to permit "type pattern matching", in C# `x is Y y`, this sequence will permit use of a ByRefLike type on any instruction, but does not permit the use of generic parameters being exposed to `isinst` or `unbox.any`. + +`box` ; `unbox.any` – Valid to use ByRefLike types. + +`box` ; `br_true/false` – Valid to use ByRefLike types. ## Examples -Below are valid and invalid examples of ByRefLike as Generic parameters. +Below are currently (.NET 9) valid and invalid examples of ByRefLike as Generic parameters. **1) Valid** ```csharp @@ -250,13 +332,4 @@ class B : A // not aware they could be calling B. override void M(); } -``` - -**8) Valid** -```csharp -class A -{ - public bool M(T t) where T: allows ref struct - => t is U; -} ``` \ No newline at end of file diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 1331d26b005663..171fb9876d6295 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -1043,20 +1043,6 @@ Update the `SpecialConstraintMask` flag value and description, and add a new fla | ... | ... | ... | | `AllowByRefLike` | `0x20` | The generic parameter is allowed to be ByRefLike | -### III.4 -New sub-section should be added after III.4.33 that describes sequences of IL instructions that can be used on ByRefLike types when using within a generic context. - -#### III.4.X -The following are IL sequences involving the `box` instruction. They are used for ByRefLike types and shall be valid in cases where the result can be computed at run-time and elided safely—through JIT compilation or interpretation. These sequences **must** now be elided when the target type is ByRefLike. The conditions where each sequence is elided are described below. - -`box` ; `unbox.any` – Becomes a NOP, if the box target type is equal to the unboxed target type. - -`box` ; `br_true/false` – Becomes the constant `true`, if the box target type is non-`Nullable`. - -`box` ; `isinst` ; `unbox.any` – Becomes a NOP, if the box, `isinst`, and unbox target types are all equal. - -`box` ; `isinst` ; `br_true/false` – Can become a constant, if the box target type is ByRefLike or the box target type is `Nullable` and target type equalities are computed to be equal. This sequence can be interpreted as behaving as if the ByRefLike nature of the input type doesn't exist. - ## Rules for IL Rewriters There are apis such as `System.Runtime.CompilerServices.RuntimeHelpers.CreateSpan(...)` which require that the PE file have a particular structure. In particular, that api requires that the associated RVA of a FieldDef which is used to create a span must be naturally aligned over the data type that `CreateSpan` is instantiated over. There are 2 major concerns. diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index 01950377c2724e..5796fca6fcf26b 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -230,7 +230,7 @@ // Begin sequence box !T isinst ByRefLikeType - unbox.any !T + unbox.any ByRefLikeType // End sequence pop ldc.i4.1 @@ -819,6 +819,76 @@ ret } + .method public hidebysig static + bool BoxIsinstBranchCapture(!!T) cil managed + { + .locals init ( + [0] int32, + [1] valuetype [System.Runtime]System.Span`1, + [2] string, + [3] !!U + ) + + // if (t is int i) + ldarg.0 + box !!T + isinst int32 + brfalse.s NEXT_1 + ldarg.0 + box !!T + isinst int32 + unbox.any int32 + stloc.0 + ldc.i4.1 + ret + + NEXT_1: + // if (t is Span s) + ldarg.0 + box !!T + isinst valuetype [System.Runtime]System.Span`1 + brfalse.s NEXT_2 + ldarg.0 + box !!T + isinst valuetype [System.Runtime]System.Span`1 + unbox.any valuetype [System.Runtime]System.Span`1 + stloc.1 + ldc.i4.1 + ret + + NEXT_2: + // if (t is string s) + ldarg.0 + box !!T + isinst string + brfalse.s NEXT_3 + ldarg.0 + box !!T + isinst string + unbox.any string + stloc.2 + ldc.i4.1 + ret + + NEXT_3: + // if (t is U u) + ldarg.0 + box !!T + isinst !!U + brfalse.s NEXT_4 + ldarg.0 + box !!T + isinst !!U + unbox.any !!U + stloc 3 + ldc.i4.1 + ret + + NEXT_4: + ldc.i4.0 + ret + } + .method public hidebysig static void BoxIsinstUnboxAny() cil managed { diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 13b806a56d07f7..b26a67185dc16b 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -68,10 +68,12 @@ ref struct RS { } ref struct RS_I1 : I1 { } ref struct RS_I1 : I1 { } + sealed class Ignored { } + [Fact] - public static void Validate_RecognizedOpCodeSequences_Scenarios() + public static void Validate_RecognizedOpCodeSequences() { - Console.WriteLine($"{nameof(Validate_RecognizedOpCodeSequences_Scenarios)}..."); + Console.WriteLine($"{nameof(Validate_RecognizedOpCodeSequences)}..."); Exec.BoxUnboxAny(); Exec.BoxBranch(); @@ -124,6 +126,33 @@ public static void Validate_RecognizedOpCodeSequences_Scenarios() Assert.True(Exec.BoxIsinstBranch, I1>(default)); } + [Fact] + public static void Validate_RecognizedOpCodeSequences_TypeCheckAndLocal() + { + Console.WriteLine($"{nameof(Validate_RecognizedOpCodeSequences_TypeCheckAndLocal)}..."); + + // Hard coded local variables in function + Assert.True(Exec.BoxIsinstBranchCapture(1)); + Assert.True(Exec.BoxIsinstBranchCapture, Ignored>(new Span())); + Assert.True(Exec.BoxIsinstBranchCapture(string.Empty)); + + // Using generic parameter local. + Assert.False(Exec.BoxIsinstBranchCapture(default)); + Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); + Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); + Assert.True(Exec.BoxIsinstBranchCapture(default)); + Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); + Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); + + // These are not support due to issues with "box; isinst; box.any" + // Assert.False(Exec.BoxIsinstBranchCapture(default)); + // Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); + // Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); + // Assert.True(Exec.BoxIsinstBranchCapture(default)); + // Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); + // Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); + } + [Fact] public static void Validate_RecognizedOpCodeSequences_Mismatch() { From 3eccef8b91e0b25a5a4169135edd0b3c4b5d638b Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 24 Jun 2024 14:32:31 -0700 Subject: [PATCH 16/28] Updates --- docs/design/features/byreflike-generics.md | 40 ++++++++++++++++------ 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index bb4f6953ab1f42..e65bbd2723076b 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -142,7 +142,9 @@ With the above IL composition implemented, the following C# describes the follow ```csharp struct S {} +struct S {} ref struct RS {} +ref struct RS {} interface I {} class C {} class C {} @@ -150,17 +152,29 @@ class C {} // Not currently valid C# void M(T t) where T: allows ref struct { - if (t is int i) // Valid - if (t is S vc) // Valid - if (t is RS rs) // Valid - if (t is Span s) // Valid - if (t is string str) // Valid - if (t is I itf) // Valid - if (t is C c) // Valid - if (t is C ci) // Valid - if (t is C cu) // Invalid - if (t is U u) // Invalid - if (t is Span su) // Invalid + // Valid + if (t is int i) + + if (t is S s) + if (t is S sc) + if (t is S su) + + if (t is RS rs) + if (t is RS rsc) + if (t is RS rsu) + + if (t is string str) + if (t is C c) + if (t is C ci) + if (t is C cu) + + // Can be made to work in IL. + if (t is I itf) // A new local "I" would not be used for ByRefLike scenarios. + // The local would be the ByRefLike type, not "I". + + // Invalid + if (t is object o) // ByRefLike types evaluate "true" for object. + if (t is U u) } ``` @@ -179,6 +193,10 @@ namespace System.Runtime.CompilerServices where TTo: allows ref struct; // Replacement for the [box; isinst; unbox.any] sequence. + // Would throw InvalidCastException for invalid use at run-time. + // For example: + // TFrom: RS, TTo: object => always throws + // TFrom: RS, TTo: => always throws public static TTo CastTo(TFrom source) where TFrom: allows ref struct where TTo: allows ref struct; From c43cddd369275ec92dff01d769df6bfd95edbfa5 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 28 Jun 2024 20:29:52 -0700 Subject: [PATCH 17/28] Updates --- docs/design/features/byreflike-generics.md | 4 +- .../Runtime/CompilerServices/CastHelpers.cs | 29 +------- src/coreclr/inc/corinfo.h | 1 - src/coreclr/inc/jiteeversionguid.h | 10 +-- src/coreclr/inc/jithelpers.h | 1 - src/coreclr/jit/importer.cpp | 19 ----- .../Internal/Runtime/ReadyToRunConstants.cs | 1 - .../Common/JitInterface/CorInfoHelpFunc.cs | 1 - .../ILCompiler.Compiler/Compiler/JitHelper.cs | 3 - .../IL/ILImporter.Scanner.cs | 8 +-- .../JitInterface/CorInfoImpl.RyuJit.cs | 3 - src/coreclr/vm/corelib.h | 1 - src/coreclr/vm/ecall.cpp | 4 -- src/coreclr/vm/jithelpers.cpp | 15 ---- src/coreclr/vm/jitinterface.h | 3 - src/coreclr/vm/metasig.h | 1 - src/coreclr/vm/qcallentrypoints.cpp | 1 - .../generics/ByRefLike/InvalidCSharp.il | 70 ------------------- .../generics/ByRefLike/Validate.cs | 47 ------------- 19 files changed, 11 insertions(+), 211 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index e65bbd2723076b..89d30be4842ea5 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -112,7 +112,7 @@ throw Adding `gpAcceptByRefLike` to the metadata of a Generic parameter will be considered a non-breaking binary change. -Enumerating of constructors/methods on `Span` and `ReadOnlySpan` may throw `TypeLoadException` if `T` is a ByRefLike type. See "Troublesome APIs" above for the list of APIs that cause this condition. +Enumerating of constructors/methods on `Span` and `ReadOnlySpan` may throw `TypeLoadException` if `T` is a ByRefLike type. See "Troublesome API mitigation" above for the list of APIs that cause this condition. ## Options for invalid IL @@ -180,7 +180,7 @@ void M(T t) where T: allows ref struct ### Option 1) Compiler helpers -The following two helper functions could be introduced and would replace currently invalid IL sequences. Their behavior would broadly be defined to operate as if the ByRefLike aspect of either the `TFrom` and `TTo` is not present. +The following two helper functions could be introduced and would replace currently invalid IL sequences. Their behavior would broadly be defined to operate as if the ByRefLike aspect of either the `TFrom` and `TTo` is not present. An alternative approach would be consult with the Roslyn team and define the semantics of these functions to adhere to C# language rules. ```csharp namespace System.Runtime.CompilerServices diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index d42a1708069858..c2333f3854eedb 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -8,7 +8,7 @@ namespace System.Runtime.CompilerServices { [StackTraceHidden] [DebuggerStepThrough] - internal static unsafe partial class CastHelpers + internal static unsafe class CastHelpers { // In coreclr the table is allocated and written to on the native side. internal static int[]? s_table; @@ -25,9 +25,6 @@ internal static unsafe partial class CastHelpers [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CastHelpers_AreTypesAssignableHelper")] - private static partial Interop.BOOL AreTypesAssignableHelper(void* fromTypeHnd, void* toTypeHnd); - // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, // this test must deal with all kinds of type tests @@ -319,30 +316,6 @@ internal static unsafe partial class CastHelpers return ChkCastClassSpecial(toTypeHnd, obj); } - [DebuggerHidden] - private static bool AreTypesAssignable(void* fromTypeHnd, void* toTypeHnd) - { - if (fromTypeHnd == toTypeHnd) - { - return true; - } - - CastResult result = CastCache.TryGet(s_table!, (nuint)fromTypeHnd, (nuint)toTypeHnd); - if (result == CastResult.CanCast) - { - return true; - } - else if (result == CastResult.CannotCast) - { - return false; - } - else - { - // Result will be cached in the helper. - return AreTypesAssignableHelper(fromTypeHnd, toTypeHnd) != Interop.BOOL.FALSE; - } - } - // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check [DebuggerHidden] diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index f7c2b7ee57ad3b..01571b78b7c296 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -439,7 +439,6 @@ enum CorInfoHelpFunc CORINFO_HELP_CHKCASTANY, CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check - CORINFO_HELP_ARETYPESASSIGNABLE, // Used to check if TypeHandle can be assigned to TypeHandle CORINFO_HELP_ISINSTANCEOF_EXCEPTION, diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 969d967404412f..76a1bbc466d790 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 5939fe51-f24a-4480-8cea-1cd50a21e5e8 */ - 0x5939fe51, - 0xf24a, - 0x4480, - {0x8c, 0xea, 0x1c, 0xd5, 0x0a, 0x21, 0xe5, 0xe8} +constexpr GUID JITEEVersionIdentifier = { /* e428e66d-5e0e-4320-ad8a-fa5a50f6da07 */ + 0xe428e66d, + 0x5e0e, + 0x4320, + {0xad, 0x8a, 0xfa, 0x5a, 0x50, 0xf6, 0xda, 0x07} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 8c41794c21b662..50beaabe66e903 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -100,7 +100,6 @@ DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTANY, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS_SPECIAL, NULL, CORINFO_HELP_SIG_REG_ONLY) - DYNAMICJITHELPER(CORINFO_HELP_ARETYPESASSIGNABLE, NULL, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_ISINSTANCEOF_EXCEPTION, JIT_IsInstanceOfException, CORINFO_HELP_SIG_REG_ONLY) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 2b8a4254a36de4..05dcb9ad10490d 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -3021,25 +3021,6 @@ int Compiler::impBoxPatternMatch(CORINFO_RESOLVED_TOKEN* pResolvedToken, typeInfo(TYP_INT)); return returnToken; } - - // Casts that result to TypeCompareState::May and involve a ByRefLike type, - // must be checked at run-time. - if (opts == BoxPatterns::IsByRefLike) - { - assert(castResult == TypeCompareState::May); - JITDUMP( - "\n Importing BOX; ISINST; BR_TRUE/FALSE for ByRefLike, must be checked at run-time\n"); - - impSpillSideEffects(false, CHECK_SPILL_ALL DEBUGARG("spilling side-effects")); - impPopStack(); - - GenTree* toType = impTokenToHandle(pResolvedToken); - GenTree* fromType = impTokenToHandle(&isInstResolvedToken); - impPushOnStack(gtNewHelperCallNode(CORINFO_HELP_ARETYPESASSIGNABLE, TYP_INT, toType, - fromType), - typeInfo(TYP_INT)); - return returnToken; - } } else if ((foldAsHelper == CORINFO_HELP_BOX_NULLABLE) && ((impStackTop().val->gtFlags & GTF_SIDE_EFFECT) == 0)) diff --git a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs index b7c4796e4df167..af86b107e5b31e 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs @@ -365,7 +365,6 @@ public enum ReadyToRunHelper GetRuntimeType, - AreTypesAssignable, CheckCastInterface, CheckCastClass, CheckCastClassSpecial, diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs index 8973a747ab9ba7..47d202909bce34 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs @@ -81,7 +81,6 @@ which is the right helper to use to allocate an object of a given type. */ CORINFO_HELP_CHKCASTANY, CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check - CORINFO_HELP_ARETYPESASSIGNABLE, CORINFO_HELP_ISINSTANCEOF_EXCEPTION, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs index 00737af6a94a30..c04658df379f7a 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs @@ -287,9 +287,6 @@ public static void GetEntryPoint(TypeSystemContext context, ReadyToRunHelper id, case ReadyToRunHelper.CheckCastClassSpecial: mangledName = "RhTypeCast_CheckCastClassSpecial"; break; - case ReadyToRunHelper.AreTypesAssignable: - mangledName = "RhTypeCast_AreTypesAssignable"; - break; case ReadyToRunHelper.CheckInstanceAny: mangledName = "RhTypeCast_IsInstanceOfAny"; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs index b826b3aa2277fd..b124fe824333c2 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs @@ -1095,6 +1095,7 @@ private void ImportBox(int token) // There are some sequences of box with ByRefLike types that are allowed // per the extension to the ECMA-335 specification. + // Everything else is invalid. if (!type.IsRuntimeDeterminedType && type.IsByRefLike) { ILReader reader = new ILReader(_ilBytes, _currentOffset); @@ -1128,6 +1129,8 @@ private void ImportBox(int token) } } } + + ThrowHelper.ThrowInvalidProgramException(); } AddBoxingDependencies(type, "Box"); @@ -1159,11 +1162,6 @@ private void AddBoxingDependencies(TypeDesc type, string reason) { _dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.Box), reason); } - - if (type.IsByRefLike) - { - _dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.AreTypesAssignable), reason); - } } private void ImportLeave(BasicBlock target) diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 538928988d3b90..46e3b1af0e4628 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -737,9 +737,6 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum) case CorInfoHelpFunc.CORINFO_HELP_CHKCASTCLASS_SPECIAL: id = ReadyToRunHelper.CheckCastClassSpecial; break; - case CorInfoHelpFunc.CORINFO_HELP_ARETYPESASSIGNABLE: - id = ReadyToRunHelper.AreTypesAssignable; - break; case CorInfoHelpFunc.CORINFO_HELP_ISINSTANCEOFANY: case CorInfoHelpFunc.CORINFO_HELP_ISINSTANCEOFARRAY: diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 7746c566e15510..2e1a7e970563f7 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1161,7 +1161,6 @@ DEFINE_METHOD(CASTHELPERS, CHKCASTANY, ChkCastAny, SM_Ptr DEFINE_METHOD(CASTHELPERS, CHKCASTINTERFACE, ChkCastInterface, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASS, ChkCastClass, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_PtrVoid_Obj_RetObj) -DEFINE_METHOD(CASTHELPERS, ARETYPESASSIGNABLE, AreTypesAssignable, SM_PtrVoid_PtrVoid_RetBool) DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte) DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj) diff --git a/src/coreclr/vm/ecall.cpp b/src/coreclr/vm/ecall.cpp index a384f8e7dc2e50..d3e8415d6adeea 100644 --- a/src/coreclr/vm/ecall.cpp +++ b/src/coreclr/vm/ecall.cpp @@ -133,10 +133,6 @@ void ECall::PopulateManagedHelpers() pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_CHKCASTCLASS_SPECIAL, pDest); - pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__ARETYPESASSIGNABLE)); - pDest = pMD->GetMultiCallableAddrOfCode(); - SetJitHelperFunction(CORINFO_HELP_ARETYPESASSIGNABLE, pDest); - pMD = CoreLibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__UNBOX)); pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_UNBOX, pDest); diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 2e96843f2f3caa..66d46b10c0c9e3 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2141,21 +2141,6 @@ HCIMPL3(Object*, JIT_NewMDArr, CORINFO_CLASS_HANDLE classHnd, unsigned dwNumArgs } HCIMPLEND -extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignableHelper(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd) -{ - QCALL_CONTRACT; - - BOOL ret = FALSE; - - BEGIN_QCALL; - TypeHandle fromType(fromTypeHnd); - TypeHandle toType(toTypeHnd); - ret = fromType.CanCastTo(toType); - END_QCALL; - - return ret; -} - #include //======================================================================== diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 26d856a500dc6d..ee84883a9a584a 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -18,7 +18,6 @@ #define MAX_UNCHECKED_OFFSET_FOR_NULL_OBJECT ((GetOsPageSize() / 2) - 1) #endif // !TARGET_UNIX #include "pgo.h" -#include "qcall.h" enum StompWriteBarrierCompletionAction { @@ -247,8 +246,6 @@ extern "C" FCDECL2(Object*, IsInstanceOfAny_NoCacheLookup, CORINFO_CLASS_HANDLE extern "C" FCDECL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj); extern "C" FCDECL3(void, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Object* obj); -extern "C" BOOL QCALLTYPE CastHelpers_AreTypesAssignableHelper(CORINFO_CLASS_HANDLE fromTypeHnd, CORINFO_CLASS_HANDLE toTypeHnd); - // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location extern "C" FCDECL2(VOID, JIT_WriteBarrier_Callable, Object **dst, Object *ref); diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 7d46b2b2418b80..69eae039076f8b 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -355,7 +355,6 @@ DEFINE_METASIG(SM(PtrChar_RetVoid, P(u), v)) DEFINE_METASIG(SM(IntPtr_IntPtr_RetIntPtr, I I, I)) DEFINE_METASIG(SM(IntPtr_IntPtr_Int_RetIntPtr, I I i, I)) DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetVoid, P(v) P(v), v)) -DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetBool, P(v) P(v), F)) DEFINE_METASIG(SM(PtrVoid_PtrVoid_UInt_RetVoid, P(v) P(v) K, v)) DEFINE_METASIG(IM(Obj_RetBool, j, F)) DEFINE_METASIG(SM(Obj_RetVoid, j, v)) diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index d57667a2db3740..e76ecb82fdbd38 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -80,7 +80,6 @@ static const Entry s_QCall[] = DllImportEntry(ArgIterator_GetNextArgType) DllImportEntry(ArgIterator_GetNextArg) DllImportEntry(ArgIterator_GetNextArg2) - DllImportEntry(CastHelpers_AreTypesAssignableHelper) DllImportEntry(CustomAttribute_ParseAttributeUsageAttribute) DllImportEntry(CustomAttribute_CreateCustomAttributeInstance) DllImportEntry(CustomAttribute_CreatePropertyOrFieldData) diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index 5796fca6fcf26b..36fd12c3248ac1 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -819,76 +819,6 @@ ret } - .method public hidebysig static - bool BoxIsinstBranchCapture(!!T) cil managed - { - .locals init ( - [0] int32, - [1] valuetype [System.Runtime]System.Span`1, - [2] string, - [3] !!U - ) - - // if (t is int i) - ldarg.0 - box !!T - isinst int32 - brfalse.s NEXT_1 - ldarg.0 - box !!T - isinst int32 - unbox.any int32 - stloc.0 - ldc.i4.1 - ret - - NEXT_1: - // if (t is Span s) - ldarg.0 - box !!T - isinst valuetype [System.Runtime]System.Span`1 - brfalse.s NEXT_2 - ldarg.0 - box !!T - isinst valuetype [System.Runtime]System.Span`1 - unbox.any valuetype [System.Runtime]System.Span`1 - stloc.1 - ldc.i4.1 - ret - - NEXT_2: - // if (t is string s) - ldarg.0 - box !!T - isinst string - brfalse.s NEXT_3 - ldarg.0 - box !!T - isinst string - unbox.any string - stloc.2 - ldc.i4.1 - ret - - NEXT_3: - // if (t is U u) - ldarg.0 - box !!T - isinst !!U - brfalse.s NEXT_4 - ldarg.0 - box !!T - isinst !!U - unbox.any !!U - stloc 3 - ldc.i4.1 - ret - - NEXT_4: - ldc.i4.0 - ret - } - .method public hidebysig static void BoxIsinstUnboxAny() cil managed { diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index b26a67185dc16b..511471782a58f6 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -104,53 +104,6 @@ public static void Validate_RecognizedOpCodeSequences() Assert.True(Exec.BoxIsinstBranch(default)); Assert.True(Exec.BoxIsinstBranch, I1>(default)); Assert.True(Exec.BoxIsinstBranch, I1>(default)); - - Assert.True(Exec.BoxIsinstBranch(default)); - Assert.True(Exec.BoxIsinstBranch, object>(default)); - Assert.True(Exec.BoxIsinstBranch, object>(default)); - Assert.True(Exec.BoxIsinstBranch(default)); - Assert.True(Exec.BoxIsinstBranch, RS>(default)); - Assert.True(Exec.BoxIsinstBranch, RS>(default)); - Assert.False(Exec.BoxIsinstBranch(default)); - Assert.False(Exec.BoxIsinstBranch, I1>(default)); - Assert.False(Exec.BoxIsinstBranch, I1>(default)); - - Assert.True(Exec.BoxIsinstBranch(default)); - Assert.True(Exec.BoxIsinstBranch, object>(default)); - Assert.True(Exec.BoxIsinstBranch, object>(default)); - Assert.True(Exec.BoxIsinstBranch(default)); - Assert.True(Exec.BoxIsinstBranch, RS_I1>(default)); - Assert.True(Exec.BoxIsinstBranch, RS_I1>(default)); - Assert.True(Exec.BoxIsinstBranch(default)); - Assert.True(Exec.BoxIsinstBranch, I1>(default)); - Assert.True(Exec.BoxIsinstBranch, I1>(default)); - } - - [Fact] - public static void Validate_RecognizedOpCodeSequences_TypeCheckAndLocal() - { - Console.WriteLine($"{nameof(Validate_RecognizedOpCodeSequences_TypeCheckAndLocal)}..."); - - // Hard coded local variables in function - Assert.True(Exec.BoxIsinstBranchCapture(1)); - Assert.True(Exec.BoxIsinstBranchCapture, Ignored>(new Span())); - Assert.True(Exec.BoxIsinstBranchCapture(string.Empty)); - - // Using generic parameter local. - Assert.False(Exec.BoxIsinstBranchCapture(default)); - Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); - Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); - Assert.True(Exec.BoxIsinstBranchCapture(default)); - Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); - Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); - - // These are not support due to issues with "box; isinst; box.any" - // Assert.False(Exec.BoxIsinstBranchCapture(default)); - // Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); - // Assert.False(Exec.BoxIsinstBranchCapture, I1>(default)); - // Assert.True(Exec.BoxIsinstBranchCapture(default)); - // Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); - // Assert.True(Exec.BoxIsinstBranchCapture, I1>(default)); } [Fact] From 391c7f62199cf61b14349a751df85997e9c57dc5 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 1 Jul 2024 10:24:21 -0700 Subject: [PATCH 18/28] Updates --- docs/design/features/byreflike-generics.md | 7 +- .../generics/ByRefLike/InvalidCSharp.il | 73 ++++++------------- .../generics/ByRefLike/Validate.cs | 7 +- 3 files changed, 31 insertions(+), 56 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 89d30be4842ea5..1526f65ac8a6b1 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -9,13 +9,14 @@ Using ByRefLike types in Generic parameters is possible by building upon support Supporting ByRefLike type as Generic parameters will impact the following IL instructions: +Throws `InvalidProgramException`: - `box` – Types with ByRefLike parameters used in fields cannot be boxed. +- `constrained.callvirt` – When applied to a ByRefLike instance, if this IL sequence resolves to a method implemented on `object` or a default interface method, an error will occur during the attempt to box the ByRefLike instance. + +Throws `TypeLoadException`: - `stsfld` / `ldsfld` – Type fields of a ByRefLike parameter cannot be marked `static`. - `newarr` / `stelem` / `ldelem` / `ldelema` – Arrays are not able to contain ByRefLike types. - `newobj` – For multi-dimensional array construction. -- `constrained.callvirt` – When applied to a ByRefLike instance, if this IL sequence resolves to a method implemented on `object` or a default interface method, an error will occur during the attempt to box the ByRefLike instance. - -If any of the above instructions are attempted to be used with a ByRefLike type, the runtime will throw an `InvalidProgramException`. The following instructions are already set up to support this feature since their behavior will fail as currently defined due to the inability to box a ByRefLike type. diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index 36fd12c3248ac1..f6b138fe7e016b 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -93,14 +93,6 @@ ret } - .method public hidebysig - instance object BoxAsObject(!T) cil managed - { - ldarg.1 - box !T - ret - } - .method public hidebysig instance bool BoxUnboxAny(!T) cil managed { @@ -368,17 +360,6 @@ ret } - .method public hidebysig - instance bool AllocMultiDimArrayOfT() cil managed - { - ldc.i4.1 - ldc.i4.1 - newobj instance void !T[0..., 0...]::.ctor(int32, int32) - ldnull - cgt.un - ret - } - .method public hidebysig instance bool InstanceOfT( object o @@ -724,18 +705,10 @@ } .method public hidebysig static - object BoxAsObject() cil managed + object BoxAsObject(!!Y) cil managed noinlining { - .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 - ) - - ldloca.s 0 - initobj valuetype InvalidCSharp.GenericByRefLike_Over`1 - ldloca.s 0 - ldloc 0 - ldfld !0 valuetype InvalidCSharp.GenericByRefLike_Over`1::Field - call instance object valuetype InvalidCSharp.GenericByRefLike_Over`1::BoxAsObject(!0) + ldarg.0 + box !!Y ret } @@ -937,32 +910,23 @@ } .method public hidebysig static - void AllocArrayOfT_Invalid() cil managed + bool AllocArray() cil managed noinlining { - .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 - ) - - ldloca.s 0 - initobj valuetype InvalidCSharp.GenericByRefLike_Over`1 - ldloca.s 0 - call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::AllocArrayOfT() - pop + ldc.i4.1 + newarr !!Y + ldnull + cgt.un ret } .method public hidebysig static - void AllocMultiDimArrayOfT_Invalid() cil managed + bool AllocMultiDimArray() cil managed noinlining { - .locals init ( - [0] valuetype InvalidCSharp.GenericByRefLike_Over`1 - ) - - ldloca.s 0 - initobj valuetype InvalidCSharp.GenericByRefLike_Over`1 - ldloca.s 0 - call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1::AllocMultiDimArrayOfT() - pop + ldc.i4.1 + ldc.i4.1 + newobj instance void !!Y[0..., 0...]::.ctor(int32, int32) + ldnull + cgt.un ret } @@ -975,6 +939,15 @@ ret } + .method public hidebysig static + string CallStringOnObject(!!T t) cil managed noinlining + { + ldarga.s 0 + constrained. !!T + callvirt instance string [System.Runtime]System.Object::ToString() + ret + } + .method public hidebysig static bool InstanceOfT(object) cil managed { diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 511471782a58f6..74150c04857cac 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -124,12 +124,13 @@ public static void Validate_InvalidOpCode_Scenarios() // These methods uses opcodes that are not able to handle ByRefLike type operands. // The TypeLoader prevents these invalid types from being constructed. We rely on // the failure to construct these invalid types to block opcode usage. - Assert.Throws(() => { Exec.AllocArrayOfT_Invalid(); }); - Assert.Throws(() => { Exec.AllocMultiDimArrayOfT_Invalid(); }); + Assert.Throws(() => { Exec.AllocArray(); }); + Assert.Throws(() => { Exec.AllocMultiDimArray(); }); Assert.Throws(() => { Exec.GenericClassWithStaticField_Invalid(); }); // Test that explicitly tries to box a ByRefLike type. - Assert.Throws(() => { Exec.BoxAsObject(); }); + Assert.Throws(() => { Exec.BoxAsObject(new RS()); }); + Assert.Throws(() => { Exec.CallStringOnObject(new RS()); }); } [Fact] From 84141d5a05194e917c9c3b69fbb068920e4c5f2c Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 8 Jul 2024 13:30:33 -0700 Subject: [PATCH 19/28] Feedback --- docs/design/features/byreflike-generics.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 1526f65ac8a6b1..cacaf7c639e803 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -7,13 +7,13 @@ Using ByRefLike types in Generic parameters is possible by building upon support ## Runtime impact -Supporting ByRefLike type as Generic parameters will impact the following IL instructions: +Supporting ByRefLike type as Generic parameters will impact the following IL instructions. -Throws `InvalidProgramException`: -- `box` – Types with ByRefLike parameters used in fields cannot be boxed. -- `constrained.callvirt` – When applied to a ByRefLike instance, if this IL sequence resolves to a method implemented on `object` or a default interface method, an error will occur during the attempt to box the ByRefLike instance. +When used with a ByRefLike type these instructions result in undefined behavior: +- `box` – When applied to ByRefLike types used in fields cannot be boxed. +- `constrained.callvirt` – If this IL sequence resolves to a method implemented on `object` or a default interface method. -Throws `TypeLoadException`: +Throws `TypeLoadException` when pass a ByRefLike type. - `stsfld` / `ldsfld` – Type fields of a ByRefLike parameter cannot be marked `static`. - `newarr` / `stelem` / `ldelem` / `ldelema` – Arrays are not able to contain ByRefLike types. - `newobj` – For multi-dimensional array construction. From 879733ad0dbe696a57080bba207b2b5e2c951de1 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Jul 2024 12:25:22 -0700 Subject: [PATCH 20/28] Update docs/design/features/byreflike-generics.md Co-authored-by: Jan Kotas --- docs/design/features/byreflike-generics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index cacaf7c639e803..8791a047bc4901 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -9,7 +9,7 @@ Using ByRefLike types in Generic parameters is possible by building upon support Supporting ByRefLike type as Generic parameters will impact the following IL instructions. -When used with a ByRefLike type these instructions result in undefined behavior: +When used with a ByRefLike type these instructions result in undefined behavior (the method containing the IL instruction may fail to execute partially or in its entirety): - `box` – When applied to ByRefLike types used in fields cannot be boxed. - `constrained.callvirt` – If this IL sequence resolves to a method implemented on `object` or a default interface method. From e01d4572fbd5d8f7cbdb99f111a90130685fa470 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 10 Jul 2024 16:15:35 -0700 Subject: [PATCH 21/28] Update tests --- .../generics/ByRefLike/InvalidCSharp.il | 67 +++++++++++++++++-- .../generics/ByRefLike/Validate.cs | 20 +++++- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il index f6b138fe7e016b..1bfb367c6d0006 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il +++ b/src/tests/Loader/classloader/generics/ByRefLike/InvalidCSharp.il @@ -483,12 +483,47 @@ } } +.class interface public auto ansi abstract InvalidCSharp.DefaultInterface +{ + .method public hidebysig newslot virtual + instance int32 Method() cil managed + { + ldc.i4.0 + ret + } +} + +.class public sequential ansi sealed beforefieldinit RS_DI1 + extends [System.Runtime]System.ValueType + implements InvalidCSharp.DefaultInterface +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( + 01 00 00 00 + ) +} + +.class public sequential ansi sealed beforefieldinit RS_DI2 + extends [System.Runtime]System.ValueType + implements InvalidCSharp.DefaultInterface +{ + .custom instance void [System.Runtime]System.Runtime.CompilerServices.IsByRefLikeAttribute::.ctor() = ( + 01 00 00 00 + ) + + .method public hidebysig newslot virtual + instance int32 Method() cil managed + { + ldc.i4.1 + ret + } +} + .class public sequential ansi sealed beforefieldinit RegularValueType extends [System.Runtime]System.ValueType { } -.class public auto ansi beforefieldinit InvalidCSharp.ClassWithInterface +.class public auto ansi beforefieldinit ClassWithInterface extends [System.Runtime]System.Object implements InvalidCSharp.SimpleInterface { @@ -878,9 +913,9 @@ NEXT_3: ldloca.s 0 - newobj instance void InvalidCSharp.ClassWithInterface::.ctor() + newobj instance void ClassWithInterface::.ctor() call instance bool valuetype InvalidCSharp.GenericByRefLike_Over`1:: - BoxIsinstBranch_CheckForSimpleInterface(!!0) + BoxIsinstBranch_CheckForSimpleInterface(!!0) brfalse.s NEXT_4 ldstr "The above is expected to be false since ClassWithInterface's interface implementation return 0." @@ -940,7 +975,7 @@ } .method public hidebysig static - string CallStringOnObject(!!T t) cil managed noinlining + string ConstrainedCallVirtToString(!!T t) cil managed noinlining { ldarga.s 0 constrained. !!T @@ -948,6 +983,30 @@ ret } + .method public hidebysig static + int32 ConstrainedCallVirtMethod(!!T t) cil managed noinlining + { + ldarga.s 0 + constrained. !!T + callvirt instance int32 InvalidCSharp.DefaultInterface::Method() + ret + } + + .method public hidebysig static + int32 ConstrainedCallVirtMethod(!!T t, bool skipCall) cil managed noinlining + { + ldarg.1 + brfalse.s CALL + ldc.i4.m1 + ret + + CALL: + ldarga.s 0 + constrained. !!T + callvirt instance int32 InvalidCSharp.DefaultInterface::Method() + ret + } + .method public hidebysig static bool InstanceOfT(object) cil managed { diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 74150c04857cac..b96d59a6086bfe 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -62,11 +62,15 @@ struct S {} struct S {} struct S_I1 : I1 {} struct S_I1 : I1 {} + struct S_DI1 : InvalidCSharp.DefaultInterface {} + struct S_DI2 : InvalidCSharp.DefaultInterface { public int Method() => 1; } ref struct RS { } ref struct RS { } ref struct RS_I1 : I1 { } ref struct RS_I1 : I1 { } + // ref struct RS_DI1 - See InvalidCSharp.il + // ref struct RS_DI2 - See InvalidCSharp.il sealed class Ignored { } @@ -104,6 +108,15 @@ public static void Validate_RecognizedOpCodeSequences() Assert.True(Exec.BoxIsinstBranch(default)); Assert.True(Exec.BoxIsinstBranch, I1>(default)); Assert.True(Exec.BoxIsinstBranch, I1>(default)); + + Assert.Equal($"{nameof(Validate)}+{nameof(S)}", Exec.ConstrainedCallVirtToString(new S())); + Assert.Equal(0, Exec.ConstrainedCallVirtMethod(new S_DI1())); + Assert.Equal(1, Exec.ConstrainedCallVirtMethod(new S_DI2())); + Assert.Equal(1, Exec.ConstrainedCallVirtMethod(new RS_DI2())); + + Assert.Equal(-1, Exec.ConstrainedCallVirtMethod(new S_DI1(), skipCall: true)); + Assert.Equal(-1, Exec.ConstrainedCallVirtMethod(new S_DI2(), skipCall: true)); + Assert.Equal(-1, Exec.ConstrainedCallVirtMethod(new RS_DI2(), skipCall: true)); } [Fact] @@ -130,7 +143,12 @@ public static void Validate_InvalidOpCode_Scenarios() // Test that explicitly tries to box a ByRefLike type. Assert.Throws(() => { Exec.BoxAsObject(new RS()); }); - Assert.Throws(() => { Exec.CallStringOnObject(new RS()); }); + + // Test that implicitly tries to box a ByRefLike type. + Assert.Throws(() => { Exec.ConstrainedCallVirtToString(new RS()); }); + Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1()); }); + Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1(), skipCall: false); }); + Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1(), skipCall: true); }); } [Fact] From 6c8daf209dc96403e68e51956c6ede8d8f3a2a1e Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 12 Jul 2024 11:47:02 -0700 Subject: [PATCH 22/28] Remove the statement of "undefined behavior". --- docs/design/features/byreflike-generics.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 8791a047bc4901..5c82cac166225f 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -9,8 +9,8 @@ Using ByRefLike types in Generic parameters is possible by building upon support Supporting ByRefLike type as Generic parameters will impact the following IL instructions. -When used with a ByRefLike type these instructions result in undefined behavior (the method containing the IL instruction may fail to execute partially or in its entirety): -- `box` – When applied to ByRefLike types used in fields cannot be boxed. +Providing a ByRefLike type with these instructions remains invalid and `InvalidProgramException` will be thrown as described below. +- `box` – When applied to ByRefLike types. - `constrained.callvirt` – If this IL sequence resolves to a method implemented on `object` or a default interface method. Throws `TypeLoadException` when pass a ByRefLike type. From 20dda6217e005de2d6d0c8ccf14e0d5ea96cfda6 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 16 Jul 2024 15:37:39 -0700 Subject: [PATCH 23/28] Updates --- docs/design/features/byreflike-generics.md | 6 +++--- docs/design/specs/Ecma-335-Augments.md | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 5c82cac166225f..4cbc9cd43abcb2 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -9,9 +9,9 @@ Using ByRefLike types in Generic parameters is possible by building upon support Supporting ByRefLike type as Generic parameters will impact the following IL instructions. -Providing a ByRefLike type with these instructions remains invalid and `InvalidProgramException` will be thrown as described below. -- `box` – When applied to ByRefLike types. -- `constrained.callvirt` – If this IL sequence resolves to a method implemented on `object` or a default interface method. +Providing a ByRefLike type to the `box` instructions remains invalid and `InvalidProgramException` will be thrown when detected. + +The `constrained.callvirt` sequence is valid if a ByRefLike type is provided. A `NotSupportedException` will be thrown at the callsite, if the target resolves to a method implemented on `object` or a default interface method. Throws `TypeLoadException` when pass a ByRefLike type. - `stsfld` / `ldsfld` – Type fields of a ByRefLike parameter cannot be marked `static`. diff --git a/docs/design/specs/Ecma-335-Augments.md b/docs/design/specs/Ecma-335-Augments.md index 171fb9876d6295..fd8a0fd93ca87f 100644 --- a/docs/design/specs/Ecma-335-Augments.md +++ b/docs/design/specs/Ecma-335-Augments.md @@ -1043,6 +1043,15 @@ Update the `SpecialConstraintMask` flag value and description, and add a new fla | ... | ... | ... | | `AllowByRefLike` | `0x20` | The generic parameter is allowed to be ByRefLike | +### III.2.1 +The following case is added as the **third** cases in the "if _thisType_" sequence. + +> If _thisType_ is ByRefLike and _thisType_ does not implement _method_ then; a `NotSupportedException` is thrown at the callsite. + +The following is added to the paragraph starting with "This last case can only occur when _method_ was defined on `System.Object`, `System.ValueType`, or `System.Enum`". + +> The third case can only occur when _method_ was defined on `System.Object` or is a Default Interface Method. + ## Rules for IL Rewriters There are apis such as `System.Runtime.CompilerServices.RuntimeHelpers.CreateSpan(...)` which require that the PE file have a particular structure. In particular, that api requires that the associated RVA of a FieldDef which is used to create a span must be naturally aligned over the data type that `CreateSpan` is instantiated over. There are 2 major concerns. From be202d9cccb1899edcb40212cb0c1b9ddd16edde Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 17 Jul 2024 11:39:35 -0700 Subject: [PATCH 24/28] Updates --- docs/design/features/byreflike-generics.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 4cbc9cd43abcb2..dff9d3a11ecdb3 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -7,13 +7,13 @@ Using ByRefLike types in Generic parameters is possible by building upon support ## Runtime impact -Supporting ByRefLike type as Generic parameters will impact the following IL instructions. +Supporting ByRefLike types as Generic parameters will impact the following IL instructions. -Providing a ByRefLike type to the `box` instructions remains invalid and `InvalidProgramException` will be thrown when detected. +Providing a ByRefLike type to the `box` instruction remains invalid and `InvalidProgramException` will be thrown when detected. The `constrained.callvirt` sequence is valid if a ByRefLike type is provided. A `NotSupportedException` will be thrown at the callsite, if the target resolves to a method implemented on `object` or a default interface method. -Throws `TypeLoadException` when pass a ByRefLike type. +Throws `TypeLoadException` when passed a ByRefLike type. - `stsfld` / `ldsfld` – Type fields of a ByRefLike parameter cannot be marked `static`. - `newarr` / `stelem` / `ldelem` / `ldelema` – Arrays are not able to contain ByRefLike types. - `newobj` – For multi-dimensional array construction. @@ -125,14 +125,14 @@ The first indented IL sequences below represents the `is-type` sequence. Combini // Type check ldarg.0 box - isinst + isinst brfalse.s NOT_INST // Unbox and store unboxed instance ldarg.0 box - isinst - unbox.any + isinst + unbox.any stloc.X NOT_INST: @@ -181,7 +181,7 @@ void M(T t) where T: allows ref struct ### Option 1) Compiler helpers -The following two helper functions could be introduced and would replace currently invalid IL sequences. Their behavior would broadly be defined to operate as if the ByRefLike aspect of either the `TFrom` and `TTo` is not present. An alternative approach would be consult with the Roslyn team and define the semantics of these functions to adhere to C# language rules. +The following two helper functions could be introduced and would replace currently invalid `is-type` IL sequences when ByRefLike types are involved. Their behavior would broadly be defined to operate as if the ByRefLike aspect of either the `TFrom` and `TTo` is not present. An alternative approach would be consult with the Roslyn team and define the semantics of these functions to adhere to C# language rules. ```csharp namespace System.Runtime.CompilerServices From cc78df48b7ef9fd3f5394c7c89f768d31a71fa8e Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 23 Jul 2024 22:17:40 -0700 Subject: [PATCH 25/28] Apply suggestions from code review Co-authored-by: Jan Kotas --- docs/design/features/byreflike-generics.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index dff9d3a11ecdb3..17abedfd27166c 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -9,9 +9,10 @@ Using ByRefLike types in Generic parameters is possible by building upon support Supporting ByRefLike types as Generic parameters will impact the following IL instructions. -Providing a ByRefLike type to the `box` instruction remains invalid and `InvalidProgramException` will be thrown when detected. +The `constrained. callvirt` sequence is valid if a ByRefLike type is provided. A `NotSupportedException` will be thrown at the callsite, if the target resolves to a method implemented on `object` or a default interface method. -The `constrained.callvirt` sequence is valid if a ByRefLike type is provided. A `NotSupportedException` will be thrown at the callsite, if the target resolves to a method implemented on `object` or a default interface method. +Throws `InvalidProgramException` when passed a ByRefLike type: +- `box` – ByRefLike types cannot be allocated on the heap. Throws `TypeLoadException` when passed a ByRefLike type. - `stsfld` / `ldsfld` – Type fields of a ByRefLike parameter cannot be marked `static`. From 3472ff98e4733c9b1893ee2c53c964303f6daf29 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 24 Jul 2024 07:21:42 -0700 Subject: [PATCH 26/28] Update docs/design/features/byreflike-generics.md --- docs/design/features/byreflike-generics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index 17abedfd27166c..fe284bfe359df1 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -14,7 +14,7 @@ The `constrained. callvirt` sequence is valid if a ByRefLike type is provided. A Throws `InvalidProgramException` when passed a ByRefLike type: - `box` – ByRefLike types cannot be allocated on the heap. -Throws `TypeLoadException` when passed a ByRefLike type. +Throws `TypeLoadException` when passed a ByRefLike type: - `stsfld` / `ldsfld` – Type fields of a ByRefLike parameter cannot be marked `static`. - `newarr` / `stelem` / `ldelem` / `ldelema` – Arrays are not able to contain ByRefLike types. - `newobj` – For multi-dimensional array construction. From 6936b11493c82bfad3eb84735000073536ebf1fd Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 24 Jul 2024 15:42:59 -0700 Subject: [PATCH 27/28] Feedback. --- docs/design/features/byreflike-generics.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/design/features/byreflike-generics.md b/docs/design/features/byreflike-generics.md index fe284bfe359df1..4bccfa85ef1b17 100644 --- a/docs/design/features/byreflike-generics.md +++ b/docs/design/features/byreflike-generics.md @@ -9,7 +9,7 @@ Using ByRefLike types in Generic parameters is possible by building upon support Supporting ByRefLike types as Generic parameters will impact the following IL instructions. -The `constrained. callvirt` sequence is valid if a ByRefLike type is provided. A `NotSupportedException` will be thrown at the callsite, if the target resolves to a method implemented on `object` or a default interface method. +The `constrained. callvirt` sequence is valid if a ByRefLike type is provided. A `NotSupportedException` will be thrown at the call-site, if the target resolves to a method implemented on `object` or a default interface method. Throws `InvalidProgramException` when passed a ByRefLike type: - `box` – ByRefLike types cannot be allocated on the heap. @@ -118,7 +118,7 @@ Enumerating of constructors/methods on `Span` and `ReadOnlySpan` may throw ## Options for invalid IL -There are two potential options below for how to address this issue. +There are two potential options below for how to address this issue. Based on communication with the Roslyn team, option (1) is the current plan of record for .NET 10. The first indented IL sequences below represents the `is-type` sequence. Combining the first with the second indented section represents the "type pattern matching" scenario in C#. The below sequence performs a type check and then, if successful, consumes the unboxed instance. From 90dfc5f6ce5ed6a0c26cf8436103257bf5e63582 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 25 Jul 2024 10:26:19 -0700 Subject: [PATCH 28/28] Updates --- .../Loader/classloader/generics/ByRefLike/Validate.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index b96d59a6086bfe..ced2fda3c95dbb 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -83,7 +83,7 @@ public static void Validate_RecognizedOpCodeSequences() Exec.BoxBranch(); Exec.BoxIsinstUnboxAny(); - Exec.BoxIsinstBranchVarious(); + // Exec.BoxIsinstBranchVarious(); Assert.True(Exec.BoxIsinstBranch(default)); Assert.False(Exec.BoxIsinstBranch(default)); @@ -145,10 +145,10 @@ public static void Validate_InvalidOpCode_Scenarios() Assert.Throws(() => { Exec.BoxAsObject(new RS()); }); // Test that implicitly tries to box a ByRefLike type. - Assert.Throws(() => { Exec.ConstrainedCallVirtToString(new RS()); }); - Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1()); }); - Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1(), skipCall: false); }); - Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1(), skipCall: true); }); + // Assert.Throws(() => { Exec.ConstrainedCallVirtToString(new RS()); }); + // Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1()); }); + // Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1(), skipCall: false); }); + // Assert.Throws(() => { Exec.ConstrainedCallVirtMethod(new RS_DI1(), skipCall: true); }); } [Fact]