From daac3aebac51b4103df2b992f0d1fe1e3a0ed9cd Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 12 Dec 2019 08:47:22 -0800 Subject: [PATCH] JIT: fix spill logic for local structs If we're appending an assignment whose LHS is is a location within a local struct, we need to spill all references to that struct from the eval stack. Update the existing logic for this to handle the case where the LHS is a field of a local struct, and the field is updated by unusual means (here, `initobj`). Fixes #764. --- src/coreclr/src/jit/importer.cpp | 21 ++++++-- .../JitBlue/Runtime_764/Runtime_764.cs | 52 +++++++++++++++++++ .../JitBlue/Runtime_764/Runtime_764.csproj | 10 ++++ 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.cs create mode 100644 src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.csproj diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp index c474744aec68e..c57d1a4d8f350 100644 --- a/src/coreclr/src/jit/importer.cpp +++ b/src/coreclr/src/jit/importer.cpp @@ -10831,11 +10831,22 @@ void Compiler::impImportBlockCode(BasicBlock* block) } else if (lhs->OperIsBlk()) { - // Check for ADDR(LCL_VAR), or ADD(ADDR(LCL_VAR),CNS_INT)) - // (the latter may appear explicitly in the IL). - // Local field stores will cause the stack to be spilled when - // they are encountered. - lclVar = lhs->AsBlk()->Addr()->IsLocalAddrExpr(); + // Check if LHS address is within some struct local, to catch + // cases where we're updating the struct by something other than a stfld + GenTree* addr = lhs->AsBlk()->Addr(); + + // Catches ADDR(LCL_VAR), or ADD(ADDR(LCL_VAR),CNS_INT)) + lclVar = addr->IsLocalAddrExpr(); + + // Catches ADDR(FIELD(... ADDR(LCL_VAR))) + if (lclVar == nullptr) + { + GenTree* lclTree = nullptr; + if (impIsAddressInLocal(addr, &lclTree)) + { + lclVar = lclTree->AsLclVarCommon(); + } + } } if (lclVar != nullptr) { diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.cs new file mode 100644 index 0000000000000..de83d4ff2f556 --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.cs @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.CompilerServices; + +// Regression test case for importer bug. +// If Release is inlined into Main, the importer may unsafely re-order trees. + +public struct Ptr where T: class +{ + private T _value; + + public Ptr(T value) + { + _value = value; + } + + public T Release() + { + T tmp = _value; + _value = null; + return tmp; + } +} + +class Runtime_764 +{ + private static int Main(string[] args) + { + Ptr ptr = new Ptr("Hello, world"); + + bool res = false; + while (res) + { + } + + string summary = ptr.Release(); + + if (summary == null) + { + Console.WriteLine("FAILED"); + return -1; + } + else + { + Console.WriteLine("PASSED"); + return 100; + } + } +} diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.csproj new file mode 100644 index 0000000000000..1100f420532dc --- /dev/null +++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/Runtime_764/Runtime_764.csproj @@ -0,0 +1,10 @@ + + + Exe + None + True + + + + +