JIT: don't keep a stale range when assertion tightening yields an empty range

src/coreclr/jit/rangecheck.cpp

@@ -1677,6 +1677,13 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler*                        comp
         else
         {
             JITDUMP("invalid range after tightening\n");
+            // The tightened range is empty, i.e. the assertions contradict the range we computed.
+            // If the assertion set really did hold here the block would be unreachable, but we get
+            // here with sets that do not hold at this point (e.g. assertions of a use block merged
+            // into the range of a definition that lives in another block). Returning while leaving
+            // the previously tightened range in place lets a fact that has just been disproven
+            // escape to the caller, which then folds branches with it. Bail out to Unknown instead.
+            *pRange = Range(Limit(Limit::keUnknown));
             return;
         }
     }

src/tests/JIT/Regression/JitBlue/Runtime_132879/Runtime_132879.cs

@@ -0,0 +1,330 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Regression test for https://github.com/dotnet/runtime/issues/132879
+//
+// RangeCheck::MergeEdgeAssertionsWorker tightens a range with the incoming assertions. When
+// tightening produced an empty range it returned early and left the previously tightened
+// range in place, so a fact the assertions had just disproven escaped to the caller.
+// Assertion propagation then folded a branch with it: here the range of "op" was computed as
+// [5..7] even though "op" is EqualUserExternalId (78), so the "op > 69" test guarding the
+// operator switch folded to false and execution fell into the default case.
+//
+// The method needs to be compiled with full opts and no PGO data (which is what crossgen2
+// does), hence AggressiveOptimization; with tier1 profile data the bad range is not formed.
+
+using System;
+using System.Collections.Generic;
+using System.Runtime.CompilerServices;
+using System.Text;
+using Xunit;
+
+#nullable enable
+
+namespace Runtime_132879
+{
+    public enum FilterConditionOperator
+    {
+        Equal = 2,
+        NotEqual = 3,
+        GreaterThan = 4,
+        LessThan = 5,
+        GreaterEqual = 6,
+        LessEqual = 7,
+        Like = 8,
+        NotLike = 9,
+        Between = 12,
+        NotBetween = 13,
+        Null = 14,
+        NotNull = 15,
+        EqualUserId = 43,
+        NotEqualUserId = 44,
+        BeginsWith = 64,
+        DoesNotBeginWith = 65,
+        DoesNotEndWith = 66,
+        EndsWith = 67,
+        Contains = 68,
+        DoesNotContain = 69,
+        EqualUserExternalId = 78,
+        NotEqualUserExternalId = 79,
+        EqualDeviceId = 86,
+        NotEqualDeviceId = 87,
+    }
+
+    // Minimal stand-ins for the types the original method depended on. They exist only to keep
+    // the control flow of ConditionBuilder intact.
+
+    public enum MobileObjectType { Text, DateTime, Date, Other }
+    public enum SqlSyntaxType { MicrosoftSQLServer, SQLite }
+    public enum SqlCommandType { Select, Insert, Update, Delete }
+
+    public class ObjectAlias
+    {
+        public string? Alias;
+        public string ObjectName = "";
+        public string SqlAlias = "";
+    }
+
+    public class FilterCondition
+    {
+        public string PropertyName = "";
+        public string? Alias;
+        public FilterConditionOperator Operator;
+        public string[]? Values;
+
+        // In the original, returns a date range, or null if the operator is not a date operator.
+        public Tuple<long, long>? GetDateRange(DateTime prepareDate, object? storageTimeZone) => null;
+    }
+
+    public class DateTimeTypeInfo
+    {
+        public object? StorageTimeZone;
+    }
+
+    public class MobileObjectProperty
+    {
+
+        public MobileObjectType Type;
+        public string? ExternalType;
+        public DateTimeTypeInfo? TypeInfoAsDateTime;
+
+        public static object DeserializeValue(MobileObjectType type, string s) => (object)DateTime.Parse(s);
+        public static string SerializeValue(MobileObjectType type, DateTime dt) => dt.ToString("O");
+    }
+    public class MobileHostInfo
+    {
+        public string DeviceId = "DEV-1";
+    }
+
+    public class MetadataBindingInfo
+    {
+        public MobileHostInfo MobileHost = new();
+        public string UserId = "U-1";
+        public string UserExternalId = "EXT-1";
+        public string GetSqlPropertyName(string objectName, string propertyName) => propertyName;
+    }
+
+    public class MobileException : Exception
+    {
+        private MobileException(string msg) : base(msg) { }
+        public static MobileException CreateError(string msg, string trace) => new MobileException(msg + " | TRACE: " + trace);
+        public static Exception CreateException(string msg, Exception inner) => new MobileException(msg + " -> " + inner.Message);
+    }
+
+    public class Repro
+    {
+        public MetadataBindingInfo? MetadataBinding = new MetadataBindingInfo();
+        public SqlSyntaxType SqlSyntax = SqlSyntaxType.MicrosoftSQLServer;
+        public SqlCommandType CommandType = SqlCommandType.Select;
+        private readonly DateTime _prepareDate = DateTime.UtcNow;
+
+        // Stand-in for a helper method used by the original method (defined elsewhere in the
+        // real class) - for our test scenario it always returns a valid object.
+        private MobileObjectProperty? GetMobileObjectProperty(string objectName, List<string> validPropertyNames, Dictionary<string, string>? propertyBinding, ref string propertyName)
+        {
+            return new MobileObjectProperty { Type = MobileObjectType.Other, ExternalType = "varchar" };
+        }
+
+        private void AppendField(StringBuilder sb, string tabAlias, string sqlPropertyName, string? targetType)
+        {
+            sb.Append('[').Append(tabAlias).Append('.').Append(sqlPropertyName).Append(']');
+        }
+
+        private bool ConditionBuilder_DateRange(StringBuilder sb, MobileObjectProperty mop, Tuple<long, long>? dateRange, Action<StringBuilder> appendField)
+        {
+            // Not exercised in our test scenario (op is not a date operator) - always false.
+            return false;
+        }
+
+        private string InsertParameter(object value) => "@p_" + value;
+        private string InsertSerializedParameter(MobileObjectProperty mop, string? val) => "@p_" + val;
+        [MethodImpl(MethodImplOptions.AggressiveOptimization | MethodImplOptions.NoInlining)]
+        public string? ConditionBuilder(string objectName, IReadOnlyList<ObjectAlias> aliases, string tabAlias, FilterCondition fc, List<string> validPropertyNames, Dictionary<string, string>? propertyBinding = null)
+        {
+            try
+            {
+                var sb = new StringBuilder();
+                var propertyName = fc.PropertyName;
+                if (MetadataBinding != null)
+                {
+                    var op = fc.Operator;
+                    var vals = fc.Values;
+                    var mop = GetMobileObjectProperty(objectName, validPropertyNames, propertyBinding, ref propertyName);
+                    if (mop == null) return null;
+                    bool isNull = false;
+                    bool isNotNull = false;
+                    switch (op)
+                    {
+                        case FilterConditionOperator.Null: isNull = true; break;
+                        case FilterConditionOperator.NotNull: isNotNull = true; break;
+                        default:
+                            if (vals != null && vals.Length == 1 && (vals[0] == null || mop.Type != MobileObjectType.Text && string.IsNullOrEmpty(vals[0])))
+                            {
+                                // if the value stands for the null operator
+                                // if '=' then check for NULL
+                                // TODO: option to skip the operator for this comparison
+                                if (op == FilterConditionOperator.Equal) isNull = true;
+                                else return null; // skip this fragment
+                            }
+                            break;
+                    }
+                    string? targetType = null;
+                    void AppendCurrentField(StringBuilder sbForField)
+                    {
+                        AppendField(sbForField, tabAlias, MetadataBinding.GetSqlPropertyName(objectName, propertyName), targetType);
+                    }
+
+                    AppendCurrentField(sb);
+                    var matched = true;
+                    if (isNull) sb.Append(" IS NULL");
+                    else if (isNotNull) sb.Append(" IS NOT NULL");
+                    else matched = false;
+                    if (!matched)
+                    {
+                        var dateRange = fc.GetDateRange(_prepareDate, mop.TypeInfoAsDateTime?.StorageTimeZone);
+                        if (dateRange != null)
+                        {
+                            matched = ConditionBuilder_DateRange(sb, mop, dateRange, AppendCurrentField);
+                        }
+                    }
+                    if (!matched)
+                    {
+                        // range correction may be needed
+                        if (mop.Type == MobileObjectType.DateTime && vals != null)
+                        {
+                            if (op == FilterConditionOperator.LessEqual && vals.Length == 1)
+                            {
+                                // if only a date on the right-hand side
+                                if (vals[0].Length == 10) // YYYY-MM-DD - date without time
+                                {
+                                    var dt = (DateTime)MobileObjectProperty.DeserializeValue(MobileObjectType.Date, vals[0]);
+                                    op = FilterConditionOperator.LessThan;
+                                    vals = new[] { MobileObjectProperty.SerializeValue(MobileObjectType.Date, dt.AddDays(1)) };
+                                }
+                            }
+                            else if ((op == FilterConditionOperator.Between || op == FilterConditionOperator.NotBetween) && vals.Length == 2)
+                            {
+                                if (vals[1].Length == 10) // YYYY-MM-DD - date without time
+                                {
+                                    vals = new[] { vals[0], vals[1] + "T23:59:59.9999999" };
+                                }
+                            }
+                        }
+                    }
+                    if (!matched)
+                    {
+                        matched = true;
+                        switch (op)
+                        {
+                            case FilterConditionOperator.EqualUserId:
+                            case FilterConditionOperator.EqualUserExternalId:
+                            case FilterConditionOperator.Equal:
+                            case FilterConditionOperator.EqualDeviceId:
+                                sb.Append('='); break;
+                            case FilterConditionOperator.NotEqual:
+                            case FilterConditionOperator.NotEqualUserId:
+                            case FilterConditionOperator.NotEqualUserExternalId:
+                            case FilterConditionOperator.NotEqualDeviceId:
+                                sb.Append("<>"); break;
+                            case FilterConditionOperator.GreaterThan: sb.Append('>'); break;
+                            case FilterConditionOperator.LessThan: sb.Append('<'); break;
+                            case FilterConditionOperator.GreaterEqual: sb.Append(">="); break;
+                            case FilterConditionOperator.LessEqual: sb.Append("<="); break;
+                            case FilterConditionOperator.Like:
+                            case FilterConditionOperator.BeginsWith:
+                            case FilterConditionOperator.EndsWith:
+                            case FilterConditionOperator.Contains:
+                                sb.Append(" LIKE ");
+                                break;
+                            case FilterConditionOperator.NotLike:
+                            case FilterConditionOperator.DoesNotBeginWith:
+                            case FilterConditionOperator.DoesNotEndWith:
+                            case FilterConditionOperator.DoesNotContain:
+                                sb.Append(" NOT LIKE ");
+                                break;
+                            default: matched = false; break;
+                        }
+                        if (matched)
+                        {
+                            switch (op)
+                            {
+                                case FilterConditionOperator.EqualUserId:
+                                case FilterConditionOperator.NotEqualUserId:
+                                    sb.Append(InsertParameter(MetadataBinding.UserId));
+                                    break;
+                                case FilterConditionOperator.EqualUserExternalId:
+                                case FilterConditionOperator.NotEqualUserExternalId:
+                                    sb.Append(InsertParameter(MetadataBinding.UserExternalId));
+                                    break;
+                                case FilterConditionOperator.BeginsWith:
+                                case FilterConditionOperator.DoesNotBeginWith:
+                                    sb.Append(InsertSerializedParameter(mop, vals![0] + "%"));
+                                    break;
+                                case FilterConditionOperator.EndsWith:
+                                case FilterConditionOperator.DoesNotEndWith:
+                                    sb.Append(InsertSerializedParameter(mop, "%" + vals![0]));
+                                    break;
+                                case FilterConditionOperator.Contains:
+                                case FilterConditionOperator.DoesNotContain:
+                                    sb.Append(InsertSerializedParameter(mop, "%" + vals![0] + "%"));
+                                    break;
+                                case FilterConditionOperator.EqualDeviceId:
+                                case FilterConditionOperator.NotEqualDeviceId:
+                                    sb.Append(InsertParameter(MetadataBinding.MobileHost.DeviceId));
+                                    break;
+                                default:
+                                    sb.Append(InsertSerializedParameter(mop, vals![0]));
+                                    break;
+                            }
+                        }
+                    }
+                    if (!matched)
+                    {
+                        throw MobileException.CreateError($"Unknown FilterConditionOperator: {op.ToString()} ({(int)op}), partial condition={sb}", "");
+                    }
+                }
+                else
+                {
+                    sb.Append(propertyName);
+                    sb.Append(' ');
+                    sb.Append(fc.Operator); // quick workaround - sufficient for comparison
+                    sb.Append(' ');
+                    if (fc.Values != null)
+                    {
+                        foreach (var v in fc.Values)
+                        {
+                            sb.Append('\'');
+                            sb.Append(v.Replace("'", "''"));
+                            sb.Append('\'');
+                        }
+                    }
+                }
+                return sb.ToString();
+            }
+            catch (Exception ex)
+            {
+                throw MobileException.CreateException($"Condition: '{objectName}'.'{fc?.PropertyName}' {fc?.Operator}", ex);
+            }
+        }
+    }
+
+    public class Runtime_132879
+    {
+        [Fact]
+        public static void TestEntryPoint()
+        {
+            var repro = new Repro();
+            var fc = new FilterCondition
+            {
+                PropertyName = "ExternalId",
+                Alias = null,
+                Operator = FilterConditionOperator.EqualUserExternalId,
+                Values = null
+            };
+
+            string? result = repro.ConditionBuilder("MyObject", new List<ObjectAlias>(), "t0", fc, new List<string> { "ExternalId" });
+
+            Assert.Equal("[t0.ExternalId]=@p_EXT-1", result);
+        }
+    }
+}

src/tests/JIT/Regression/Regression_ro_2.csproj

@@ -132,6 +132,7 @@
     <Compile Include="JitBlue\Runtime_132370\Runtime_132370.cs" />
     <Compile Include="JitBlue\Runtime_132243\Runtime_132243.cs" />
     <Compile Include="JitBlue\Runtime_132784\Runtime_132784.cs" />
+    <Compile Include="JitBlue\Runtime_132879\Runtime_132879.cs" />
     <Compile Include="JitBlue\Runtime_132785\Runtime_132785.cs" />
   </ItemGroup>
   <Import Project="$(TestSourceDir)MergedTestRunner.targets" />