diff --git a/src/vm/eetwain.cpp b/src/vm/eetwain.cpp index 1fb332ecd42a..511a63550994 100644 --- a/src/vm/eetwain.cpp +++ b/src/vm/eetwain.cpp @@ -2404,14 +2404,16 @@ unsigned scanArgRegTable(PTR_CBYTE table, Returns size of things pushed on the stack for ESP frames Arguments: - table - The pointer table - curOffs - The current code offset - info - Incoming arg used to determine if there's a frame, and to save results + table - The pointer table + curOffsRegs - The current code offset that should be used for reporting registers + curOffsArgs - The current code offset that should be used for reporting args + info - Incoming arg used to determine if there's a frame, and to save results */ static -unsigned scanArgRegTableI(PTR_CBYTE table, - unsigned curOffs, +unsigned scanArgRegTableI(PTR_CBYTE table, + unsigned curOffsRegs, + unsigned curOffsArgs, hdrInfo * info) { CONTRACTL { @@ -2433,6 +2435,10 @@ unsigned scanArgRegTableI(PTR_CBYTE table, bool isThis = false; bool iptr = false; + // The comment before the call to scanArgRegTableI in EnumGCRefs + // describes why curOffsRegs can be smaller than curOffsArgs. + _ASSERTE(curOffsRegs <= curOffsArgs); + #if VERIFY_GC_TABLES _ASSERTE(*castto(table, unsigned short *)++ == 0xBABE); #endif @@ -2506,7 +2512,7 @@ unsigned scanArgRegTableI(PTR_CBYTE table, /* Have we reached the instruction we're looking for? */ - while (ptrOffs <= curOffs) + while (ptrOffs <= curOffsArgs) { unsigned val; @@ -2543,10 +2549,14 @@ unsigned scanArgRegTableI(PTR_CBYTE table, regNum reg; ptrOffs += (val ) & 0x7; - if (ptrOffs > curOffs) { + if (ptrOffs > curOffsArgs) { iptr = isThis = false; goto REPORT_REFS; } + else if (ptrOffs > curOffsRegs) { + iptr = isThis = false; + continue; + } reg = (regNum)((val >> 3) & 0x7); regMask = 1 << reg; // EAX,ECX,EDX,EBX,---,EBP,ESI,EDI @@ -2606,7 +2616,7 @@ unsigned scanArgRegTableI(PTR_CBYTE table, /* A small argument encoding */ ptrOffs += (val & 0x07); - if (ptrOffs > curOffs) { + if (ptrOffs > curOffsArgs) { iptr = isThis = false; goto REPORT_REFS; } @@ -2736,7 +2746,7 @@ unsigned scanArgRegTableI(PTR_CBYTE table, /* non-ptr arg push */ _ASSERTE(!hasPartialArgInfo); ptrOffs += (val & 0x07); - if (ptrOffs > curOffs) { + if (ptrOffs > curOffsArgs) { iptr = isThis = false; goto REPORT_REFS; } @@ -2858,6 +2868,7 @@ unsigned GetPushedArgSize(hdrInfo * info, PTR_CBYTE table, DWORD curOffs) if (info->interruptible) { sz = scanArgRegTableI(skipToArgReg(*info, table), + curOffs, curOffs, info); } @@ -4436,7 +4447,15 @@ bool EECodeManager::EnumGcRefs( PREGDISPLAY pContext, if (info.interruptible) { - pushedSize = scanArgRegTableI(skipToArgReg(info, table), curOffs, &info); + // If we are not on the active stack frame, we need to report gc registers + // that are live before the call. The reason is that the liveness of gc registers + // may change across a call to a method that does not return. In this case the instruction + // after the call may be a jump target and a register that didn't have a live gc pointer + // before the call may have a live gc pointer after the jump. To make sure we report the + // registers that have live gc pointers before the call we subtract 1 from curOffs. + unsigned curOffsRegs = (flags & ActiveStackFrame) != 0 ? curOffs : curOffs - 1; + + pushedSize = scanArgRegTableI(skipToArgReg(info, table), curOffsRegs, curOffs, &info); RegMask regs = info.regMaskResult; RegMask iregs = info.iregMaskResult; @@ -5342,7 +5361,7 @@ OBJECTREF EECodeManager::GetInstance( PREGDISPLAY pContext, if (info.interruptible) { - stackDepth = scanArgRegTableI(skipToArgReg(info, table), (unsigned)relOffset, &info); + stackDepth = scanArgRegTableI(skipToArgReg(info, table), relOffset, relOffset, &info); } else { @@ -6046,6 +6065,7 @@ TADDR EECodeManager::GetAmbientSP(PREGDISPLAY pContext, if (stateBuf->hdrInfoBody.interruptible) { baseSP += scanArgRegTableI(skipToArgReg(stateBuf->hdrInfoBody, table), + dwRelOffset, dwRelOffset, &stateBuf->hdrInfoBody); } diff --git a/tests/src/Regressions/coreclr/GitHub_17398/test17398.cs b/tests/src/Regressions/coreclr/GitHub_17398/test17398.cs new file mode 100644 index 000000000000..a9cc4b998091 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_17398/test17398.cs @@ -0,0 +1,84 @@ +// 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; + +// Repro case for CoreCLR 17398 + +class X +{ + static int v; + + string s; + + public override string ToString() => s; + + [MethodImpl(MethodImplOptions.NoInlining)] + X(int x) + { + s = "String" + x; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void F() { } + + public static void T0(object o, int x) + { + GC.Collect(2); + throw new Exception(o.ToString()); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Test() + { + object x1 = new X(1); + object x2 = new X(2); + + if (v == 1) + { + // Generate enough pressure here to + // kill ESI so in linear flow it is dead + // at the call to T0 + int w = v; + int x = v; + int y = v; + int z = v; + + // Unbounded loop here forces fully interruptible GC + for (int i = 0; i < v; i++) + { + w++; + } + + T0(x2, w + x + y + z); + } + + // Encourage x1 to be in callee save (ESI) + F(); + + if (v == 2) + { + T0(x1, 0); + } + } + + public static int Main() + { + v = 1; + int r = 0; + + try + { + Test(); + } + catch (Exception e) + { + Console.WriteLine(e.Message); + r = 100; + } + + return r; + } +} diff --git a/tests/src/Regressions/coreclr/GitHub_17398/test17398.csproj b/tests/src/Regressions/coreclr/GitHub_17398/test17398.csproj new file mode 100644 index 000000000000..53d0e2ed2138 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_17398/test17398.csproj @@ -0,0 +1,41 @@ + + + + + Debug + AnyCPU + 2.0 + {E55A6F8B-B9E3-45CE-88F4-22AE70F606CB} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + true + BuildAndRun + + + + + + + + + False + + + + + True + + + + + + + + + + + + + + \ No newline at end of file