From 858019ca1a5fb458f790789437dfa7029a1169b0 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 4 Jul 2022 18:24:39 +0300 Subject: [PATCH 1/5] Add a test for the "src" case --- .../JitBlue/Runtime_71601/Runtime_71601.cs | 41 +++++++++++++++++++ .../Runtime_71601/Runtime_71601.csproj | 9 ++++ 2 files changed, 50 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs new file mode 100644 index 00000000000000..605dcee45d3bbf --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs @@ -0,0 +1,41 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Numerics; +using System.Runtime.CompilerServices; + +public class Runtime_71601 +{ + public static int Main() + { + if (ProblemWithPrimitiveSrc()) + { + return 101; + } + + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool ProblemWithPrimitiveSrc() + { + WrapTuple p = new WrapTuple { FieldOne = { Value = 1 }, FieldTwo = { Value = 2 } }; + Wrap a = p.FieldTwo; + Wrap b = WrapTuple.GetFieldTwo(ref p); + + return a.Value != b.Value; + } + + struct WrapTuple + { + public Wrap FieldOne; + public Wrap FieldTwo; + + public static ref Wrap GetFieldTwo(ref WrapTuple t) => ref Unsafe.Add(ref t.FieldOne, 1); + } + + struct Wrap + { + public int Value; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.csproj new file mode 100644 index 00000000000000..f492aeac9d056b --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + \ No newline at end of file From c278d3f1971d9937325d5c28963fd429908a2258 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 4 Jul 2022 18:57:04 +0300 Subject: [PATCH 2/5] Fix the "src" case --- src/coreclr/jit/morphblock.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 7a19e46d09eead..b46b90500c1ad2 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -566,7 +566,18 @@ void MorphCopyBlockHelper::PrepareSrc() ssize_t srcLclOffset = 0; if (m_src->AsIndir()->Addr()->DefinesLocalAddr(&m_srcLclNode, &srcLclOffset)) { - m_srcLclOffset = static_cast(srcLclOffset); + // Treat out-of-bounds access to locals opaquely to simplify downstream logic. + unsigned srcLclSize = m_comp->lvaLclExactSize(m_srcLclNode->GetLclNum()); + + if (!FitsIn(srcLclOffset) || ((static_cast(srcLclOffset) + m_blockSize) > srcLclSize)) + { + assert(m_comp->lvaGetDesc(m_srcLclNode)->IsAddressExposed()); + m_srcLclNode = nullptr; + } + else + { + m_srcLclOffset = static_cast(srcLclOffset); + } } } From 6d3ed5ab3e1650e6f7261bdddb429b1a40718a7f Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 4 Jul 2022 19:20:54 +0300 Subject: [PATCH 3/5] Add a test for the "dst" case --- .../JitBlue/Runtime_71601/Runtime_71601.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs index 605dcee45d3bbf..51fedb1ce88e6c 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs @@ -13,6 +13,11 @@ public static int Main() return 101; } + if (ProblemWithPrimitiveDst()) + { + return 102; + } + return 100; } @@ -26,6 +31,16 @@ private static bool ProblemWithPrimitiveSrc() return a.Value != b.Value; } + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool ProblemWithPrimitiveDst() + { + WrapTuple p = new WrapTuple { FieldOne = { Value = 1 }, FieldTwo = { Value = 2 } }; + Wrap a = p.FieldTwo; + WrapTuple.GetFieldTwo(ref p) = a; + + return a.Value == p.FieldOne.Value; + } + struct WrapTuple { public Wrap FieldOne; From 9e12b3c08ecf0f145f3c5795823fcb12899a1f84 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 4 Jul 2022 19:35:15 +0300 Subject: [PATCH 4/5] Fix the "dst" case --- src/coreclr/jit/morphblock.cpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index b46b90500c1ad2..6ab429a34f9491 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -192,12 +192,10 @@ void MorphInitBlockHelper::PrepareDst() if (m_dst->IsLocal()) { m_dstLclNode = m_dst->AsLclVarCommon(); - m_dstVarDsc = m_comp->lvaGetDesc(m_dstLclNode); m_dstLclOffset = m_dstLclNode->GetLclOffs(); if (m_dst->TypeIs(TYP_STRUCT)) { - assert(m_dstVarDsc->GetLayout()->GetSize() == m_dstVarDsc->lvExactSize); m_blockLayout = m_dstLclNode->GetLayout(m_comp); } } @@ -212,9 +210,19 @@ void MorphInitBlockHelper::PrepareDst() ssize_t dstLclOffset = 0; if (dstAddr->DefinesLocalAddr(&m_dstLclNode, &dstLclOffset)) { - // Note that lclNode can be a field, like `BLK<4> struct(ADD(ADDR(LCL_FLD int), CNST_INT))`. - m_dstVarDsc = m_comp->lvaGetDesc(m_dstLclNode); - m_dstLclOffset = static_cast(dstLclOffset); + // Treat out-of-bounds access to locals opaquely to simplify downstream logic. + unsigned dstLclSize = m_comp->lvaLclExactSize(m_dstLclNode->GetLclNum()); + + if (!FitsIn(dstLclOffset) || + ((static_cast(dstLclOffset) + m_dst->AsIndir()->Size()) > dstLclSize)) + { + assert(m_comp->lvaGetDesc(m_dstLclNode)->IsAddressExposed()); + m_dstLclNode = nullptr; + } + else + { + m_dstLclOffset = static_cast(dstLclOffset); + } } if (m_dst->TypeIs(TYP_STRUCT)) @@ -236,6 +244,10 @@ void MorphInitBlockHelper::PrepareDst() if (m_dstLclNode != nullptr) { m_dstLclNum = m_dstLclNode->GetLclNum(); + m_dstVarDsc = m_comp->lvaGetDesc(m_dstLclNum); + + assert((m_dstVarDsc->TypeGet() != TYP_STRUCT) || + (m_dstVarDsc->GetLayout()->GetSize() == m_dstVarDsc->lvExactSize)); // Kill everything about m_dstLclNum (and its field locals) if (m_comp->optLocalAssertionProp && (m_comp->optAssertionCount > 0)) From d46c875a5e4d4d0260a2e108da74ce362f29e9d9 Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Mon, 4 Jul 2022 19:51:23 +0300 Subject: [PATCH 5/5] Add tests for negative offsets --- .../JitBlue/Runtime_71601/Runtime_71601.cs | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs index 51fedb1ce88e6c..f71e0cf0d78d50 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_71601/Runtime_71601.cs @@ -28,7 +28,20 @@ private static bool ProblemWithPrimitiveSrc() Wrap a = p.FieldTwo; Wrap b = WrapTuple.GetFieldTwo(ref p); - return a.Value != b.Value; + if (a.Value != b.Value) + { + return true; + } + + a = p.FieldOne; + b = WrapTuple.GetFieldOne(ref p); + + if (a.Value != b.Value) + { + return true; + } + + return false; } [MethodImpl(MethodImplOptions.NoInlining)] @@ -38,7 +51,19 @@ private static bool ProblemWithPrimitiveDst() Wrap a = p.FieldTwo; WrapTuple.GetFieldTwo(ref p) = a; - return a.Value == p.FieldOne.Value; + if (a.Value == p.FieldOne.Value) + { + return true; + } + + WrapTuple.GetFieldOne(ref p) = a; + + if (a.Value != p.FieldOne.Value) + { + return true; + } + + return false; } struct WrapTuple @@ -46,6 +71,7 @@ struct WrapTuple public Wrap FieldOne; public Wrap FieldTwo; + public static ref Wrap GetFieldOne(ref WrapTuple t) => ref Unsafe.Add(ref t.FieldTwo, -1); public static ref Wrap GetFieldTwo(ref WrapTuple t) => ref Unsafe.Add(ref t.FieldOne, 1); }