From 3486b7b5d88a62226b7aec62f48d568ff5da1871 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 13 Dec 2021 22:40:04 +0300 Subject: [PATCH 1/2] Add a test --- .../JitBlue/Runtime_13762/Runtime_13762.cs | 39 +++++++++++++++++++ .../Runtime_13762/Runtime_13762.csproj | 9 +++++ 2 files changed, 48 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.cs b/src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.cs new file mode 100644 index 00000000000000..f02a9af90da58c --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.cs @@ -0,0 +1,39 @@ +// 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; + +public class Runtime_13762 +{ + public static int Main() + { + try + { + Problem(null); + } + catch (NullReferenceException) + { + return 100; + } + + return 101; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Problem(ClassWithField a) + { + a.Field = a.Call(ref a.Field); + } +} + +public class ClassWithField +{ + public int Field; + + [MethodImpl(MethodImplOptions.NoInlining)] + public int Call(ref int a) + { + throw new Exception("This should have been an NRE!"); + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.csproj new file mode 100644 index 00000000000000..f492aeac9d056b --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_13762/Runtime_13762.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + \ No newline at end of file From 2f8d248dfcae091b37bae60e50609b23c962aed6 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Thu, 25 Nov 2021 13:04:59 +0300 Subject: [PATCH 2/2] Fix the bug --- src/coreclr/jit/assertionprop.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index b842e1363fed62..71d2dc688968e5 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -2558,8 +2558,14 @@ void Compiler::optAssertionGen(GenTree* tree) switch (tree->gtOper) { case GT_ASG: + // An indirect store - we can create a non-null assertion. Note that we do not lose out + // on the dataflow assertions here as local propagation only deals with LCL_VAR LHSs. + if (tree->AsOp()->gtGetOp1()->OperIsIndir()) + { + assertionInfo = optCreateAssertion(tree->AsOp()->gtGetOp1()->AsIndir()->Addr(), nullptr, OAK_NOT_EQUAL); + } // VN takes care of non local assertions for assignments and data flow. - if (optLocalAssertionProp) + else if (optLocalAssertionProp) { assertionInfo = optCreateAssertion(tree->AsOp()->gtOp1, tree->AsOp()->gtOp2, OAK_EQUAL); } @@ -2573,16 +2579,26 @@ void Compiler::optAssertionGen(GenTree* tree) case GT_BLK: case GT_DYN_BLK: case GT_IND: - case GT_NULLCHECK: - // All indirections create non-null assertions - assertionInfo = optCreateAssertion(tree->AsIndir()->Addr(), nullptr, OAK_NOT_EQUAL); + // R-value indirections create non-null assertions, but not all indirections are R-values. + // Those under ADDR nodes or on the LHS of ASGs are "locations", and will not end up + // dereferencing their operands. We cannot reliably detect them here, however, and so + // will have to rely on the conservative approximation of the GTF_NO_CSE flag. + if (tree->CanCSE()) + { + assertionInfo = optCreateAssertion(tree->AsIndir()->Addr(), nullptr, OAK_NOT_EQUAL); + } break; case GT_ARR_LENGTH: - // An array length is an indirection (but doesn't derive from GenTreeIndir). + // An array length is an (always R-value) indirection (but doesn't derive from GenTreeIndir). assertionInfo = optCreateAssertion(tree->AsArrLen()->ArrRef(), nullptr, OAK_NOT_EQUAL); break; + case GT_NULLCHECK: + // Explicit null checks always create non-null assertions. + assertionInfo = optCreateAssertion(tree->AsIndir()->Addr(), nullptr, OAK_NOT_EQUAL); + break; + case GT_BOUNDS_CHECK: if (!optLocalAssertionProp) {