From 807b896475b862b038f49c98a0c22af6eff6390b Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 29 Oct 2021 23:02:22 +0300 Subject: [PATCH 1/2] [interp] Fix GetType called on ptr constrained to Nullable` We were statically optimizing this call to return the actual constrained class type, which is incorrect for nullables, because boxing of a nullable (as part of the constrained call) actually creates an object with the type of the nullable's value (or null if there is no value). --- src/mono/mono/mini/interp/transform.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index fd5138dafdc95..ab05967d0dc4a 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2440,7 +2440,7 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas if (!strcmp (tm, "InternalGetHashCode")) { *op = MINT_INTRINS_GET_HASHCODE; } else if (!strcmp (tm, "GetType")) { - if (constrained_class && m_class_is_valuetype (constrained_class)) { + if (constrained_class && m_class_is_valuetype (constrained_class) && !mono_class_is_nullable (constrained_class)) { // If constrained_class is valuetype we already know its type. // Resolve GetType to a constant so we can fold type comparisons ERROR_DECL(error); @@ -2457,8 +2457,16 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas return TRUE; } else { if (constrained_class) { - // deref the managed pointer to get the object - interp_add_ins (td, MINT_LDIND_I); + if (mono_class_is_nullable (constrained_class)) { + // We can't determine the behavior here statically because we don't know if the + // nullable vt has a value or not. If it has a value, the result type is + // m_class_get_cast_class (constrained_class), otherwise GetType should throw NRE. + interp_add_ins (td, MINT_BOX_NULLABLE_PTR); + td->last_ins->data [0] = get_data_item_index (td, constrained_class); + } else { + // deref the managed pointer to get the object + interp_add_ins (td, MINT_LDIND_I); + } td->sp--; interp_ins_set_sreg (td->last_ins, td->sp [0].local); push_simple_type (td, STACK_TYPE_O); From 92ee61398fdd238ecfbffb2c02ae5c119454d672 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 1 Nov 2021 12:23:45 +0200 Subject: [PATCH 2/2] Add test for GetType call on ptr constrained to nullable --- .../JIT/Directed/nullabletypes/gettype.cs | 48 +++++++++++++++++++ .../Directed/nullabletypes/gettype_d.csproj | 13 +++++ .../Directed/nullabletypes/gettype_do.csproj | 13 +++++ .../Directed/nullabletypes/gettype_r.csproj | 13 +++++ .../Directed/nullabletypes/gettype_ro.csproj | 13 +++++ 5 files changed, 100 insertions(+) create mode 100644 src/tests/JIT/Directed/nullabletypes/gettype.cs create mode 100644 src/tests/JIT/Directed/nullabletypes/gettype_d.csproj create mode 100644 src/tests/JIT/Directed/nullabletypes/gettype_do.csproj create mode 100644 src/tests/JIT/Directed/nullabletypes/gettype_r.csproj create mode 100644 src/tests/JIT/Directed/nullabletypes/gettype_ro.csproj diff --git a/src/tests/JIT/Directed/nullabletypes/gettype.cs b/src/tests/JIT/Directed/nullabletypes/gettype.cs new file mode 100644 index 0000000000000..15bbef729d26f --- /dev/null +++ b/src/tests/JIT/Directed/nullabletypes/gettype.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Linq; +using System.Collections.Generic; + +class C +{ + public IEnumerable Data { get; set; } + + public C() { } + + public bool Check() + { + return Data.ElementAt(0).GetType() == typeof(bool); + } +} + +public class P +{ + public static int Main() + { + C c = new(); + + // Try a nullable with value + c.Data = new List { true }; + if(!c.Check()) + return 666; + + // Try a nullable without value. Should throw NRE + c.Data = new List { new Nullable() }; + + bool thrown = false; + try + { + c.Check(); + } + catch(NullReferenceException) + { + thrown = true; + } + if(!thrown) + return 667; + return 100; + } +} + diff --git a/src/tests/JIT/Directed/nullabletypes/gettype_d.csproj b/src/tests/JIT/Directed/nullabletypes/gettype_d.csproj new file mode 100644 index 0000000000000..749b1f23c8975 --- /dev/null +++ b/src/tests/JIT/Directed/nullabletypes/gettype_d.csproj @@ -0,0 +1,13 @@ + + + Exe + 1 + + + Full + False + + + + + diff --git a/src/tests/JIT/Directed/nullabletypes/gettype_do.csproj b/src/tests/JIT/Directed/nullabletypes/gettype_do.csproj new file mode 100644 index 0000000000000..34744812c8927 --- /dev/null +++ b/src/tests/JIT/Directed/nullabletypes/gettype_do.csproj @@ -0,0 +1,13 @@ + + + Exe + 1 + + + Full + True + + + + + diff --git a/src/tests/JIT/Directed/nullabletypes/gettype_r.csproj b/src/tests/JIT/Directed/nullabletypes/gettype_r.csproj new file mode 100644 index 0000000000000..fea75d031b0ed --- /dev/null +++ b/src/tests/JIT/Directed/nullabletypes/gettype_r.csproj @@ -0,0 +1,13 @@ + + + Exe + 1 + + + None + False + + + + + diff --git a/src/tests/JIT/Directed/nullabletypes/gettype_ro.csproj b/src/tests/JIT/Directed/nullabletypes/gettype_ro.csproj new file mode 100644 index 0000000000000..f51b4298ce4d6 --- /dev/null +++ b/src/tests/JIT/Directed/nullabletypes/gettype_ro.csproj @@ -0,0 +1,13 @@ + + + Exe + 1 + + + None + True + + + + +