From 187ae8a791d7c8dd6e4bea906221c8385ea126c7 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 10 Sep 2021 23:37:23 +0200 Subject: [PATCH] Fix underestimation of temps size It was possible for us to underestimate the size of temps required. This happened because we used the state of the frame layout to check if the size of temps was computed and that check was wrong. This could lead us to make different decisions about which registers needed to be saved in the prolog and epilog on ARM32. In particular, if the frame size was around the size where a stack probe is necessary, this could be possible. There are also other comments that suggest that this could result in failure while encoding instructions that reference the stack locals. Fix #58293 --- src/coreclr/jit/codegenarm.cpp | 1 + src/coreclr/jit/lclvars.cpp | 2 +- src/coreclr/jit/lsra.cpp | 2 + src/coreclr/jit/regset.cpp | 2 +- src/coreclr/jit/regset.h | 10 ++ .../JitBlue/Runtime_58293/Runtime_58293.cs | 104 ++++++++++++++++++ .../Runtime_58293/Runtime_58293.csproj | 13 +++ 7 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.csproj diff --git a/src/coreclr/jit/codegenarm.cpp b/src/coreclr/jit/codegenarm.cpp index 7c8c75ff9a6208..11c7dc330c4f6f 100644 --- a/src/coreclr/jit/codegenarm.cpp +++ b/src/coreclr/jit/codegenarm.cpp @@ -1883,6 +1883,7 @@ void CodeGen::genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pIni INS_FLAGS_DONT_CARE, REG_STACK_PROBE_HELPER_ARG); regSet.verifyRegUsed(REG_STACK_PROBE_HELPER_ARG); genEmitHelperCall(CORINFO_HELP_STACK_PROBE, 0, EA_UNKNOWN, REG_STACK_PROBE_HELPER_CALL_TARGET); + regSet.verifyRegUsed(REG_STACK_PROBE_HELPER_CALL_TARGET); compiler->unwindPadding(); GetEmitter()->emitIns_Mov(INS_mov, EA_PTRSIZE, REG_SPBASE, REG_STACK_PROBE_HELPER_ARG, /* canSkip */ false); diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 24b80030891b27..8c7948d2981381 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -4688,7 +4688,7 @@ unsigned Compiler::lvaGetMaxSpillTempSize() { unsigned result = 0; - if (lvaDoneFrameLayout >= REGALLOC_FRAME_LAYOUT) + if (codeGen->regSet.hasComputedTmpSize()) { result = codeGen->regSet.tmpGetTotalSize(); } diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index 244ccb4eb4bea7..7220558c67b9ca 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -6474,6 +6474,8 @@ void LinearScan::recordMaxSpill() maxSpill[TYP_FLOAT] += 1; } #endif // TARGET_X86 + + compiler->codeGen->regSet.tmpBeginPreAllocateTemps(); for (int i = 0; i < TYP_COUNT; i++) { if (var_types(i) != RegSet::tmpNormalizeType(var_types(i))) diff --git a/src/coreclr/jit/regset.cpp b/src/coreclr/jit/regset.cpp index 00c8a35c74f545..808a443c10a2b6 100644 --- a/src/coreclr/jit/regset.cpp +++ b/src/coreclr/jit/regset.cpp @@ -625,7 +625,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void RegSet::tmpInit() { tmpCount = 0; - tmpSize = 0; + tmpSize = UINT_MAX; #ifdef DEBUG tmpGetCount = 0; #endif diff --git a/src/coreclr/jit/regset.h b/src/coreclr/jit/regset.h index 4f966329fa1e5a..34a9bcea64629e 100644 --- a/src/coreclr/jit/regset.h +++ b/src/coreclr/jit/regset.h @@ -192,13 +192,23 @@ class RegSet bool tmpAllFree() const; #endif // DEBUG + void tmpBeginPreAllocateTemps() + { + tmpSize = 0; + } void tmpPreAllocateTemps(var_types type, unsigned count); unsigned tmpGetTotalSize() { + assert(hasComputedTmpSize()); return tmpSize; } + bool hasComputedTmpSize() + { + return tmpSize != UINT_MAX; + } + private: unsigned tmpCount; // Number of temps unsigned tmpSize; // Size of all the temps diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.cs b/src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.cs new file mode 100644 index 00000000000000..8c3926c8247b8a --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.cs @@ -0,0 +1,104 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Generated by Fuzzlyn v1.4 on 2021-08-24 18:42:34 +// Run on .NET 7.0.0-dev on Arm Linux +// Seed: 4314857431407232792 +// Reduced from 582.3 KiB to 3.3 KiB in 02:00:04 +// Crashes the runtime +public interface I0 +{ +} + +public interface I3 +{ +} + +public struct S0 : I0 +{ + public sbyte F0; + public short F1; + public sbyte F2; + public int F4; + public bool F5; + public S0(bool f5): this() + { + } + + public S1 M34(S1[] arg0) + { + return default(S1); + } +} + +public struct S1 : I0, I3 +{ + public short F0; + public S0 F1; + public S0 F2; + public S0 F3; + public S1(S0 f1, S0 f2, S0 f3): this() + { + } +} + +public class Runtime_58293 +{ + private static I s_rt; + public static I3 s_21; + public static S1 s_29; + public static S1[][] s_32 = new S1[][]{new S1[]{new S1(new S0(false), new S0(false), new S0(true))}}; + public static I3[][] s_42; + public static int Main() + { + s_rt = new C(); + var vr3 = s_32[0][0].F3.F2; + M33(vr3); + return 100; + } + + public static I0[] M33(sbyte arg0) + { + S1 var3; + I3 var8; + S1 var9; + uint var11; + S0 var12; + var vr24 = s_32[0]; + new S0(false).M34(vr24); + I0 var0 = new S0(true); + var vr0 = new S1[]{new S1(new S0(true), new S0(false), new S0(false)), new S1(new S0(true), new S0(false), new S0(false)), new S1(new S0(true), new S0(false), new S0(true))}; + new S0(true).M34(vr0); + try + { + bool var1 = s_29.F2.F5; + s_rt.WriteLine(var1); + } + finally + { + s_42 = new I3[][]{new I3[]{new S1(new S0(true), new S0(true), new S0(true)), new S1(new S0(true), new S0(true), new S0(true))}, new I3[]{new S1(new S0(false), new S0(true), new S0(true)), new S1(new S0(false), new S0(true), new S0(true)), new S1(new S0(true), new S0(false), new S0(true)), new S1(new S0(true), new S0(false), new S0(false))}, new I3[]{new S1(new S0(true), new S0(true), new S0(false)), new S1(new S0(true), new S0(false), new S0(true)), new S1(new S0(false), new S0(false), new S0(false)), new S1(new S0(false), new S0(false), new S0(true)), new S1(new S0(true), new S0(true), new S0(false))}, new I3[]{new S1(new S0(false), new S0(true), new S0(false)), new S1(new S0(true), new S0(false), new S0(false)), new S1(new S0(true), new S0(false), new S0(false)), new S1(new S0(true), new S0(false), new S0(true)), new S1(new S0(true), new S0(false), new S0(true))}, new I3[]{new S1(new S0(false), new S0(false), new S0(false)), new S1(new S0(true), new S0(false), new S0(true)), new S1(new S0(true), new S0(false), new S0(true)), new S1(new S0(false), new S0(true), new S0(true)), new S1(new S0(false), new S0(true), new S0(true))}, new I3[]{new S1(new S0(false), new S0(true), new S0(true)), new S1(new S0(false), new S0(false), new S0(false))}}; + } + + var vr30 = new S1[]{new S1(new S0(true), new S0(false), new S0(false)), new S1(new S0(true), new S0(false), new S0(false)), new S1(new S0(true), new S0(false), new S0(true)), new S1(new S0(true), new S0(false), new S0(true)), new S1(new S0(true), new S0(false), new S0(false))}; + s_21 = s_29.F2.M34(vr30); + var0 = new S1(new S0(true), new S0(false), new S0(true)); + var vr2 = new S0(true); + I3 vr23; + I3 var2 = new S1(new S0(true), new S0(true), new S0(false)); + bool vr25; + bool vr26; + bool vr28; + bool vr29; + bool vr31; + return new I0[]{new S0(false), new S0(true), new S1(new S0(true), new S0(true), new S0(false))}; + } +} + +public interface I { void WriteLine(T val); } +public class C : I +{ + public void WriteLine(T val) + { + System.Console.WriteLine(val); + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.csproj new file mode 100644 index 00000000000000..e7284d26ab2f18 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_58293/Runtime_58293.csproj @@ -0,0 +1,13 @@ + + + Exe + True + + + None + True + + + + +