Fix instantiation stub maker for case where no register is used by function parameters (#25558)

* Fix instantiation stub maker for case where no register is used by function parameters

3 files changed, 186 insertions, 20 deletions

diff --git a/src/vm/arm/stubs.cpp b/src/vm/arm/stubs.cpp
index 7e7435c107..b876224e1b 100644
--- a/src/vm/arm/stubs.cpp
+++ b/src/vm/arm/stubs.cpp
@@ -1824,33 +1824,42 @@ void StubLinkerCPU::ThumbEmitCallWithGenericInstantiationParameter(MethodDesc *p
     if (cArgDescriptors > 1)
     {
         // Start by assuming we have all four register destination descriptors.
-        DWORD idxLastRegDesc = min(3, cArgDescriptors - 1);
+        int idxLastRegDesc = min(3, cArgDescriptors - 1);
 
         // Adjust that count to match reality.
-        while (!rgArgDescs[idxLastRegDesc].m_fDstIsReg)
+        while (idxLastRegDesc >= 0 && !rgArgDescs[idxLastRegDesc].m_fDstIsReg)
         {
-            _ASSERTE(idxLastRegDesc > 0);
             idxLastRegDesc--;
         }
-
-        // First move to stack location happens after the last move to register location
-        idxFirstMoveToStack = idxLastRegDesc+1;
-
-        // Calculate how many descriptors we'll need to swap.
-        DWORD cSwaps = (idxLastRegDesc + 1) / 2;
-
-        // Finally we can swap the descriptors.
-        DWORD idxFirstRegDesc = 0;
-        while (cSwaps)
+        
+        if (idxLastRegDesc < 0)
         {
-            ArgDesc sTempDesc = rgArgDescs[idxLastRegDesc];
-            rgArgDescs[idxLastRegDesc] = rgArgDescs[idxFirstRegDesc];
-            rgArgDescs[idxFirstRegDesc] = sTempDesc;
+            // No register is used to pass any of the parameters. No need to reverse the order of the descriptors
+            idxFirstMoveToStack = 0;
+        }
+        else
+        {
+            _ASSERTE(idxLastRegDesc >= 0 && ((DWORD)idxLastRegDesc) < cArgDescriptors);
+            
+            // First move to stack location happens after the last move to register location
+            idxFirstMoveToStack = idxLastRegDesc+1;
 
-            _ASSERTE(idxFirstRegDesc < idxLastRegDesc);
-            idxFirstRegDesc++;
-            idxLastRegDesc--;
-            cSwaps--;
+            // Calculate how many descriptors we'll need to swap.
+            DWORD cSwaps = (idxLastRegDesc + 1) / 2;
+
+            // Finally we can swap the descriptors.
+            int idxFirstRegDesc = 0;
+            while (cSwaps)
+            {
+                ArgDesc sTempDesc = rgArgDescs[idxLastRegDesc];
+                rgArgDescs[idxLastRegDesc] = rgArgDescs[idxFirstRegDesc];
+                rgArgDescs[idxFirstRegDesc] = sTempDesc;
+
+                _ASSERTE(idxFirstRegDesc < idxLastRegDesc);
+                idxFirstRegDesc++;
+                idxLastRegDesc--;
+                cSwaps--;
+            }
         }
     }
 
diff --git a/tests/src/Regressions/coreclr/GitHub_24701/test24701.cs b/tests/src/Regressions/coreclr/GitHub_24701/test24701.cs
new file mode 100644
index 0000000000..cb820cc425
--- /dev/null
+++ b/tests/src/Regressions/coreclr/GitHub_24701/test24701.cs
@@ -0,0 +1,126 @@
+// 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;
+
+public struct Data
+{
+    public int _a;
+    public bool _b;
+    public long _c;
+    
+    public override bool Equals(object otherAsObj)
+    {
+        Data other = (Data)otherAsObj;
+        return other._a == _a && other._b == _b && other._c == _c;
+    }
+}
+
+// Test focuses on arm instantiating stub behavior
+
+public struct StructAPITest<T>
+{
+    public string _id;
+    
+    // r0 = thisptr. r1 = instParam
+    public int ReturnsIntNoParams()
+    {
+        Program.s_InstStr = typeof(StructAPITest<T>).ToString() + " - " + _id;
+        return 123;
+    }
+    
+    // r0 = thisptr. r1 = retBuffer. r2 = instParam
+    public Data ReturnNeedBufferNoParams()
+    {
+        Program.s_InstStr = typeof(StructAPITest<T>).ToString() + " - " + _id;
+        Data d; d._a = 123; d._b = true; d._c = 456;
+        return d;
+    }
+
+    // r0 = thisptr. r1 = instParam. r2 = 'val'
+    public int ReturnsIntSmallParam(int val)
+    {
+        Program.s_InstStr = typeof(StructAPITest<T>).ToString() + " - " + _id;
+        return val;
+    }
+
+    // r0 = thisptr. r1 = instParam. r2+r3 = 'val'
+    public int ReturnsIntDoubleRegisterParam(long val)
+    {
+        Program.s_InstStr = typeof(StructAPITest<T>).ToString() + " - " + _id;
+        return (int)val;
+    }
+
+    // r0 = thisptr. r1 = retBuffer. r2 = instParam. r3 = 'val'
+    public Data ReturnNeedBufferSmallParam(int val)
+    {
+        Program.s_InstStr = typeof(StructAPITest<T>).ToString() + " - " + _id;
+        Data d; d._a = val; d._b = true; d._c = 999;
+        return d;
+    }
+
+    // r0 = thisptr. r1 = retBuffer. r2 = instParam. r3 = unused. Stack0+Stack1+Stack2 = 'val'
+    public Data ReturnNeedBufferLargeParam(Data val)
+    {
+        Program.s_InstStr = typeof(StructAPITest<T>).ToString() + " - " + _id;
+        return val;
+    }
+}
+
+public class Program
+{
+    public static string s_InstStr;
+    
+    static int AssertEqual<T>(T actual, T expected)
+    {
+        if (!actual.Equals(expected))
+        {
+            Console.WriteLine("Failed Scenario. Actual = {0}. Expected = {1}", actual, expected);
+            return 1;
+        }
+        return 0;
+    }
+        
+    static int Main()
+    {
+        int numFailures = 0;
+        var foo = new StructAPITest<string>(); foo._id = "ABC";
+        Data d; d._a = 123; d._b = true; d._c = 456;
+        
+        var ReturnsIntNoParams = typeof(StructAPITest<string>).GetMethod("ReturnsIntNoParams");
+        var ReturnNeedBufferNoParams = typeof(StructAPITest<string>).GetMethod("ReturnNeedBufferNoParams");
+        var ReturnsIntSmallParam = typeof(StructAPITest<string>).GetMethod("ReturnsIntSmallParam");
+        var ReturnsIntDoubleRegisterParam = typeof(StructAPITest<string>).GetMethod("ReturnsIntDoubleRegisterParam");
+        var ReturnNeedBufferSmallParam = typeof(StructAPITest<string>).GetMethod("ReturnNeedBufferSmallParam");
+        var ReturnNeedBufferLargeParam = typeof(StructAPITest<string>).GetMethod("ReturnNeedBufferLargeParam");
+        
+        s_InstStr = "";
+        numFailures += AssertEqual(ReturnsIntNoParams.Invoke(foo, new object[] { }), 123);
+        numFailures += AssertEqual(s_InstStr, "StructAPITest`1[System.String] - ABC");
+        
+        s_InstStr = "";
+        numFailures += AssertEqual(ReturnNeedBufferNoParams.Invoke(foo, new object[] { }), d);
+        numFailures += AssertEqual(s_InstStr, "StructAPITest`1[System.String] - ABC");
+        
+        s_InstStr = "";
+        numFailures += AssertEqual(ReturnsIntSmallParam.Invoke(foo, new object[] { (int)3434 }), 3434);
+        numFailures += AssertEqual(s_InstStr, "StructAPITest`1[System.String] - ABC");
+
+        s_InstStr = "";
+        numFailures += AssertEqual(ReturnsIntDoubleRegisterParam.Invoke(foo, new object[] { (long)5656 }), 5656);
+        numFailures += AssertEqual(s_InstStr, "StructAPITest`1[System.String] - ABC");
+
+        s_InstStr = "";
+        d._a = 789;
+        d._c = 999;
+        numFailures += AssertEqual(ReturnNeedBufferSmallParam.Invoke(foo, new object[] { (int)789 }), d);
+        numFailures += AssertEqual(s_InstStr, "StructAPITest`1[System.String] - ABC");
+
+        s_InstStr = "";
+        numFailures += AssertEqual(ReturnNeedBufferLargeParam.Invoke(foo, new object[] { d }), d);
+        numFailures += AssertEqual(s_InstStr, "StructAPITest`1[System.String] - ABC");
+        
+        return numFailures == 0 ? 100 : -1;
+    }
+}
diff --git a/tests/src/Regressions/coreclr/GitHub_24701/test24701.csproj b/tests/src/Regressions/coreclr/GitHub_24701/test24701.csproj
new file mode 100644
index 0000000000..4eec517e75
--- /dev/null
+++ b/tests/src/Regressions/coreclr/GitHub_24701/test24701.csproj
@@ -0,0 +1,31 @@
+<?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>{CBD0D777-3583-49CC-8538-DD84447F6522}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>1</CLRTestPriority>
+    <DebugType>None</DebugType>
+    <Optimize>False</Optimize>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <ItemGroup>
+    <!-- Add Compile Object Here -->
+    <Compile Include="test24701.cs" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>
\ No newline at end of file