From 249a7b7a6ec4be4fdd293a9fc4bcea03b13df21f Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 28 Aug 2026 19:39:58 +0200 Subject: [PATCH] JIT: don't keep a stale range when assertion tightening yields an empty range RangeCheck::MergeEdgeAssertionsWorker tightens *pRange with each incoming assertion. When the tightened range came out empty it logged "invalid range after tightening" and returned, leaving the previously tightened range in place. That range has just been contradicted by the assertions, so the caller ends up holding a fact that was disproven. Keeping it would only be safe if the assertion set really held at that point (then the block is unreachable and any range is vacuously fine). It does not always hold: ComputeRangeForLocalDef merges the use block's bbAssertionIn into the range of a definition that lives in another block, and that set can contain mutually exclusive assertions. Since #129354 assertion propagation folds conditions using these ranges, so the stale range turns into wrong code. In #132879 the range of an enum local was computed as [5..7] while its value was 78, which folded the "op > 69" test guarding a switch to false and sent execution to the default case. Bail out to Unknown instead. Fixes #132879 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 71e21c42-60e7-47f1-9026-4bf2df7a8e51 --- src/coreclr/jit/rangecheck.cpp | 7 + .../JitBlue/Runtime_132879/Runtime_132879.cs | 330 ++++++++++++++++++ .../JIT/Regression/Regression_ro_2.csproj | 1 + 3 files changed, 338 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_132879/Runtime_132879.cs diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index d053476b69c905..7ae3e2118ad165 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/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; } } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_132879/Runtime_132879.cs b/src/tests/JIT/Regression/JitBlue/Runtime_132879/Runtime_132879.cs new file mode 100644 index 00000000000000..5d1d64c2dcd452 --- /dev/null +++ b/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? 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 validPropertyNames, Dictionary? 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? dateRange, Action 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 aliases, string tabAlias, FilterCondition fc, List validPropertyNames, Dictionary? 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(), "t0", fc, new List { "ExternalId" }); + + Assert.Equal("[t0.ExternalId]=@p_EXT-1", result); + } + } +} diff --git a/src/tests/JIT/Regression/Regression_ro_2.csproj b/src/tests/JIT/Regression/Regression_ro_2.csproj index 2c1cc036ff1350..607a2bbea36ce2 100644 --- a/src/tests/JIT/Regression/Regression_ro_2.csproj +++ b/src/tests/JIT/Regression/Regression_ro_2.csproj @@ -132,6 +132,7 @@ +