From 5ed389b783e839b2ea8c6adac9957c3a89d1f8f7 Mon Sep 17 00:00:00 2001
From: Sergey Andreenko <seandree@microsoft.com>
Date: Thu, 5 Nov 2020 14:17:58 -1000
Subject: [PATCH] Forbid `- byref cnst` -> `+ (byref -cnst)` transformation.
 (#44266)

* Add a repro test.

* Forbid the transformation for byrefs.

* Update src/coreclr/src/jit/morph.cpp

Co-authored-by: Andy Ayers <andya@microsoft.com>

* Update src/coreclr/src/jit/morph.cpp

* Fix the test return value.

WriteLine is just to make sure we don't delete the value.

* improve the test.

avoid a possible overflow and don't waste time on printing.

Co-authored-by: Andy Ayers <andya@microsoft.com>
---
 src/coreclr/src/jit/morph.cpp                      |  5 +-
 .../JitBlue/Runtime_44266/Runtime_44266.il         | 63 ++++++++++++++++++++++
 .../JitBlue/Runtime_44266/Runtime_44266.ilproj     | 10 ++++
 3 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.il
 create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.ilproj

diff --git a/src/coreclr/src/jit/morph.cpp b/src/coreclr/src/jit/morph.cpp
index d74e851..e26fc7d 100644
--- a/src/coreclr/src/jit/morph.cpp
+++ b/src/coreclr/src/jit/morph.cpp
@@ -13354,9 +13354,10 @@ DONE_MORPHING_CHILDREN:
                 /* Check for "op1 - cns2" , we change it to "op1 + (-cns2)" */
 
                 noway_assert(op2);
-                if (op2->IsCnsIntOrI())
+                if (op2->IsCnsIntOrI() && !op2->IsIconHandle())
                 {
-                    /* Negate the constant and change the node to be "+" */
+                    // Negate the constant and change the node to be "+",
+                    // except when `op2` is a const byref.
 
                     op2->AsIntConCommon()->SetIconValue(-op2->AsIntConCommon()->IconValue());
                     op2->AsIntConRef().gtFieldSeq = FieldSeqStore::NotAField();
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.il b/src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.il
new file mode 100644
index 0000000..b51e898
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.il
@@ -0,0 +1,63 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// This test shows an inlining of `byref LCL_VAR_ADDR - byref CNST_INT` method that returns a native int.
+// However, Jit could try to optimize `-` as `+ -CNST_INT` that could lead to an incorrect `long + (-byref)`.
+
+.assembly extern System.Console {}
+.assembly extern legacy library mscorlib {}
+.assembly byrefsubbyref1 { }
+.class a extends [mscorlib]System.Object
+{
+  .field static class ctest S_1
+  .method public static native int byrefsubbyref(class ctest& V_1, class ctest& V_2) aggressiveinlining
+  {
+    ldarg 0
+    ldarg 1
+    sub
+    ret
+  }
+
+  .method public static int32 main() cil managed
+  {
+    .entrypoint
+    .maxstack  2
+    .locals init (class ctest V_1,
+             class ctest V_2,
+             native int V_3)
+    newobj     instance void ctest::.ctor()
+    stloc.0
+    newobj     instance void ctest::.ctor()
+    dup
+    stsfld class ctest a::S_1
+    stloc.1
+
+    ldloca V_1
+    ldsflda class ctest a::S_1
+    call native int a::byrefsubbyref(class ctest&, class ctest&)
+    stloc V_3
+    ldloc V_3
+    call       void a::KeepA(native int)
+    ldc.i4.s   100    
+    ret
+  }
+  
+  .method public hidebysig static void  KeepA(native int a) cil managed noinlining
+  {
+    .maxstack  8
+    ret
+  }
+}
+
+.class private auto ansi ctest
+       extends [mscorlib]System.Object
+{
+  .method public specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    .maxstack  1
+    ldarg.0
+    call       instance void [mscorlib]System.Object::.ctor()
+    ret
+  } 
+}
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.ilproj b/src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.ilproj
new file mode 100644
index 0000000..7dab57f
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_44266/Runtime_44266.ilproj
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk.IL">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).il" />
+  </ItemGroup>
+</Project>
-- 
2.7.4