From c566378507153007262072645847639cc0c5cc85 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 19 Jul 2023 13:18:12 +0200 Subject: [PATCH] JIT: Expand inlined delegate calls in correct order The access of the target instance was incorrectly inserted right after the location of the delegate instance. Since this indirection can throw a NRE this is incorrect; to get the proper inlined behavior, the indirection must happen only after all arguments have been evaluated. Fix #75832 --- src/coreclr/jit/lower.cpp | 9 ++++- .../JitBlue/Runtime_75832/Runtime_75832.cs | 38 +++++++++++++++++++ .../Runtime_75832/Runtime_75832.csproj | 9 +++++ 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.csproj diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 79b10a5201fcf..157fda65dbf2b 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4958,9 +4958,14 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) GenTree* newThis = comp->gtNewIndir(TYP_REF, newThisAddr); - BlockRange().InsertAfter(thisExpr, newThisAddr, newThis); - + // Insert the new 'this' arg right before the call to get the correct null + // behavior (the NRE that would logically happen inside Delegate.Invoke + // should happen after all args are evaluated). We must also move the + // PUTARG_REG node ahead. thisArgNode->AsOp()->gtOp1 = newThis; + BlockRange().Remove(thisArgNode); + BlockRange().InsertBefore(call, newThisAddr, newThis, thisArgNode); + ContainCheckIndir(newThis->AsIndir()); // the control target is diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.cs b/src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.cs new file mode 100644 index 0000000000000..edfcc9f177f3c --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.cs @@ -0,0 +1,38 @@ +// 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.Runtime.CompilerServices; +using Xunit; + +public class Runtime_75832 +{ + [Fact] + public static int TestEntryPoint() + { + try + { + Test(0); + Console.WriteLine("FAIL: No exception thrown"); + } + catch (DivideByZeroException) + { + return 100; + } + catch (Exception ex) + { + Console.WriteLine("FAIL: Caught {0}", ex.GetType().Name); + } + + return 101; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Test(int i) + { + GetAction()(100 / i); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static Action GetAction() => null; +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.csproj new file mode 100644 index 0000000000000..1bb887ea34b0f --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_75832/Runtime_75832.csproj @@ -0,0 +1,9 @@ + + + True + None + + + + + \ No newline at end of file