From: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 7 Jan 2022 19:00:15 +0000 (-0800) Subject: [release/6.0] Add explicit null-check for tailcalls to VSD (#62769) X-Git-Tag: accepted/tizen/unified/20221103.165808~235^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d575a6ff9093b577d716efd4a8e5aaaad9d645b9;p=platform%2Fupstream%2Fdotnet%2Fruntime.git [release/6.0] Add explicit null-check for tailcalls to VSD (#62769) * Add explicit null-check for tailcalls to VSD 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 * Revert cleanup to make potential backport easier Co-authored-by: Jakob Botsch Nielsen --- diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f6bc9a4..ba5f450 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -7896,6 +7896,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) @@ -8622,15 +8631,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 0000000..3285ee9 --- /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 0000000..6946bed --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_61486/Runtime_61486.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + +