Add back missing conv opcodes when compiling via System.Linq.Expressions (#76024)
authorVlad Brezae <brezaevlad@gmail.com>
Sat, 24 Sep 2022 06:23:23 +0000 (09:23 +0300)
committerGitHub <noreply@github.com>
Sat, 24 Sep 2022 06:23:23 +0000 (09:23 +0300)
* Add new convert tests

With expression funcs that return int32 instead of short/byte to prevent implicit conversions.

* Add back missing conv opcodes when compiling via System.Linq.Expressions

The conversion opcodes are still necessary when the sign of the value might change, in which case the conversion opcode will do a sign/zero extend to the full i32 storage used by the IL execution stack.

For example, before this change, conversions from ushort to short were ignored. Consider expressions converting the value `ushort.MaxValue` to short (testcase ConvertUShortToShortTest). `ushort.MaxValue` will be pushed to execution stack as a i32 ldc of value 0xffff. The conv.i2 opcode would change the value on the stack to 0xffffffff so it shouldn't be omitted.

src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/ILGen.cs
src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs

index 5cd7fd5..bb7aaa1 100644 (file)
@@ -673,11 +673,6 @@ namespace System.Linq.Expressions.Compiler
                     }
                     else
                     {
-                        if (tf == TypeCode.Byte)
-                        {
-                            return;
-                        }
-
                         convCode = OpCodes.Conv_I1;
                     }
 
@@ -689,11 +684,6 @@ namespace System.Linq.Expressions.Compiler
                     }
                     else
                     {
-                        if (tf == TypeCode.SByte)
-                        {
-                            return;
-                        }
-
                         convCode = OpCodes.Conv_U1;
                     }
 
@@ -704,14 +694,6 @@ namespace System.Linq.Expressions.Compiler
                         case TypeCode.SByte:
                         case TypeCode.Byte:
                             return;
-                        case TypeCode.Char:
-                        case TypeCode.UInt16:
-                            if (!isChecked)
-                            {
-                                return;
-                            }
-
-                            break;
                     }
 
                     convCode = isChecked
@@ -726,14 +708,6 @@ namespace System.Linq.Expressions.Compiler
                         case TypeCode.Char:
                         case TypeCode.UInt16:
                             return;
-                        case TypeCode.SByte:
-                        case TypeCode.Int16:
-                            if (!isChecked)
-                            {
-                                return;
-                            }
-
-                            break;
                     }
 
                     convCode = isChecked
index eb7d20b..dd9c274 100644 (file)
@@ -183,6 +183,15 @@ namespace System.Linq.Expressions.Tests
         }
 
         [Theory, ClassData(typeof(CompilationTypes))]
+        public static void ConvertByteToSByteRetIntTest(bool useInterpreter)
+        {
+            foreach (byte value in new byte[] { 0, 1, byte.MaxValue })
+            {
+                VerifyByteToSByteRetInt(value, useInterpreter);
+            }
+        }
+
+        [Theory, ClassData(typeof(CompilationTypes))]
         public static void ConvertByteToNullableSByteTest(bool useInterpreter)
         {
             foreach (byte value in new byte[] { 0, 1, byte.MaxValue })
@@ -4413,6 +4422,15 @@ namespace System.Linq.Expressions.Tests
         }
 
         [Theory, ClassData(typeof(CompilationTypes))]
+        public static void ConvertSByteToByteRetIntTest(bool useInterpreter)
+        {
+            foreach (sbyte value in new sbyte[] { 0, 1, -1, sbyte.MinValue, sbyte.MaxValue })
+            {
+                VerifySByteToByteRetInt(value, useInterpreter);
+            }
+        }
+
+        [Theory, ClassData(typeof(CompilationTypes))]
         public static void ConvertSByteToNullableByteTest(bool useInterpreter)
         {
             foreach (sbyte value in new sbyte[] { 0, 1, -1, sbyte.MinValue, sbyte.MaxValue })
@@ -5151,6 +5169,15 @@ namespace System.Linq.Expressions.Tests
         }
 
         [Theory, ClassData(typeof(CompilationTypes))]
+        public static void ConvertShortToUShortRetIntTest(bool useInterpreter)
+        {
+            foreach (short value in new short[] { 0, 1, -1, short.MinValue, short.MaxValue })
+            {
+                VerifyShortToUShortRetInt(value, useInterpreter);
+            }
+        }
+
+        [Theory, ClassData(typeof(CompilationTypes))]
         public static void ConvertShortToNullableUShortTest(bool useInterpreter)
         {
             foreach (short value in new short[] { 0, 1, -1, short.MinValue, short.MaxValue })
@@ -6609,6 +6636,15 @@ namespace System.Linq.Expressions.Tests
         }
 
         [Theory, ClassData(typeof(CompilationTypes))]
+        public static void ConvertUShortToShortRetIntTest(bool useInterpreter)
+        {
+            foreach (ushort value in new ushort[] { 0, 1, ushort.MaxValue })
+            {
+                VerifyUShortToShortRetInt(value, useInterpreter);
+            }
+        }
+
+        [Theory, ClassData(typeof(CompilationTypes))]
         public static void ConvertUShortToNullableShortTest(bool useInterpreter)
         {
             foreach (ushort value in new ushort[] { 0, 1, ushort.MaxValue })
@@ -7136,6 +7172,17 @@ namespace System.Linq.Expressions.Tests
             Assert.Equal(unchecked((sbyte)value), f());
         }
 
+        private static void VerifyByteToSByteRetInt(byte value, bool useInterpreter)
+        {
+            Expression<Func<int>> e =
+                Expression.Lambda<Func<int>>(
+                    Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(byte)), typeof(sbyte)), typeof(int)),
+                    Enumerable.Empty<ParameterExpression>());
+            Func<int> f = e.Compile(useInterpreter);
+
+            Assert.Equal((int)unchecked((sbyte)value), f());
+        }
+
         private static void VerifyByteToNullableSByte(byte value, bool useInterpreter)
         {
             Expression<Func<sbyte?>> e =
@@ -13178,6 +13225,17 @@ namespace System.Linq.Expressions.Tests
             Assert.Equal(unchecked((byte)value), f());
         }
 
+        private static void VerifySByteToByteRetInt(sbyte value, bool useInterpreter)
+        {
+            Expression<Func<int>> e =
+                Expression.Lambda<Func<int>>(
+                    Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(sbyte)), typeof(byte)), typeof(int)),
+                    Enumerable.Empty<ParameterExpression>());
+            Func<int> f = e.Compile(useInterpreter);
+
+            Assert.Equal((int)unchecked((byte)value), f());
+        }
+
         private static void VerifySByteToNullableByte(sbyte value, bool useInterpreter)
         {
             Expression<Func<byte?>> e =
@@ -14122,6 +14180,17 @@ namespace System.Linq.Expressions.Tests
             Assert.Equal(unchecked((ushort)value), f());
         }
 
+        private static void VerifyShortToUShortRetInt(short value, bool useInterpreter)
+        {
+            Expression<Func<int>> e =
+                Expression.Lambda<Func<int>>(
+                    Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(short)), typeof(ushort)), typeof(int)),
+                    Enumerable.Empty<ParameterExpression>());
+            Func<int> f = e.Compile(useInterpreter);
+
+            Assert.Equal((int)unchecked((ushort)value), f());
+        }
+
         private static void VerifyShortToNullableUShort(short value, bool useInterpreter)
         {
             Expression<Func<ushort?>> e =
@@ -16030,6 +16099,17 @@ namespace System.Linq.Expressions.Tests
             Assert.Equal(unchecked((short)value), f());
         }
 
+        private static void VerifyUShortToShortRetInt(ushort value, bool useInterpreter)
+        {
+            Expression<Func<int>> e =
+                Expression.Lambda<Func<int>>(
+                    Expression.Convert(Expression.Convert(Expression.Constant(value, typeof(ushort)), typeof(short)), typeof(int)),
+                    Enumerable.Empty<ParameterExpression>());
+            Func<int> f = e.Compile(useInterpreter);
+
+            Assert.Equal((int)unchecked((short)value), f());
+        }
+
         private static void VerifyUShortToNullableShort(ushort value, bool useInterpreter)
         {
             Expression<Func<short?>> e =