From d73ae9854a337b1e0533bb6d5aa9004f9ed0a962 Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Tue, 24 Nov 2020 20:48:25 -0800 Subject: [PATCH 01/12] Add regression tests for https://github.com/dotnet/runtime/issues/45090 --- .../JitBlue/Runtime_45090/Runtime_45090.cs | 149 ++++++++++++++++++ .../Runtime_45090/Runtime_45090.csproj | 15 ++ 2 files changed, 164 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs b/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs new file mode 100644 index 00000000000000..0bc96bb1e5b5a2 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs @@ -0,0 +1,149 @@ +// 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 System.Threading; +using System.Threading.Tasks; + +#pragma warning disable CS0649 + +namespace Runtime_45090 +{ + struct _4 + { + public byte _0; + public byte _1; + public byte _2; + public byte _3; + } + + struct _10 + { + public long _0; + public long _8; + } + + struct _100 + { + public _10 _0; + public _10 _1; + public _10 _2; + public _10 _3; + public _10 _4; + public _10 _5; + public _10 _6; + public _10 _7; + public _10 _8; + public _10 _9; + public _10 _a; + public _10 _b; + public _10 _c; + public _10 _d; + public _10 _e; + public _10 _f; + } + + struct _e00 + { + public _100 _0; + public _100 _1; + public _100 _2; + public _100 _3; + public _100 _4; + public _100 _5; + public _100 _6; + public _100 _7; + public _100 _8; + public _100 _9; + public _100 _a; + public _100 _b; + public _100 _c; + public _100 _d; + } + + struct _1000 + { + public _100 _0; + public _100 _1; + public _100 _2; + public _100 _3; + public _100 _4; + public _100 _5; + public _100 _6; + public _100 _7; + public _100 _8; + public _100 _9; + public _100 _a; + public _100 _b; + public _100 _c; + public _100 _d; + public _100 _e; + public _100 _f; + } + + abstract class AllocFrame + { + public abstract int VirtMethodEspBasedFrame(); + } + + class PushReg : AllocFrame + { + // Frame size is 4 bytes and allocated with 'push eax' instruction. + public unsafe override int VirtMethodEspBasedFrame() + { + _4 tmp = new _4(); + *(int*)(&tmp) = 45090; + return (int)tmp._1; + } + } + + class NoProbe : AllocFrame + { + // Frame size is less than 0x1000 bytes and doesn't require probing. + public override int VirtMethodEspBasedFrame() + { + _100 tmp = new _100(); + return (int)tmp._0._0; + } + } + + class InlineProbe : AllocFrame + { + // Frame size is less than 0x1000 bytes and uses inline probing with 'test eax, [esp]' instruction. + public override int VirtMethodEspBasedFrame() + { + _e00 tmp = new _e00(); + return (int)tmp._0._0._0; + } + } + + class HelperProbe : AllocFrame + { + // Frame is greater than 0x1000 bytes and uses JIT_StackProbe helper. + public override int VirtMethodEspBasedFrame() + { + _1000 tmp = new _1000(); + return (int)tmp._0._0._0; + } + } + + class Program + { + [MethodImpl(MethodImplOptions.NoInlining)] + static void TestSkipAllocFrame(AllocFrame scenario) + { + scenario.VirtMethodEspBasedFrame(); + } + + static int Main(string[] args) + { + TestSkipAllocFrame(new PushReg()); + TestSkipAllocFrame(new NoProbe()); + TestSkipAllocFrame(new InlineProbe()); + TestSkipAllocFrame(new HelperProbe()); + + return 100; + } + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.csproj new file mode 100644 index 00000000000000..1253dec6839300 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.csproj @@ -0,0 +1,15 @@ + + + Exe + None + True + True + + + + + + From 0b88d716a4af8165dbfa73a09857778ca68a5dff Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Tue, 24 Nov 2020 14:43:54 -0800 Subject: [PATCH 02/12] Remove padding for ReJit in CodeGen::genAllocLclFrame in src/coreclr/jit/codegenxarch.cpp --- src/coreclr/jit/codegenxarch.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index b3cc225dd411d4..c919818655a4f6 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -2009,8 +2009,6 @@ void CodeGen::genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pIni GetEmitter()->emitIns_R_AR(INS_lea, EA_PTRSIZE, REG_STACK_PROBE_HELPER_ARG, REG_SPBASE, spOffset); regSet.verifyRegUsed(REG_STACK_PROBE_HELPER_ARG); - // Can't have a call until we have enough padding for ReJit. - genPrologPadForReJit(); genEmitHelperCall(CORINFO_HELP_STACK_PROBE, 0, EA_UNKNOWN); if (compiler->info.compPublishStubParam) @@ -2029,8 +2027,6 @@ void CodeGen::genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pIni GetEmitter()->emitIns_R_AR(INS_lea, EA_PTRSIZE, REG_STACK_PROBE_HELPER_ARG, REG_SPBASE, -(int)frameSize); regSet.verifyRegUsed(REG_STACK_PROBE_HELPER_ARG); - // Can't have a call until we have enough padding for ReJit. - genPrologPadForReJit(); genEmitHelperCall(CORINFO_HELP_STACK_PROBE, 0, EA_UNKNOWN); if (initReg == REG_DEFAULT_HELPER_CALL_TARGET) From fdd1b78dd0cafc06982140d16a3414d4f2750719 Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Tue, 24 Nov 2020 14:50:26 -0800 Subject: [PATCH 03/12] Add instruction opcodes in src/coreclr/vm/eetwain.cpp --- src/coreclr/vm/eetwain.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index 88ba0a6b6f7f3a..a20fbb4bf83f3a 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -36,13 +36,17 @@ #define X86_INSTR_NOP5_5 0x90 // 5th byte of 5-byte nop #define X86_INSTR_INT3 0xCC // int3 #define X86_INSTR_HLT 0xF4 // hlt +#define X86_INSTR_PUSH_EAX 0x50 // push eax #define X86_INSTR_PUSH_EBP 0x55 // push ebp #define X86_INSTR_W_MOV_EBP_ESP 0xEC8B // mov ebp, esp #define X86_INSTR_POP_ECX 0x59 // pop ecx #define X86_INSTR_RET 0xC2 // ret imm16 #define X86_INSTR_RETN 0xC3 // ret +#define X86_INSTR_w_TEST_ESP_EAX 0x0485 // test [esp], eax #define X86_INSTR_w_LEA_ESP_EBP_BYTE_OFFSET 0x658d // lea esp, [ebp-bOffset] #define X86_INSTR_w_LEA_ESP_EBP_DWORD_OFFSET 0xa58d // lea esp, [ebp-dwOffset] +#define X86_INSTR_w_LEA_EAX_ESP_BYTE_OFFSET 0x448d // lea eax, [esp-bOffset] +#define X86_INSTR_w_LEA_EAX_ESP_DWORD_OFFSET 0x848d // lea eax, [esp-dwOffset] #define X86_INSTR_JMP_NEAR_REL32 0xE9 // near jmp rel32 #define X86_INSTR_w_JMP_FAR_IND_IMM 0x25FF // far jmp [addr32] From 07c0cb88c8438c98be05d74f494b2ee8e7f08f01 Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Tue, 24 Nov 2020 14:53:25 -0800 Subject: [PATCH 04/12] Add SKIP_LEA_EAX_ESP in src/coreclr/vm/eetwain.cpp --- src/coreclr/vm/eetwain.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index a20fbb4bf83f3a..82a671af2a2f1b 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -3067,6 +3067,29 @@ inline unsigned SKIP_LEA_ESP_EBP(int val, PTR_CBYTE base, unsigned offset) return(offset + delta); } +inline unsigned SKIP_LEA_EAX_ESP(int val, PTR_CBYTE base, unsigned offset) +{ + LIMITED_METHOD_DAC_CONTRACT; + +#ifdef _DEBUG + WORD wOpcode = *(PTR_WORD)(base + offset); + if (CheckInstrWord(wOpcode, X86_INSTR_w_LEA_EAX_ESP_BYTE_OFFSET)) + { + _ASSERTE(val == *(PTR_SBYTE)(base + offset + 3)); + _ASSERTE(CAN_COMPRESS(val)); + } + else + { + _ASSERTE(CheckInstrWord(wOpcode, X86_INSTR_w_LEA_EAX_ESP_DWORD_OFFSET)); + _ASSERTE(val == *(PTR_INT32)(base + offset + 3)); + _ASSERTE(!CAN_COMPRESS(val)); + } +#endif + + unsigned delta = 3 + (CAN_COMPRESS(-val) ? 1 : 4); + return(offset + delta); +} + unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) { CONTRACTL { From 1b0636ec21a56ff3ae3cc465b6ec27e22b4a240d Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Tue, 24 Nov 2020 14:53:51 -0800 Subject: [PATCH 05/12] Add SKIP_HELPER_CALL in src/coreclr/vm/eetwain.cpp --- src/coreclr/vm/eetwain.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index 82a671af2a2f1b..f2fa6c3774395e 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -3090,6 +3090,28 @@ inline unsigned SKIP_LEA_EAX_ESP(int val, PTR_CBYTE base, unsigned offset) return(offset + delta); } +inline unsigned SKIP_HELPER_CALL(PTR_CBYTE base, unsigned offset) +{ + LIMITED_METHOD_DAC_CONTRACT; + + unsigned delta; + + if (CheckInstrByte(base[offset], X86_INSTR_CALL_REL32)) + { + delta = 5; + } + else + { +#ifdef _DEBUG + WORD wOpcode = *(PTR_WORD)(base+offset); + _ASSERTE(CheckInstrWord(wOpcode, X86_INSTR_W_CALL_IND_IMM)); +#endif + delta = 6; + } + + return(offset+delta); +} + unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) { CONTRACTL { From 478c9e6ed36e1da2f1f638f59856de4c4d9a1868 Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Tue, 24 Nov 2020 14:54:37 -0800 Subject: [PATCH 06/12] Update SKIP_ALLOC_FRAME in src/coreclr/vm/eetwain.cpp --- src/coreclr/vm/eetwain.cpp | 70 ++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index f2fa6c3774395e..e3822f6cbadc3f 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -3128,42 +3128,54 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) return (SKIP_PUSH_REG(base, offset)); } - if (size >= (int)GetOsPageSize()) + const int pageSize = (int)GetOsPageSize(); + + if (size < pageSize) { - if (size < int(3 * GetOsPageSize())) + // sub esp, size + offset = SKIP_ARITH_REG(size, base, offset); + + const int STACK_PROBE_BOUNDARY_THRESHOLD_BYTES = 1024; + + unsigned delta = 0; + + if (size + STACK_PROBE_BOUNDARY_THRESHOLD_BYTES > pageSize) { - // add 7 bytes for one or two TEST EAX, [ESP+GetOsPageSize()] - offset += (size / GetOsPageSize()) * 7; +#ifdef _DEBUG + WORD wOpcode = *(PTR_WORD)(base+offset); + _ASSERTE(CheckInstrWord(wOpcode, X86_INSTR_w_TEST_ESP_EAX)); +#endif + // test [esp], eax + delta = 3; + } + + return (offset+delta); + } + else + { + if (CheckInstrByte(base[offset], X86_INSTR_PUSH_EAX)) + { + // push eax + offset = SKIP_PUSH_REG(base, offset); + // lea eax, [esp-size+4] + offset = SKIP_LEA_EAX_ESP(-size+4, base, offset); + // call JIT_StackProbe + offset = SKIP_HELPER_CALL(base, offset); + // pop eax + offset = SKIP_POP_REG(base, offset); + // sub esp, size + return (SKIP_ARITH_REG(size, base, offset)); } else { - // xor eax, eax 2 - // [nop] 0-3 - // loop: - // test [esp + eax], eax 3 - // sub eax, 0x1000 5 - // cmp EAX, -size 5 - // jge loop 2 - offset += 2; - - // NGEN images that support rejit may have extra nops we need to skip over - while (offset < 5) - { - if (CheckInstrByte(base[offset], X86_INSTR_NOP)) - { - offset++; - } - else - { - break; - } - } - offset += 15; + // lea eax, [esp-size] + offset = SKIP_LEA_EAX_ESP(-size, base, offset); + // call JIT_StackProbe + offset = SKIP_HELPER_CALL(base, offset); + // mov esp, eax + return (SKIP_MOV_REG_REG(base, offset)); } } - - // sub ESP, size - return (SKIP_ARITH_REG(size, base, offset)); } From c2ce8daed5c33e8e642ee1d07846e89a71c01e77 Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Thu, 10 Dec 2020 19:07:24 -0800 Subject: [PATCH 07/12] Use constant (0x1000) rather than calling GetOsPageSize() in SKIP_ALLOC_FRAME in src/coreclr/vm/eetwain.cpp --- src/coreclr/vm/eetwain.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index e3822f6cbadc3f..f65dd4e63f8afa 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -3128,9 +3128,9 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) return (SKIP_PUSH_REG(base, offset)); } - const int pageSize = (int)GetOsPageSize(); + const int sizeProbingRequired = 0x1000; - if (size < pageSize) + if (size < sizeProbingRequired) { // sub esp, size offset = SKIP_ARITH_REG(size, base, offset); @@ -3139,7 +3139,7 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) unsigned delta = 0; - if (size + STACK_PROBE_BOUNDARY_THRESHOLD_BYTES > pageSize) + if (size + STACK_PROBE_BOUNDARY_THRESHOLD_BYTES > sizeProbingRequired) { #ifdef _DEBUG WORD wOpcode = *(PTR_WORD)(base+offset); From 32206548ffe8a7a928bea98bf750c61c89b577c5 Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Wed, 16 Dec 2020 19:22:51 -0800 Subject: [PATCH 08/12] Draft of the changes that are back-compatible with earlier versions of R2R code in src/coreclr/vm/eetwain.cpp --- src/coreclr/vm/eetwain.cpp | 152 ++++++++++++++++++++++++++++--------- 1 file changed, 115 insertions(+), 37 deletions(-) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index f65dd4e63f8afa..f5635a873720a3 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -20,7 +20,6 @@ #include "argdestination.h" -#define X86_INSTR_W_TEST_ESP 0x4485 // test [esp+N], eax #define X86_INSTR_TEST_ESP_SIB 0x24 #define X86_INSTR_PUSH_0 0x6A // push 00, entire instruction is 0x6A00 #define X86_INSTR_PUSH_IMM 0x68 // push NNNN, @@ -42,13 +41,15 @@ #define X86_INSTR_POP_ECX 0x59 // pop ecx #define X86_INSTR_RET 0xC2 // ret imm16 #define X86_INSTR_RETN 0xC3 // ret +#define X86_INSTR_XOR 0x33 // xor #define X86_INSTR_w_TEST_ESP_EAX 0x0485 // test [esp], eax +#define X86_INSTR_w_TEST_ESP_DWORD_OFFSET_EAX 0x8485 // test [esp-dwOffset], eax #define X86_INSTR_w_LEA_ESP_EBP_BYTE_OFFSET 0x658d // lea esp, [ebp-bOffset] #define X86_INSTR_w_LEA_ESP_EBP_DWORD_OFFSET 0xa58d // lea esp, [ebp-dwOffset] #define X86_INSTR_w_LEA_EAX_ESP_BYTE_OFFSET 0x448d // lea eax, [esp-bOffset] #define X86_INSTR_w_LEA_EAX_ESP_DWORD_OFFSET 0x848d // lea eax, [esp-dwOffset] -#define X86_INSTR_JMP_NEAR_REL32 0xE9 // near jmp rel32 -#define X86_INSTR_w_JMP_FAR_IND_IMM 0x25FF // far jmp [addr32] +#define X86_INSTR_JMP_NEAR_REL32 0xE9 // near jmp rel32 +#define X86_INSTR_w_JMP_FAR_IND_IMM 0x25FF // far jmp [addr32] #ifndef USE_GC_INFO_DECODER @@ -3124,60 +3125,137 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) if (size == sizeof(void*)) { - // We do "push eax" instead of "sub esp,4" + // JIT emits "push eax" instead of "sub esp,4" return (SKIP_PUSH_REG(base, offset)); } - const int sizeProbingRequired = 0x1000; + const int STACK_PROBE_PAGE_SIZE_BYTES = 4096; + const int STACK_PROBE_BOUNDARY_THRESHOLD_BYTES = 1024; - if (size < sizeProbingRequired) + int lastProbedLocToFinalSp = size; + + if (size < STACK_PROBE_PAGE_SIZE_BYTES) { // sub esp, size offset = SKIP_ARITH_REG(size, base, offset); + } + else + { + WORD wOpcode = *(PTR_WORD)(base + offset); - const int STACK_PROBE_BOUNDARY_THRESHOLD_BYTES = 1024; + if (CheckInstrWord(wOpcode, X86_INSTR_w_TEST_ESP_DWORD_OFFSET_EAX)) + { + // In .NET 5.0 and earlier JIT inlines one or two 'test eax, [esp-dwOffset]' instruction + // sequence for a frame that has size smaller than 0x3000 bytes. + _ASSERTE(size < 0x3000); - unsigned delta = 0; + // test eax, [esp-0x1000] + offset += 7; + lastProbedLocToFinalSp -= 0x1000; - if (size + STACK_PROBE_BOUNDARY_THRESHOLD_BYTES > sizeProbingRequired) - { + if (size >= 0x2000) + { #ifdef _DEBUG - WORD wOpcode = *(PTR_WORD)(base+offset); - _ASSERTE(CheckInstrWord(wOpcode, X86_INSTR_w_TEST_ESP_EAX)); + wOpcode = *(PTR_WORD)(base + offset); + _ASSERTE(CheckInstrWord(wOpcode, X86_INSTR_w_TEST_ESP_DWORD_OFFSET_EAX)); #endif - // test [esp], eax - delta = 3; - } + //test eax, [esp-0x2000] + offset += 7; + lastProbedLocToFinalSp -= 0x1000; + } - return (offset+delta); - } - else - { - if (CheckInstrByte(base[offset], X86_INSTR_PUSH_EAX)) - { - // push eax - offset = SKIP_PUSH_REG(base, offset); - // lea eax, [esp-size+4] - offset = SKIP_LEA_EAX_ESP(-size+4, base, offset); - // call JIT_StackProbe - offset = SKIP_HELPER_CALL(base, offset); - // pop eax - offset = SKIP_POP_REG(base, offset); // sub esp, size - return (SKIP_ARITH_REG(size, base, offset)); + offset = SKIP_ARITH_REG(size, base, offset); } else { - // lea eax, [esp-size] - offset = SKIP_LEA_EAX_ESP(-size, base, offset); - // call JIT_StackProbe - offset = SKIP_HELPER_CALL(base, offset); - // mov esp, eax - return (SKIP_MOV_REG_REG(base, offset)); + bool pushedStubParam = false; + + if (CheckInstrByte(base[offset], X86_INSTR_PUSH_EAX)) + { + // push eax + offset = SKIP_PUSH_REG(base, offset); + pushedStubParam = true; + } + + if (CheckInstrByte(base[offset], X86_INSTR_XOR)) + { + // In .NET Core 3.1 and earlier JIT inlines stack probing loop + // for a frame that has size greater than or equal to 0x3000 bytes. + _ASSERTE(size >= 0x3000); + + offset += 2; + // xor eax, eax 2 + // [nop] 0-3 + // loop: + // test [esp + eax], eax 3 + // sub eax, 0x1000 5 + // cmp eax, -size 5 + // jge loop 2 + + // R2R images that support ReJIT may have extra nops we need to skip over + while (offset < 5) + { + if (CheckInstrByte(base[offset], X86_INSTR_NOP)) + { + offset++; + } + else + { + break; + } + } + + offset += 15; + + if (pushedStubParam) + { + // pop eax + offset = SKIP_POP_REG(base, offset); + } + + lastProbedLocToFinalSp = 0; + } + else + { + // In .NET 5.0 and later JIT emits a call to JIT_StackProbe helper. + + if (pushedStubParam) + { + // lea eax, [esp-size+4] + offset = SKIP_LEA_EAX_ESP(-size + 4, base, offset); + // call JIT_StackProbe + offset = SKIP_HELPER_CALL(base, offset); + // pop eax + offset = SKIP_POP_REG(base, offset); + // sub esp, size + return SKIP_ARITH_REG(size, base, offset); + } + else + { + // lea eax, [esp-size] + offset = SKIP_LEA_EAX_ESP(-size, base, offset); + // call JIT_StackProbe + offset = SKIP_HELPER_CALL(base, offset); + // mov esp, eax + return SKIP_MOV_REG_REG(base, offset); + } + } } } -} + if (lastProbedLocToFinalSp + STACK_PROBE_BOUNDARY_THRESHOLD_BYTES > STACK_PROBE_PAGE_SIZE_BYTES) + { +#ifdef _DEBUG + WORD wOpcode = *(PTR_WORD)(base + offset); + _ASSERTE(CheckInstrWord(wOpcode, X86_INSTR_w_TEST_ESP_EAX)); +#endif + // test [esp], eax + offset += 3; + } + + return offset; +} #endif // !USE_GC_INFO_DECODER From d864022f84ebf90d128eb2eae7e3e8b53b471dd4 Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Thu, 17 Dec 2020 21:18:41 -0800 Subject: [PATCH 09/12] Update src/coreclr/vm/eetwain.cpp and include missing SKIP_ARITH_REG for 3.1 stack probing loop --- src/coreclr/vm/eetwain.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index f5635a873720a3..a7e5e013d0eb67 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -3126,7 +3126,7 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) if (size == sizeof(void*)) { // JIT emits "push eax" instead of "sub esp,4" - return (SKIP_PUSH_REG(base, offset)); + return SKIP_PUSH_REG(base, offset); } const int STACK_PROBE_PAGE_SIZE_BYTES = 4096; @@ -3145,8 +3145,8 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) if (CheckInstrWord(wOpcode, X86_INSTR_w_TEST_ESP_DWORD_OFFSET_EAX)) { - // In .NET 5.0 and earlier JIT inlines one or two 'test eax, [esp-dwOffset]' instruction - // sequence for a frame that has size smaller than 0x3000 bytes. + // In .NET 5.0 and earlier for frames that have size smaller than 0x3000 bytes + // JIT emits one or two 'test eax, [esp-dwOffset]' instructions before adjusting the stack pointer. _ASSERTE(size < 0x3000); // test eax, [esp-0x1000] @@ -3180,8 +3180,8 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) if (CheckInstrByte(base[offset], X86_INSTR_XOR)) { - // In .NET Core 3.1 and earlier JIT inlines stack probing loop - // for a frame that has size greater than or equal to 0x3000 bytes. + // In .NET Core 3.1 and earlier for frames that have size greater than or equal to 0x3000 bytes + // JIT emits the following loop. _ASSERTE(size >= 0x3000); offset += 2; @@ -3193,7 +3193,7 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) // cmp eax, -size 5 // jge loop 2 - // R2R images that support ReJIT may have extra nops we need to skip over + // R2R images that support ReJIT may have extra nops we need to skip over. while (offset < 5) { if (CheckInstrByte(base[offset], X86_INSTR_NOP)) @@ -3214,7 +3214,8 @@ unsigned SKIP_ALLOC_FRAME(int size, PTR_CBYTE base, unsigned offset) offset = SKIP_POP_REG(base, offset); } - lastProbedLocToFinalSp = 0; + // sub esp, size + return SKIP_ARITH_REG(size, base, offset); } else { From b5e174400db5e3ed5118127a7c43b09bad41dc5e Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Thu, 17 Dec 2020 21:34:51 -0800 Subject: [PATCH 10/12] Update test and include more scenarios --- .../JitBlue/Runtime_45090/Runtime_45090.cs | 157 ++++++++++++++++-- 1 file changed, 147 insertions(+), 10 deletions(-) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs b/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs index 0bc96bb1e5b5a2..b9469ec419f50a 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_45090/Runtime_45090.cs @@ -8,6 +8,22 @@ #pragma warning disable CS0649 +// This test is to validate that SKIP_ALLOC_FRAME() procedure used by UnwindEspFrameProlog() in eetwain.cpp +// functions correctly on win-x86. The platform doesn't have unwind information as other platforms and hence unwinder either uses ebp chain of the stack frames +// or, in a case of an esp-based frame, unwinds the frame "manually" by decoding instructions in a function prolog. +// +// In order to validate that the unwinding procedure works correctly with such frames: +// +// 1) We need to construct a method with esp-based frame. +// That means that such method cannot call other methods (i.e. it must be a leaf method). +// The method also should not be inlined into its caller. +// However, we cannot simply decorate the method with NoInlining attribute, since the method in that case would have JIT_FLAG_FRAMED set by VM and will have ebp-based frame. +// Therefore, in order to prevent such method from inlining it can be declared as virtual. +// +// 2) There are a number of different options for the JIT to allocate a frame of a method. Such frame may or may not require stack probing. +// All of these options need to be tested. +// This include testing that runtime is back compatible and can run (and unwind) R2R code generated by earlier versions of the JIT. + namespace Runtime_45090 { struct _4 @@ -82,6 +98,32 @@ struct _1000 public _100 _f; } + struct _1e00 + { + public _1000 _0; + public _e00 _1; + } + + struct _2000 + { + public _1000 _0; + public _1000 _1; + } + + struct _2e00 + { + public _1000 _0; + public _1000 _1; + public _e00 _2; + } + + struct _3000 + { + public _1000 _0; + public _1000 _1; + public _1000 _2; + } + abstract class AllocFrame { public abstract int VirtMethodEspBasedFrame(); @@ -89,7 +131,8 @@ abstract class AllocFrame class PushReg : AllocFrame { - // Frame size is 4 bytes and allocated with 'push eax' instruction. + // Frame size is 4 bytes + // push eax public unsafe override int VirtMethodEspBasedFrame() { _4 tmp = new _4(); @@ -98,9 +141,10 @@ public unsafe override int VirtMethodEspBasedFrame() } } - class NoProbe : AllocFrame + class SubSp : AllocFrame { - // Frame size is less than 0x1000 bytes and doesn't require probing. + // Frame size is such that it doesn't require probing + // sub esp, #frameSize public override int VirtMethodEspBasedFrame() { _100 tmp = new _100(); @@ -108,9 +152,11 @@ public override int VirtMethodEspBasedFrame() } } - class InlineProbe : AllocFrame + class ProbeAfterSubSp : AllocFrame { - // Frame size is less than 0x1000 bytes and uses inline probing with 'test eax, [esp]' instruction. + // Frame size is smaller than 0x1000 bytes, but needs probing **after** allocation + // sub esp, #frameSize + // test eax, [esp] public override int VirtMethodEspBasedFrame() { _e00 tmp = new _e00(); @@ -118,9 +164,16 @@ public override int VirtMethodEspBasedFrame() } } - class HelperProbe : AllocFrame + class ProbeBeforeSubSp1 : AllocFrame { - // Frame is greater than 0x1000 bytes and uses JIT_StackProbe helper. + // Frame size is in range [0x1000, 0x2000) bytes + // Since 6.0 + // lea eax, [esp-#frameSize] + // call JIT_StackProbe + // mov esp, eax + // Before 6.0 + // test [esp-0x1000], eax + // sub esp, #frameSize public override int VirtMethodEspBasedFrame() { _1000 tmp = new _1000(); @@ -128,6 +181,86 @@ public override int VirtMethodEspBasedFrame() } } + class ProbeBeforeSubSp2 : AllocFrame + { + // Frame size is in range [0x1000, 0x2000) bytes and needs additional probing **after** allocation + // Since 6.0 + // lea eax, [esp-#frameSize] + // call JIT_StackProbe + // mov esp, eax + // Before 6.0 + // test [esp-0x1000], eax + // sub esp, #frameSize + // test [esp], eax + public override int VirtMethodEspBasedFrame() + { + _1e00 tmp = new _1e00(); + return (int)tmp._0._0._0._0; + } + } + + class ProbeBeforeSubSp3 : AllocFrame + { + // Frame size is in range [0x2000, 0x3000) bytes + // Since 6.0 + // lea eax, [esp-#frameSize] + // call JIT_StackProbe + // mov esp, eax + // Before 6.0 + // test [esp-0x1000], eax + // test [esp-0x2000], eax + // sub esp, #frameSize + public override int VirtMethodEspBasedFrame() + { + _2000 tmp = new _2000(); + return (int)tmp._0._0._0._0; + } + } + + class ProbeBeforeSubSp4 : AllocFrame + { + // Frame size is in range [0x2000, 0x3000) bytes and needs additional probing **after** allocation + // + // Since 6.0 + // lea eax, [esp-#frameSize] + // call JIT_StackProbe + // mov esp, eax + // Before 6.0 + // test [esp-0x1000], eax + // test [esp-0x2000], eax + // sub esp, #frameSize + // test [esp], eax + public override int VirtMethodEspBasedFrame() + { + _2e00 tmp = new _2e00(); + return (int)tmp._0._0._0._0; + } + } + + class ProbeBeforeSubSp5 : AllocFrame + { + // Frame size is in range [0x3000, 0x3000) bytes + // + // Since 5.0 + // lea eax, [esp-#frameSize] + // call JIT_StackProbe + // mov esp, eax + // Before 5.0 + // xor eax, eax + // [nop] + // loop: + // test [esp + eax], eax + // sub eax, 0x1000 + // cmp eax, -#frameSize + // jge loop + // sub esp, #frameSize + public override int VirtMethodEspBasedFrame() + { + _3000 tmp = new _3000(); + return (int)tmp._0._0._0._0; + } + } + class Program { [MethodImpl(MethodImplOptions.NoInlining)] @@ -139,9 +272,13 @@ static void TestSkipAllocFrame(AllocFrame scenario) static int Main(string[] args) { TestSkipAllocFrame(new PushReg()); - TestSkipAllocFrame(new NoProbe()); - TestSkipAllocFrame(new InlineProbe()); - TestSkipAllocFrame(new HelperProbe()); + TestSkipAllocFrame(new SubSp()); + TestSkipAllocFrame(new ProbeAfterSubSp()); + TestSkipAllocFrame(new ProbeBeforeSubSp1()); + TestSkipAllocFrame(new ProbeBeforeSubSp2()); + TestSkipAllocFrame(new ProbeBeforeSubSp3()); + TestSkipAllocFrame(new ProbeBeforeSubSp4()); + TestSkipAllocFrame(new ProbeBeforeSubSp5()); return 100; } From 3fd8d148a3a5e4c69a24dd2be38e5257b9b4237a Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Thu, 17 Dec 2020 21:38:26 -0800 Subject: [PATCH 11/12] Update R2R version to 5.1 in src/coreclr/inc/readytorun.h src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs --- src/coreclr/inc/readytorun.h | 4 ++-- src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/inc/readytorun.h b/src/coreclr/inc/readytorun.h index 5122aa52610b98..4b68acba0cdc8f 100644 --- a/src/coreclr/inc/readytorun.h +++ b/src/coreclr/inc/readytorun.h @@ -15,8 +15,8 @@ #define READYTORUN_SIGNATURE 0x00525452 // 'RTR' // Keep these in sync with src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs -#define READYTORUN_MAJOR_VERSION 0x0004 -#define READYTORUN_MINOR_VERSION 0x0002 +#define READYTORUN_MAJOR_VERSION 0x0005 +#define READYTORUN_MINOR_VERSION 0x0001 #define MINIMUM_READYTORUN_MAJOR_VERSION 0x003 diff --git a/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs b/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs index e003ff6c930099..cdd5bac289a2c3 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs @@ -14,8 +14,8 @@ internal struct ReadyToRunHeaderConstants { public const uint Signature = 0x00525452; // 'RTR' - public const ushort CurrentMajorVersion = 4; - public const ushort CurrentMinorVersion = 2; + public const ushort CurrentMajorVersion = 5; + public const ushort CurrentMinorVersion = 1; } #pragma warning disable 0169 From a2f2cc70b46cf132f225cdaaa10e92721a7534ba Mon Sep 17 00:00:00 2001 From: Egor Chesakov Date: Mon, 4 Jan 2021 14:02:35 -0800 Subject: [PATCH 12/12] Revert "Remove padding for ReJit in CodeGen::genAllocLclFrame in src/coreclr/jit/codegenxarch.cpp" This reverts commit 0b88d716a4af8165dbfa73a09857778ca68a5dff. --- src/coreclr/jit/codegenxarch.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index c919818655a4f6..b3cc225dd411d4 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -2009,6 +2009,8 @@ void CodeGen::genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pIni GetEmitter()->emitIns_R_AR(INS_lea, EA_PTRSIZE, REG_STACK_PROBE_HELPER_ARG, REG_SPBASE, spOffset); regSet.verifyRegUsed(REG_STACK_PROBE_HELPER_ARG); + // Can't have a call until we have enough padding for ReJit. + genPrologPadForReJit(); genEmitHelperCall(CORINFO_HELP_STACK_PROBE, 0, EA_UNKNOWN); if (compiler->info.compPublishStubParam) @@ -2027,6 +2029,8 @@ void CodeGen::genAllocLclFrame(unsigned frameSize, regNumber initReg, bool* pIni GetEmitter()->emitIns_R_AR(INS_lea, EA_PTRSIZE, REG_STACK_PROBE_HELPER_ARG, REG_SPBASE, -(int)frameSize); regSet.verifyRegUsed(REG_STACK_PROBE_HELPER_ARG); + // Can't have a call until we have enough padding for ReJit. + genPrologPadForReJit(); genEmitHelperCall(CORINFO_HELP_STACK_PROBE, 0, EA_UNKNOWN); if (initReg == REG_DEFAULT_HELPER_CALL_TARGET)