Merge pull request #21751 from VSadov/signExtend

Force sign-extending cast when add/sub pointers and int32 in checked context

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs

@@ -2091,14 +2091,28 @@ private BoundExpression MakeSizeOfMultiplication(BoundExpression numericOperand,
                 {
                     case SpecialType.System_Int32:
                         // add operator can take int32 and extend to 64bit if necessary
+                        // however in a case of checked operation, the operation is treated as unsigned with overflow ( add.ovf.un , sub.ovf.un )
+                        // the IL spec is a bit vague whether JIT should sign or zero extend the shorter operand in such case
+                        // and there could be inconsistencies in implementation or bugs.
+                        // As a result, in checked contexts, we will force sign-extending cast to be sure
+                        if (isChecked)
+                        {
+                            var constVal = numericOperand.ConstantValue;
+                            if (constVal == null || constVal.Int32Value < 0)
+                            {
+                                destinationType = SpecialType.System_IntPtr;
+                            }
+                        }
                         break;
                     case SpecialType.System_UInt32:
-                        // add operator treats operands as signed and will sign-extend on x64
-                        // to prevent sign-extending, convert the operand to unsigned native int.
-                        var constVal = numericOperand.ConstantValue;
-                        if (constVal == null || constVal.UInt32Value > int.MaxValue)
                         {
-                            destinationType = SpecialType.System_UIntPtr;
+                            // add operator treats operands as signed and will sign-extend on x64
+                            // to prevent sign-extending, convert the operand to unsigned native int.
+                            var constVal = numericOperand.ConstantValue;
+                            if (constVal == null || constVal.UInt32Value > int.MaxValue)
+                            {
+                                destinationType = SpecialType.System_UIntPtr;
+                            }
                         }
                         break;
                     case SpecialType.System_Int64:

src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs

@@ -5513,6 +5513,7 @@ .maxstack  2
 ");
         }
 
+        [WorkItem(18871, "https://github.com/dotnet/roslyn/issues/18871")]
         [WorkItem(546750, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/546750")]
         [Fact]
         public void NumericAdditionChecked_SizeOne()
@@ -5536,6 +5537,25 @@ void Test(int i, uint u, long l, ulong ul)
             p = p + u;
             p = p + l;
             p = p + ul;
+            p = p + (-2);
+        }
+    }
+
+    void Test1(int i, uint u, long l, ulong ul)
+    {
+        checked
+        {
+            byte b = 3;
+            byte* p = &b;
+            p = p - 2;
+            p = p - 3u;
+            p = p - 4l;
+            p = p - 5ul;
+            p = p - i;
+            p = p - u;
+            p = p - l;
+            p = p - ul;
+            p = p - (-1);
         }
     }
 }
@@ -5543,9 +5563,11 @@ void Test(int i, uint u, long l, ulong ul)
             // NOTE: even when not optimized.
             // NOTE: additional conversions applied to constants of type int and uint.
             // NOTE: identical to unchecked except "add" becomes "add.ovf.un".
-            CompileAndVerify(text, options: TestOptions.UnsafeDebugDll).VerifyIL("C.Test", @"
+            var comp = CompileAndVerify(text, options: TestOptions.UnsafeDebugDll);
+
+            comp.VerifyIL("C.Test", @"
 {
-  // Code size       50 (0x32)
+  // Code size       57 (0x39)
   .maxstack  2
   .locals init (byte V_0, //b
                 byte* V_1) //p
@@ -5578,28 +5600,191 @@ .maxstack  2
   IL_001b:  stloc.1
   IL_001c:  ldloc.1
   IL_001d:  ldarg.1
-  IL_001e:  add.ovf.un
-  IL_001f:  stloc.1
-  IL_0020:  ldloc.1
-  IL_0021:  ldarg.2
-  IL_0022:  conv.u
-  IL_0023:  add.ovf.un
-  IL_0024:  stloc.1
-  IL_0025:  ldloc.1
-  IL_0026:  ldarg.3
-  IL_0027:  conv.i
-  IL_0028:  add.ovf.un
-  IL_0029:  stloc.1
-  IL_002a:  ldloc.1
-  IL_002b:  ldarg.s    V_4
-  IL_002d:  conv.u
-  IL_002e:  add.ovf.un
-  IL_002f:  stloc.1
-  IL_0030:  nop
-  IL_0031:  ret
+  IL_001e:  conv.i
+  IL_001f:  add.ovf.un
+  IL_0020:  stloc.1
+  IL_0021:  ldloc.1
+  IL_0022:  ldarg.2
+  IL_0023:  conv.u
+  IL_0024:  add.ovf.un
+  IL_0025:  stloc.1
+  IL_0026:  ldloc.1
+  IL_0027:  ldarg.3
+  IL_0028:  conv.i
+  IL_0029:  add.ovf.un
+  IL_002a:  stloc.1
+  IL_002b:  ldloc.1
+  IL_002c:  ldarg.s    V_4
+  IL_002e:  conv.u
+  IL_002f:  add.ovf.un
+  IL_0030:  stloc.1
+  IL_0031:  ldloc.1
+  IL_0032:  ldc.i4.s   -2
+  IL_0034:  conv.i
+  IL_0035:  add.ovf.un
+  IL_0036:  stloc.1
+  IL_0037:  nop
+  IL_0038:  ret
+}");
+
+            comp.VerifyIL("C.Test1", @"
+{
+  // Code size       56 (0x38)
+  .maxstack  2
+  .locals init (byte V_0, //b
+                byte* V_1) //p
+  IL_0000:  nop
+  IL_0001:  nop
+  IL_0002:  ldc.i4.3
+  IL_0003:  stloc.0
+  IL_0004:  ldloca.s   V_0
+  IL_0006:  conv.u
+  IL_0007:  stloc.1
+  IL_0008:  ldloc.1
+  IL_0009:  ldc.i4.2
+  IL_000a:  sub.ovf.un
+  IL_000b:  stloc.1
+  IL_000c:  ldloc.1
+  IL_000d:  ldc.i4.3
+  IL_000e:  sub.ovf.un
+  IL_000f:  stloc.1
+  IL_0010:  ldloc.1
+  IL_0011:  ldc.i4.4
+  IL_0012:  conv.i8
+  IL_0013:  conv.i
+  IL_0014:  sub.ovf.un
+  IL_0015:  stloc.1
+  IL_0016:  ldloc.1
+  IL_0017:  ldc.i4.5
+  IL_0018:  conv.i8
+  IL_0019:  conv.u
+  IL_001a:  sub.ovf.un
+  IL_001b:  stloc.1
+  IL_001c:  ldloc.1
+  IL_001d:  ldarg.1
+  IL_001e:  conv.i
+  IL_001f:  sub.ovf.un
+  IL_0020:  stloc.1
+  IL_0021:  ldloc.1
+  IL_0022:  ldarg.2
+  IL_0023:  conv.u
+  IL_0024:  sub.ovf.un
+  IL_0025:  stloc.1
+  IL_0026:  ldloc.1
+  IL_0027:  ldarg.3
+  IL_0028:  conv.i
+  IL_0029:  sub.ovf.un
+  IL_002a:  stloc.1
+  IL_002b:  ldloc.1
+  IL_002c:  ldarg.s    V_4
+  IL_002e:  conv.u
+  IL_002f:  sub.ovf.un
+  IL_0030:  stloc.1
+  IL_0031:  ldloc.1
+  IL_0032:  ldc.i4.m1
+  IL_0033:  conv.i
+  IL_0034:  sub.ovf.un
+  IL_0035:  stloc.1
+  IL_0036:  nop
+  IL_0037:  ret
 }");
         }
 
+        [Fact]
+        public void CheckedSignExtend()
+        {
+            var text = @"
+using System;
+
+unsafe struct S
+{
+    static void Main()
+    {
+        byte* ptr1 = default(byte*);
+
+        ptr1 = (byte*)2;
+        checked
+        {
+            // should not overflow regardless of 32/64 bit
+            ptr1 = ptr1 + 2147483649;
+        }
+
+        Console.WriteLine((long)ptr1);
+
+        byte* ptr = (byte*)2;
+        try
+        { 
+            checked
+            {
+                int i = -1;
+                // should overflow regardless of 32/64 bit
+                ptr = ptr + i;
+            }
+            Console.WriteLine((long)ptr);
+        }
+        catch (OverflowException)
+        {
+            Console.WriteLine(""overflow"");
+            Console.WriteLine((long)ptr);
+        }
+    }
+}
+";
+
+            CompileAndVerify(text, options: TestOptions.UnsafeReleaseExe, expectedOutput: @"2147483651
+overflow
+2").VerifyIL("S.Main", @"
+{
+  // Code size       67 (0x43)
+  .maxstack  2
+  .locals init (byte* V_0, //ptr1
+                byte* V_1, //ptr
+                int V_2) //i
+  IL_0000:  ldloca.s   V_0
+  IL_0002:  initobj    ""byte*""
+  IL_0008:  ldc.i4.2
+  IL_0009:  conv.i
+  IL_000a:  stloc.0
+  IL_000b:  ldloc.0
+  IL_000c:  ldc.i4     0x80000001
+  IL_0011:  conv.u
+  IL_0012:  add.ovf.un
+  IL_0013:  stloc.0
+  IL_0014:  ldloc.0
+  IL_0015:  conv.u8
+  IL_0016:  call       ""void System.Console.WriteLine(long)""
+  IL_001b:  ldc.i4.2
+  IL_001c:  conv.i
+  IL_001d:  stloc.1
+  .try
+  {
+    IL_001e:  ldc.i4.m1
+    IL_001f:  stloc.2
+    IL_0020:  ldloc.1
+    IL_0021:  ldloc.2
+    IL_0022:  conv.i
+    IL_0023:  add.ovf.un
+    IL_0024:  stloc.1
+    IL_0025:  ldloc.1
+    IL_0026:  conv.u8
+    IL_0027:  call       ""void System.Console.WriteLine(long)""
+    IL_002c:  leave.s    IL_0042
+  }
+  catch System.OverflowException
+  {
+    IL_002e:  pop
+    IL_002f:  ldstr      ""overflow""
+    IL_0034:  call       ""void System.Console.WriteLine(string)""
+    IL_0039:  ldloc.1
+    IL_003a:  conv.u8
+    IL_003b:  call       ""void System.Console.WriteLine(long)""
+    IL_0040:  leave.s    IL_0042
+  }
+  IL_0042:  ret
+}
+");
+        }
+
         [Fact]
         public void Increment()
         {