From c0c4125029663322ca456e44fa43d271e0a9d418 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 14 Dec 2021 01:15:34 +0100 Subject: [PATCH] Add explicit null-check for tailcalls to VSD (#62719) There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD as well. Fix #61486 --- src/coreclr/jit/morph.cpp | 18 ++++---- .../JitBlue/Runtime_61486/Runtime_61486.cs | 45 +++++++++++++++++++ .../Runtime_61486/Runtime_61486.csproj | 9 ++++ 3 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 288f7c16f4d9b..9e51c24734995 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -7742,6 +7742,15 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call) // Avoid potential extra work for the return (for example, vzeroupper) call->gtType = TYP_VOID; + // The runtime requires that we perform a null check on the `this` argument before + // tail calling to a virtual dispatch stub. This requirement is a consequence of limitations + // in the runtime's ability to map an AV to a NullReferenceException if + // the AV occurs in a dispatch stub that has unmanaged caller. + if (call->IsVirtualStub()) + { + call->gtFlags |= GTF_CALL_NULLCHECK; + } + // Do some target-specific transformations (before we process the args, // etc.) for the JIT helper case. if (tailCallViaJitHelper) @@ -8448,15 +8457,6 @@ void Compiler::fgMorphTailCallViaJitHelper(GenTreeCall* call) JITDUMP("fgMorphTailCallViaJitHelper (before):\n"); DISPTREE(call); - // The runtime requires that we perform a null check on the `this` argument before - // tail calling to a virtual dispatch stub. This requirement is a consequence of limitations - // in the runtime's ability to map an AV to a NullReferenceException if - // the AV occurs in a dispatch stub that has unmanaged caller. - if (call->IsVirtualStub()) - { - call->gtFlags |= GTF_CALL_NULLCHECK; - } - // For the helper-assisted tail calls, we need to push all the arguments // into a single list, and then add a few extra at the beginning or end. // diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.cs b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.cs new file mode 100644 index 0000000000000..3285ee9cc556a --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.cs @@ -0,0 +1,45 @@ +// 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.Reflection; +using System.Runtime.CompilerServices; + +public class Runtime_61486 +{ + public static int Main() + { + var my = new My(new My(null)); + var m = my.GetType().GetMethod("M"); + try + { + m.Invoke(my, null); + return -1; + } + catch (TargetInvocationException ex) when (ex.InnerException is NullReferenceException) + { + return 100; + } + } + + public interface IFace + { + void M(); + } + + public class My : IFace + { + private IFace _face; + + public My(IFace face) + { + _face = face; + } + + // We cannot handle a null ref inside a VSD if the caller is not + // managed frame. This test is verifying that JIT null checks ahead of + // time in this case. + [MethodImpl(MethodImplOptions.AggressiveOptimization)] + public void M() => _face.M(); + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj new file mode 100644 index 0000000000000..6946bed81bfd5 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + +