From 4aa8253cedbd3c35d7edb32344f99901c5725059 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 15 Nov 2021 09:39:03 +0100 Subject: [PATCH 1/5] Disable poisoning for large structs For very large structs (> 64K in size) poisoning could end up generating instructions requiring larger local var offsets than we can handle. This hits IMPL_LIMIT that throws InvalidProgramException. Turn off poisoning for larger structs that require more than 16 movs to also avoid the significant code bloat by the singular movs. This is a less risky version of #61521 for backporting to .NET 6. Fix #60852 --- src/coreclr/jit/codegencommon.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index dcf1768c60a7dd..f415b15ef8d580 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -12551,6 +12551,17 @@ void CodeGen::genPoisonFrame(regMaskTP regLiveIn) assert(varDsc->lvOnFrame); + int size = (int)compiler->lvaLclSize(varNum); + + if (size / TARGET_POINTER_SIZE > 16) + { + // For very large structs the offsets in the movs we emit below can + // grow too large to be handled properly by JIT. Furthermore, while + // this is only debug code, for very large structs this can bloat + // the code too much due to the singular movs used. + continue; + } + if (!hasPoisonImm) { #ifdef TARGET_64BIT @@ -12568,7 +12579,6 @@ void CodeGen::genPoisonFrame(regMaskTP regLiveIn) #else int addr = 0; #endif - int size = (int)compiler->lvaLclSize(varNum); int end = addr + size; for (int offs = addr; offs < end;) { From 402a1f1899fa1e872f271ee16ea4d86d616c9738 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 15 Nov 2021 09:46:13 +0100 Subject: [PATCH 2/5] Run jit-format --- src/coreclr/jit/codegencommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index f415b15ef8d580..c1df7a106e5f30 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -12579,7 +12579,7 @@ void CodeGen::genPoisonFrame(regMaskTP regLiveIn) #else int addr = 0; #endif - int end = addr + size; + int end = addr + size; for (int offs = addr; offs < end;) { #ifdef TARGET_64BIT From 9c4ca5fea506e9529941573e919dce4c3697a2af Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 15 Nov 2021 16:35:35 +0100 Subject: [PATCH 3/5] Add regression test --- src/tests/JIT/Directed/debugging/poison.cs | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/tests/JIT/Directed/debugging/poison.cs b/src/tests/JIT/Directed/debugging/poison.cs index 463894a4e6a35d..cd6de033d3dc3d 100644 --- a/src/tests/JIT/Directed/debugging/poison.cs +++ b/src/tests/JIT/Directed/debugging/poison.cs @@ -19,6 +19,22 @@ public static unsafe int Main() WithoutGCRef poisoned2; Unsafe.SkipInit(out poisoned2); result &= VerifyPoison(&poisoned2, sizeof(WithoutGCRef)); + + Massive poisoned3; + Unsafe.SkipInit(out poisoned3); + result &= VerifyPoison(&poisoned3, sizeof(Massive)); + + WithoutGCRef poisoned4; + Unsafe.SkipInit(out poisoned4); + result &= VerifyPoison(&poisoned4, sizeof(WithoutGCRef)); + + Massive poisoned5; + Unsafe.SkipInit(out poisoned5); + result &= VerifyPoison(&poisoned5, sizeof(Massive)); + + GCRef zeroed2; + Unsafe.SkipInit(out zeroed2); + result &= VerifyZero(Unsafe.AsPointer(ref zeroed2), Unsafe.SizeOf()); return result ? 100 : 101; } @@ -53,4 +69,9 @@ private struct WithoutGCRef public int ANumber; public float AFloat; } -} + + private unsafe struct Massive + { + public fixed byte Bytes[0x10008]; + } +} \ No newline at end of file From ae69d12ec8c4940424fefffae8e8f62e57f69923 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 15 Nov 2021 17:36:38 +0100 Subject: [PATCH 4/5] Update src/coreclr/jit/codegencommon.cpp Co-authored-by: Andy Ayers --- src/coreclr/jit/codegencommon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index c1df7a106e5f30..f3e2566330da66 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -12553,7 +12553,7 @@ void CodeGen::genPoisonFrame(regMaskTP regLiveIn) int size = (int)compiler->lvaLclSize(varNum); - if (size / TARGET_POINTER_SIZE > 16) + if ((size / TARGET_POINTER_SIZE) > 16) { // For very large structs the offsets in the movs we emit below can // grow too large to be handled properly by JIT. Furthermore, while From a11aba82d31cc7cf4b450ec655741d12e45ae073 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 15 Nov 2021 19:20:14 +0100 Subject: [PATCH 5/5] Don't check poisoning for large struct in test Since it's disabled, there is nothing to check. --- src/tests/JIT/Directed/debugging/poison.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/tests/JIT/Directed/debugging/poison.cs b/src/tests/JIT/Directed/debugging/poison.cs index cd6de033d3dc3d..81c669a8037282 100644 --- a/src/tests/JIT/Directed/debugging/poison.cs +++ b/src/tests/JIT/Directed/debugging/poison.cs @@ -20,17 +20,19 @@ public static unsafe int Main() Unsafe.SkipInit(out poisoned2); result &= VerifyPoison(&poisoned2, sizeof(WithoutGCRef)); - Massive poisoned3; - Unsafe.SkipInit(out poisoned3); - result &= VerifyPoison(&poisoned3, sizeof(Massive)); + Massive notPoisoned; + Unsafe.SkipInit(out notPoisoned); + // too large to be poisoned, just expose it but don't check return value + VerifyPoison(¬Poisoned, sizeof(Massive)); WithoutGCRef poisoned4; Unsafe.SkipInit(out poisoned4); result &= VerifyPoison(&poisoned4, sizeof(WithoutGCRef)); - Massive poisoned5; - Unsafe.SkipInit(out poisoned5); - result &= VerifyPoison(&poisoned5, sizeof(Massive)); + Massive notPoisoned2; + Unsafe.SkipInit(out notPoisoned2); + // too large to be poisoned, just expose it but don't check return value + VerifyPoison(¬Poisoned2, sizeof(Massive)); GCRef zeroed2; Unsafe.SkipInit(out zeroed2);