Fix Issue #25134 - AssertionProp incorrectly removes cast from uint

Add additional check for the GT_UNSIGNED flag
+ Ran clang-format
+ Code review feedback, use IsUnsigned()

5 files changed, 156 insertions, 10 deletions

diff --git a/src/jit/assertionprop.cpp b/src/jit/assertionprop.cpp
index 5b62e88303..ae73c8800b 100644
--- a/src/jit/assertionprop.cpp
+++ b/src/jit/assertionprop.cpp
@@ -2213,13 +2213,16 @@ AssertionIndex Compiler::optFindComplementary(AssertionIndex assertIndex)
 
 /*****************************************************************************
  *
- *  Given a lclNum and a toType, return assertion index of the assertion that
- *  claims that a variable's value is always a valid subrange of toType.
- *  Thus we can discard or omit a cast to toType. Returns NO_ASSERTION_INDEX
+ *  Given a lclNum, a fromType and a toType, return assertion index of the assertion that
+ *  claims that a variable's value is always a valid subrange of the formType.
+ *  Thus we can discard or omit a cast to fromType. Returns NO_ASSERTION_INDEX
  *  if one such assertion could not be found in "assertions."
  */
 
-AssertionIndex Compiler::optAssertionIsSubrange(GenTree* tree, var_types toType, ASSERT_VALARG_TP assertions)
+AssertionIndex Compiler::optAssertionIsSubrange(GenTree*         tree,
+                                                var_types        fromType,
+                                                var_types        toType,
+                                                ASSERT_VALARG_TP assertions)
 {
     if (!optLocalAssertionProp && BitVecOps::IsEmpty(apTraits, assertions))
     {
@@ -2243,6 +2246,16 @@ AssertionIndex Compiler::optAssertionIsSubrange(GenTree* tree, var_types toType,
                 continue;
             }
 
+            // If we have an unsigned fromType, then the loBound can't be negative
+            //
+            if (varTypeIsUnsigned(fromType))
+            {
+                if (curAssertion->op2.u2.loBound < 0)
+                {
+                    continue;
+                }
+            }
+
             // Make sure the toType is within current assertion's bounds.
             switch (toType)
             {
@@ -3384,11 +3397,18 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTree* t
 {
     assert(tree->gtOper == GT_CAST);
 
-    var_types toType = tree->gtCast.gtCastType;
-    GenTree*  op1    = tree->gtCast.CastOp();
+    var_types fromType = tree->CastFromType();
+    var_types toType   = tree->gtCast.gtCastType;
+    GenTree*  op1      = tree->gtCast.CastOp();
+
+    // force the fromType to unsigned if GT_UNSIGNED flag is set
+    if (tree->IsUnsigned())
+    {
+        fromType = genUnsignedType(fromType);
+    }
 
     // If we have a cast involving floating point types, then bail.
-    if (varTypeIsFloating(toType) || varTypeIsFloating(op1->TypeGet()))
+    if (varTypeIsFloating(toType) || varTypeIsFloating(fromType))
     {
         return nullptr;
     }
@@ -3406,7 +3426,7 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTree* t
         return nullptr;
     }
 
-    AssertionIndex index = optAssertionIsSubrange(lcl, toType, assertions);
+    AssertionIndex index = optAssertionIsSubrange(lcl, fromType, toType, assertions);
     if (index != NO_ASSERTION_INDEX)
     {
         LclVarDsc* varDsc = &lvaTable[lcl->gtLclVarCommon.gtLclNum];
diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index bdc9a5c262..90aec385b6 100644
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -6667,7 +6667,10 @@ public:
     ASSERT_TP optGetVnMappedAssertions(ValueNum vn);
 
     // Used for respective assertion propagations.
-    AssertionIndex optAssertionIsSubrange(GenTree* tree, var_types toType, ASSERT_VALARG_TP assertions);
+    AssertionIndex optAssertionIsSubrange(GenTree*         tree,
+                                          var_types        fromType,
+                                          var_types        toType,
+                                          ASSERT_VALARG_TP assertions);
     AssertionIndex optAssertionIsSubtype(GenTree* tree, GenTree* methodTableArg, ASSERT_VALARG_TP assertions);
     AssertionIndex optAssertionIsNonNullInternal(GenTree* op, ASSERT_VALARG_TP assertions);
     bool optAssertionIsNonNull(GenTree*         op,
diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp
index 5c13c2bc1a..610bd17805 100644
--- a/src/jit/morph.cpp
+++ b/src/jit/morph.cpp
@@ -6084,7 +6084,7 @@ GenTree* Compiler::fgMorphLocalVar(GenTree* tree, bool forceRemorph)
     {
 #if LOCAL_ASSERTION_PROP
         /* Assertion prop can tell us to omit adding a cast here */
-        if (optLocalAssertionProp && optAssertionIsSubrange(tree, varType, apFull) != NO_ASSERTION_INDEX)
+        if (optLocalAssertionProp && optAssertionIsSubrange(tree, TYP_INT, varType, apFull) != NO_ASSERTION_INDEX)
         {
             return tree;
         }
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_25134/GitHub_25134.cs b/tests/src/JIT/Regression/JitBlue/GitHub_25134/GitHub_25134.cs
new file mode 100644
index 0000000000..e68e2b82e8
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_25134/GitHub_25134.cs
@@ -0,0 +1,90 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Runtime.CompilerServices;
+
+class Program
+{
+    static bool s_caughtException;
+    static uint s_value = int.MaxValue + 1U;
+    static int  s_result = 0;
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    static int CastToIntChecked(uint value)
+    {
+        return checked((int)value);
+    }
+
+    // Testing a checked cast to Uint -- the inlining case
+    //
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void Test1()
+    {
+        int result = CastToIntChecked(s_value);
+	s_result = result;
+	Console.WriteLine("Result is " + result);
+    }
+
+    // Testing a checked cast to Uint -- the non-inlining case
+    //
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static void Test2()
+    {
+        uint copy = 0;
+        try
+        {
+            s_caughtException = false;
+
+            copy = s_value;
+
+            int result = checked((int)copy);
+            s_result = result;
+            Console.WriteLine("Result is " + result);
+        }
+        catch (System.OverflowException ex)
+        {
+            s_caughtException = true;
+            Console.WriteLine("CORRECT: " + ex);
+            copy = 0;
+        }
+    }
+
+    static int Main()
+    {
+        bool failed = false;
+
+        try
+        {
+            Test1();
+        }
+        catch (System.OverflowException ex)
+        {
+            s_caughtException = true;
+            Console.WriteLine("CORRECT: " + ex);
+        }
+
+        if (s_caughtException == false)
+        {
+            Console.WriteLine("FAILED - Test1");
+            failed = true;
+        }
+
+        Test2();
+        if (s_caughtException == false)
+        {
+            Console.WriteLine("FAILED - Test2");
+            failed = true;
+        }
+
+        if (failed)
+        {
+            return 101;
+        }
+        else
+        {
+            return 100;
+        }
+    }
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_25134/GitHub_25134.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_25134/GitHub_25134.csproj
new file mode 100644
index 0000000000..95052d9884
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_25134/GitHub_25134.csproj
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' " />
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>