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 d529f18f140df..0a19e23d42642 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 f6b8f9b964eff..7b4b0123f2d32 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 0a19e23d42642..b626e39e93ce6 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 7b4b0123f2d32..0bc5ec73c1b0d 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 0bc5ec73c1b0d..9006e5da98bc8 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 b626e39e93ce6..9562b0b2e71a9 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 9006e5da98bc8..c18f0c7013d5e 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 02a9037ca10ae..9e2580b9300a9 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 280a567f2deb8..5b304e7956804 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 9562b0b2e71a9..ea95ca1f56f53 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 c18f0c7013d5e..7bcf723d05f41 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 0c640dd1c2f0a..5d4e5c542f55c 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 db5499b81fd33..517d5751df978 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 ec89b618b909b..257d6c3899c57 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 a98a564d403b9..4ae7a1e6a97d8 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 0161e8c47c150..1c7c32131fa14 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 af86b107e5b31..4ad3e6eb6b0a3 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 14bf90a9aaaed..07b1b1b85232b 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 c04658df379f7..a0f4b6b231d83 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 46e3b1af0e462..0ae284cf09f11 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 df42e52eab952..7a4cd8c417f1d 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 d3e8415d6adee..0b07c8dfe821d 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 13fbeaab2332d..e11448950b07d 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 51e2b959d6f69..3f42574717216 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 35e6472b4b8da..bc741689dd5f1 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 2c719586aed98..2fc22bc0ae797 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 9e2580b9300a9..f946d0261f99b 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 5b304e7956804..91042e9740437 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 35a7e623b9a9b..969d967404412 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 6bcb13b5a9265..969d967404412 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 67ac284dfe09b..2319ecee3a658 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 e0017a77c861a..33a959d7bfb15 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 00cdab659e511..68605f1df043c 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 304e77b465202..4d5b43a549789 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 2319ecee3a658..8e17de0c34d98 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 1c7c32131fa14..4a8c0d921e2f4 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 4ad3e6eb6b0a3..d74f5693c71ea 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 628d6ccdd4222..cb33f38651a9a 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 a0f4b6b231d83..eb3afba20d58f 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 0ae284cf09f11..a7ad35ae7d7c4 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 3a384b157e346..bd7efc69f836a 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 0b07c8dfe821d..0f9e8d6d3dd86 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 c87a876d69930..c045c9b8f6898 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 6f99f001423a5..01e99112e323a 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 cd2f5a50007a6..7d46b2b2418b8 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 698b6dd92ea09..59e4841018ce3 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 76a1bbc466d79..969d967404412 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 a4fa81ff0536f..d42a170806985 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 6412625998733..f7c2b7ee57ad3 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 4d5b43a549789..8c41794c21b66 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 8e17de0c34d98..2b8a4254a36de 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 4a8c0d921e2f4..f0725a1b40ac4 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 d74f5693c71ea..b7c4796e4df16 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 cb33f38651a9a..8973a747ab9ba 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 eb3afba20d58f..00737af6a94a3 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 b124fe824333c..b826b3aa2277f 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 a7ad35ae7d7c4..538928988d3b9 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 4433b4848bf0a..7746c566e1551 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 0f9e8d6d3dd86..a384f8e7dc2e5 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 eae5c28df2924..2e96843f2f3ca 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 4d648670f9a2e..26d856a500dc6 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 59e4841018ce3..d57667a2db374 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 f946d0261f99b..01950377c2724 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 91042e9740437..13b806a56d07f 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 ea95ca1f56f53..0c4e3c9204920 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 7bcf723d05f41..1331d26b00566 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 0c4e3c9204920..bb4f6953ab1f4 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 1331d26b00566..171fb9876d629 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 01950377c2724..5796fca6fcf26 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 13b806a56d07f..b26a67185dc16 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 bb4f6953ab1f4..e65bbd2723076 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 e65bbd2723076..89d30be4842ea 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 d42a170806985..c2333f3854eed 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 f7c2b7ee57ad3..01571b78b7c29 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 969d967404412..76a1bbc466d79 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 8c41794c21b66..50beaabe66e90 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 2b8a4254a36de..05dcb9ad10490 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 b7c4796e4df16..af86b107e5b31 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 8973a747ab9ba..47d202909bce3 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 00737af6a94a3..c04658df379f7 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 b826b3aa2277f..b124fe824333c 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 538928988d3b9..46e3b1af0e462 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 7746c566e1551..2e1a7e970563f 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 a384f8e7dc2e5..d3e8415d6adee 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 2e96843f2f3ca..66d46b10c0c9e 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 26d856a500dc6..ee84883a9a584 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 7d46b2b2418b8..69eae039076f8 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 d57667a2db374..e76ecb82fdbd3 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 5796fca6fcf26..36fd12c3248ac 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 b26a67185dc16..511471782a58f 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 89d30be4842ea..1526f65ac8a6b 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 36fd12c3248ac..f6b138fe7e016 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 511471782a58f..74150c04857ca 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 1526f65ac8a6b..cacaf7c639e80 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 cacaf7c639e80..8791a047bc490 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 f6b138fe7e016..1bfb367c6d000 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 74150c04857ca..b96d59a6086bf 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 8791a047bc490..5c82cac166225 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 5c82cac166225..4cbc9cd43abcb 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 171fb9876d629..fd8a0fd93ca87 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 4cbc9cd43abcb..dff9d3a11ecdb 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 dff9d3a11ecdb..17abedfd27166 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 17abedfd27166..fe284bfe359df 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 fe284bfe359df..4bccfa85ef1b1 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 b96d59a6086bf..ced2fda3c95db 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]